Review Request: Optimize loading speed of tables

[Thorsten Zachmann]

> On Dec. 15, 2011, 1:59 a.m., Marijn Kruisselbrink wrote:
> > tables/Region.cpp, line 173
> > <http://git.reviewboard.kde.org/r/103408/diff/1/?file=43356#file43356line173>
> >
> >     commented out code is probably a leftover from something else?

That was for the next step so I can check how much we benefit from it. Will be commented out before I commit.


> On Dec. 15, 2011, 1:59 a.m., Marijn Kruisselbrink wrote:
> > tables/Util.cpp, line 467
> > <http://git.reviewboard.kde.org/r/103408/diff/1/?file=43358#file43358line467>
> >
> >     I wonder if it wouldn't be possible to completely eliminate the copying of parts of the expression, by instead of having the current midString.startsWith() have expression.midRef(i, 10) == QLatin1String("ERROR.TYPE") and similar for the other functions

Will check and measure the effect, but it looks like it should work easily.


- Thorsten


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


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/20111215/d8a0bb90/attachment.htm>