<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/111773/">http://git.reviewboard.kde.org/r/111773/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 29th, 2013, 4:32 a.m. UTC, <b>Thorsten Zachmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hello Jos,
have you been doing some comparison in speed and memory usage for the writing out of odf? I would be very interested to see how it performs there.</pre>
</blockquote>
<p>On July 29th, 2013, 7:17 a.m. UTC, <b>Jos van den Oever</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">No, I havent done any. With the commandline program ppttoodp and a ppt with many slides this could be done. I'm pretty sure the effect will be invisible, but I can try it. What analytical runs would you like to see?</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Built Calligra with CMAKE_BUILD_TYPE=Release
Ran this command:
LD_LIBRARY_PATH= /usr/bin/time filters/stage/powerpoint/ppttoodp calligratests/import/powerpoint/75776.ppt out.odp
Without patch:
lib/libkoodf.so.13.0.0 731284 bytes
filters/stage/powerpoint/ppttoodp 2027008 bytes
2.32user 0.14system 0:02.55elapsed 96%CPU (0avgtext+0avgdata 60880maxresident)k
0inputs+37904outputs (0major+17666minor)pagefaults 0swaps
2.37user 0.15system 0:02.61elapsed 96%CPU (0avgtext+0avgdata 60876maxresident)k
0inputs+37904outputs (0major+17154minor)pagefaults 0swaps
2.31user 0.14system 0:02.54elapsed 96%CPU (0avgtext+0avgdata 60944maxresident)k
0inputs+37904outputs (0major+18175minor)pagefaults 0swaps
With patch:
filters/stage/powerpoint/ppttoodp 2053906 bytes
lib/libkoodf.so.13.0.0 731284 bytes
2.31user 0.15system 0:02.54elapsed 96%CPU (0avgtext+0avgdata 60964maxresident)k
0inputs+37904outputs (0major+15116minor)pagefaults 0swaps
2.30user 0.15system 0:02.53elapsed 97%CPU (0avgtext+0avgdata 61032maxresident)k
0inputs+37904outputs (0major+15627minor)pagefaults 0swaps
2.35user 0.20system 0:02.64elapsed 96%CPU (0avgtext+0avgdata 61028maxresident)k
0inputs+37904outputs (0major+13584minor)pagefaults 0swaps
Summary:
Binary is 1.3% larger.
user runtime from 2.35 to 2.32 (slightly faster)
maxresident from 60900 to 61008 (slightly larger)
minor page faults from 17665 to 14776 (quite a bit less)
So it seems code locality compensates for the extra instructions.
Overall the difference is not that significant and when most or all of the code is ported it will be easy to do global optimizations by optimizing the generated code.
</pre>
<br />
<p>- Jos</p>
<br />
<p>On July 28th, 2013, 9:47 p.m. UTC, Jos van den Oever wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Calligra.</div>
<div>By Jos van den Oever.</div>
<p style="color: grey;"><i>Updated July 28, 2013, 9:47 p.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;">This patch is also available in the branch libs-writeodf-vandenoever-2.
Two years ago I wrote an initial version of this patch and a detailed discussion on the mailing list [1] followed. The main objections to the patch have been dealt with (see below).
Most of this new version was written at Akademy in Bilbao.
Very short summary of the patch:
This patch should help everybody, young and old, with coding C++ for writing ODF and make errors easier to catch.
The OpenDocument Format specification is published with a Relax NG file that specifies the XML format. This file can be used to check if ODF files are valid. It can also be used to generate a C++ API headers. This is what this patch does.
Example:
Instead of writing:
==
xmlWriter->startElement("text:p");
xmlWriter->addAttribute("text:style-name", "italic");
xmlWriter->startElement("text:p");
xmlWriter->addAttribute("text:style-name", "bold");
xmlWriter->addTextNode("Hello World!");
xmlWriter->endElement();
xmlWriter->endElement();
==
you can write:
==
text_p p(xmlWriter);
p.set_text_style_name("italic");
text_span span(p.add_text_span());
span.set_text_style_name("italic");
span.addTextNode("Hello World!");
==
Some advantages:
- autocompletion when coding: faster coding
- tag and attribute names are not strings but class and function names: less errors
- nesting is checked by the compiler
- you write to elements (span, p), not xmlwriter: easier to read
- required attributes are part of the element constructor
Implementation considerations:
- Calligra is large, so the generated code mixes well with the use of KoXmlWriter and porting can be done in small steps.
- class and function names are similar to the xml tags with ':' and '-' replaced by '_'.
- stack based: no heap allocations
- only header files: all code will inline and have low impact on runtime
- modular: one header file per namespace to reduce compile overhead
- code generator is Qt code, part of Calligra and runs as a build step
Not in this patch (and places where you can help in the future):
- generate enumerations based on Relax NG
- check data type for attributes (you can still write "hello" to an integer attribute)
- complete port of Calligra to the generated code
- improved speed by using static QString instead of const char*
Provided solutions to previously raised issues:
- modular headers to reduce compile overhead
- function "end()" to optionally close an element before it goes out of scope
- use structure of Relax NG file to reduce header sizes by inheritance from common groups
- provide most KoXmlWriter functionality safely through the element instances
- closing elements is now automatic at a slight runtime overhead
[1] http://lists.kde.org/?t=130768700500002</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;">Opened several ppt and xls files.
Checked ppt conversion for two files and checked that XML was equivalent.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>devtools/CMakeLists.txt <span style="color: grey">(15008fb)</span></li>
<li>devtools/rng2cpp/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>devtools/rng2cpp/rng2cpp.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filters/libmso/CMakeLists.txt <span style="color: grey">(6bc145f)</span></li>
<li>filters/libmso/shapes.cpp <span style="color: grey">(073e061)</span></li>
<li>filters/libmso/shapes2.cpp <span style="color: grey">(0f0b906)</span></li>
<li>filters/sheets/excel/import/CMakeLists.txt <span style="color: grey">(2466218)</span></li>
<li>filters/sheets/excel/import/excelimporttoods.cc <span style="color: grey">(de788d4)</span></li>
<li>filters/stage/powerpoint/PptToOdp.h <span style="color: grey">(8d85c1f)</span></li>
<li>filters/stage/powerpoint/PptToOdp.cpp <span style="color: grey">(9258564)</span></li>
<li>libs/kotext/KoInlineNote.cpp <span style="color: grey">(6faa9a9)</span></li>
<li>libs/odf/CMakeLists.txt <span style="color: grey">(a2e3695)</span></li>
<li>libs/odf/writeodf/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/odf/writeodf/helpers.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/odf/writeodf/odfwriter.h <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111773/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>