[prev in list] [next in list] [prev in thread] [next in thread]
List: nepomuk
Subject: Re: [Nepomuk] Review Request: ClassAndPropertyTree: Optimize variantListToNodeSet
From: "Sebastian Trueg" <sebastian () trueg ! de>
Date: 2012-10-08 14:50:30
Message-ID: 20121008145030.13903.67519 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106740/#review20077
-----------------------------------------------------------
services/storage/classandpropertytree.cpp
<http://git.reviewboard.kde.org/r/106740/#comment15905>
no need to have this twice.
services/storage/classandpropertytree.cpp
<http://git.reviewboard.kde.org/r/106740/#comment15906>
see above.
services/storage/classandpropertytree.cpp
<http://git.reviewboard.kde.org/r/106740/#comment15907>
you could keep the layout here: first check if its rdfs:Literal, then r=
ange.isEmpty. Then the patch would be that much simpler to review. ;)
- Sebastian Trueg
On Oct. 5, 2012, 3:56 p.m., Vishesh Handa wrote:
> =
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106740/
> -----------------------------------------------------------
> =
> (Updated Oct. 5, 2012, 3:56 p.m.)
> =
> =
> Review request for Nepomuk and Sebastian Trueg.
> =
> =
> Description
> -------
> =
> The extra url comparsions take approximately 50% of the time spent in
> executing variantListToNodeSet. These comparisons are for special cas=
es
> and are done based on the property.
> =
> They can be done when the tree is being created, instead of each time
> this highly important function is called.
> =
> * Caching the literal type - QVariant::Type
> * Special handling for xsd:duration
> * Special handling for rdfs:Literal
> =
> =
> Diffs
> -----
> =
> services/storage/classandpropertytree.h 3e3174d =
> services/storage/classandpropertytree.cpp 1a337b4 =
> =
> Diff: http://git.reviewboard.kde.org/r/106740/diff/
> =
> =
> Testing
> -------
> =
> =
> Thanks,
> =
> Vishesh Handa
> =
>
[Attachment #5 (text/html)]
<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/106740/">http://git.reviewboard.kde.org/r/106740/</a>
</td>
</tr>
</table>
<br />
<div>
<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/106740/diff/1/?file=88500#file88500line226" \
style="color: black; font-weight: bold; text-decoration: \
underline;">services/storage/classandpropertytree.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; \
">Soprano::Node Nepomuk2::ClassAndPropertyTree::variantToNode(const QVariant \
&value, const QUrl &property) const</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">214</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="n">setError</span><span class="p">(</span> <span \
class="n">QString</span><span class="o">::</span><span \
class="n">fromLatin1</span><span class="p">(</span><span class="s">"Cannot set \
values for abstract property '%1'."</span><span \
class="p">)</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">no need to \
have this twice.</pre> </div>
<br />
<div>
<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/106740/diff/1/?file=88500#file88500line240" \
style="color: black; font-weight: bold; text-decoration: \
underline;">services/storage/classandpropertytree.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; \
">Soprano::Node Nepomuk2::ClassAndPropertyTree::variantToNode(const QVariant \
&value, const QUrl &property) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">216</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">range</span> <span class="o">=</span> <span class="n">XMLSchema</span><span \
class="o">::</span><span class="n">unsignedInt</span><span \
class="p">();</span></pre></td> <th bgcolor="#e9eaa8" style="border-left: 1px solid \
#C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">228</font></th> <td bgcolor="#fdfebc" width="50%"><pre style="font-size: \
8pt; line-height: 140%; margin: 0; "> <span class="n">setError</span><span \
class="p">(</span><span class="n">QString</span><span class="o">::</span><span \
class="n">fromLatin1</span><span class="p">(</span><span class="s">"Cannot set \
values for abstract property '%1'."</span><span class="p">).</span><span \
class="n">arg</span><span class="p">(</span><span class="n">property</span><span \
class="p">.</span><span class="n">toString</span><span class="p">()),</span> <span \
class="n">Soprano</span><span class="o">::</span><span class="n">Error</span><span \
class="o">::</span><span class="n">ErrorInvalidArgument</span><span \
class="p">);</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">see \
above.</pre> </div>
<br />
<div>
<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/106740/diff/1/?file=88500#file88500line257" \
style="color: black; font-weight: bold; text-decoration: \
underline;">services/storage/classandpropertytree.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; \
">Soprano::Node Nepomuk2::ClassAndPropertyTree::variantToNode(const QVariant \
&value, const QUrl &property) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">231</font></th> <td bgcolor="#ffc5ce" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="k">else</span> <span class="k">if</span><span class="p">(</span><span \
class="n">range</span><span class="p">.</span><span class="n">isEmpty</span><span \
class="p">())</span> <span class="p">{</span></pre></td> <th bgcolor="#ebb1ba" \
style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#ffc5ce" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">you could \
keep the layout here: first check if its rdfs:Literal, then range.isEmpty. Then the \
patch would be that much simpler to review. ;)</pre> </div>
<br />
<p>- Sebastian</p>
<br />
<p>On October 5th, 2012, 3:56 p.m., Vishesh Handa 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 Nepomuk and Sebastian Trueg.</div>
<div>By Vishesh Handa.</div>
<p style="color: grey;"><i>Updated Oct. 5, 2012, 3:56 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;"> The extra url comparsions take approximately 50% of the time spent \
in executing variantListToNodeSet. These comparisons are for special cases
and are done based on the property.
They can be done when the tree is being created, instead of each time
this highly important function is called.
* Caching the literal type - QVariant::Type
* Special handling for xsd:duration
* Special handling for rdfs:Literal
</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>services/storage/classandpropertytree.h <span style="color: \
grey">(3e3174d)</span></li>
<li>services/storage/classandpropertytree.cpp <span style="color: \
grey">(1a337b4)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106740/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
Nepomuk mailing list
Nepomuk@kde.org
https://mail.kde.org/mailman/listinfo/nepomuk
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic