Review Request: Optimize loading speed of tables

[Jarosław Staniek]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103408/#review8952
-----------------------------------------------------------



tables/Formula.cpp
<http://git.reviewboard.kde.org/r/103408/#comment7435>

    Maybe this class' methods could be inlined if used a lot internally?
    I am sure there are other as well.



tables/Formula.cpp
<http://git.reviewboard.kde.org/r/103408/#comment7436>

    Can't we have Token::Plus == '+' as in flex+bison-generated parsers; then this could would become much simpler and probably faster?



tables/Region.h
<http://git.reviewboard.kde.org/r/103408/#comment7437>

    Just a note: for readability of the function body, in such code I avoid using *& and instead use **.
    


- Jarosław Staniek


On Dec. 14, 2011, 11:18 a.m., Thorsten Zachmann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103408/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2011, 11:18 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> Optimize formula parsing by a factor of 2-4
>     
> This optimizes formula parsing by a factor of 2-4. The file
> www.worldmapper.org%2Fdata%2Funderlying%2FWorldmapper_U3_hdr%304_tables1-22.ods
> loads now in 25 instead of 30 seconds.
>     
> The patch tries to avoid creating QString objects when not necessary and reuse
> QString object when possible. The reduces the number of allocations quite a lot.
> Also implicit casts of 'x' and "foo" are avoided where ever possible as these
> are also quite expensive.
>     
> e.g. some of the easy optimizations are
>     
> if (QString::operator[0] == '$')
>     
> is better written as
>     
> if (QString::operator[0] == QChar('$', 0)
>     
> as this can be optimized already be the compiler and thus reduces the stuff needed
> otherwise.
>     
> Also initialize class members in initialization list of the constructor where possible
> as this avoids copying the data a second time which is also expensive if it is e.g. a
> QString.
>     
> Additionally to the speed up also the formula parsing now can recognice a escaped " in
> a formula and parsing of error codes has been updated to follow ODF 1.2. This fixes the
> failing unit tests in TestFormula.
>     
> Added unit test for decodeFormula.
> 
> 
> This addresses bug 288959.
>     http://bugs.kde.org/show_bug.cgi?id=288959
> 
> 
> Diffs
> -----
> 
>   tables/DependencyManager.cpp 2313e46 
>   tables/Formula.h 5a996f3 
>   tables/Formula.cpp e79ae1b 
>   tables/Region.h 261beb4 
>   tables/Region.cpp 1920ca9 
>   tables/Util.h b1879fb 
>   tables/Util.cpp ac03f0f 
>   tables/tests/CMakeLists.txt 9cc51db 
>   tables/tests/TestUtil.h PRE-CREATION 
>   tables/tests/TestUtil.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/103408/diff/diff
> 
> 
> Testing
> -------
> 
> Run the changes against ~ 500 docs and compared the pre patch/after values returned by scan and decodeFormula. The results of decodeFormula where all the same pre/after. The result of scan has changed due to the fixed error and string handling. 
> 
> 
> Thanks,
> 
> Thorsten Zachmann
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20111214/b9a7cc7f/attachment.htm>