<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/103408/">http://git.reviewboard.kde.org/r/103408/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 15th, 2011, 1:59 a.m., <b>Marijn Kruisselbrink</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/103408/diff/1/?file=43356#file43356line173" style="color: black; font-weight: bold; text-decoration: underline;">tables/Region.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">Region::Region(const QPoint& point, Sheet* sheet)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">173</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">//d->cells.reserve(list.d->cells.size());</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">commented out code is probably a leftover from something else?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">That was for the next step so I can check how much we benefit from it. Will be commented out before I commit.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 15th, 2011, 1:59 a.m., <b>Marijn Kruisselbrink</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/103408/diff/1/?file=43358#file43358line467" style="color: black; font-weight: bold; text-decoration: underline;">tables/Util.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool Calligra::Tables::Util::localReferenceAnchor(const QString &_ref)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">454</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span> <span class="n">midString</span><span class="p">(</span><span class="n">expression</span><span class="p">.</span><span class="n">mid</span><span class="p">(</span><span class="n">i</span><span class="p">));</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Will check and measure the effect, but it looks like it should work easily.</pre>
<br />
<p>- Thorsten</p>
<br />
<p>On December 14th, 2011, 11:18 a.m., Thorsten Zachmann wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Calligra.</div>
<div>By Thorsten Zachmann.</div>
<p style="color: grey;"><i>Updated Dec. 14, 2011, 11:18 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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. </pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=288959">288959</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>tables/DependencyManager.cpp <span style="color: grey">(2313e46)</span></li>
<li>tables/Formula.h <span style="color: grey">(5a996f3)</span></li>
<li>tables/Formula.cpp <span style="color: grey">(e79ae1b)</span></li>
<li>tables/Region.h <span style="color: grey">(261beb4)</span></li>
<li>tables/Region.cpp <span style="color: grey">(1920ca9)</span></li>
<li>tables/Util.h <span style="color: grey">(b1879fb)</span></li>
<li>tables/Util.cpp <span style="color: grey">(ac03f0f)</span></li>
<li>tables/tests/CMakeLists.txt <span style="color: grey">(9cc51db)</span></li>
<li>tables/tests/TestUtil.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tables/tests/TestUtil.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103408/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>