<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/107140/">http://git.reviewboard.kde.org/r/107140/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 31st, 2012, 7:55 a.m., <b>Andreas Pakulat</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;">Can you explain a bit more why pasting big chunks is a problem in how the timer helps with that? The problem I'm seeing is that you're making some code asynchronous that was synchronous before, this can have nasty side-effects if there are unwritten assumptions about this elsewhere.</pre>
</blockquote>
<p>On October 31st, 2012, 12:34 p.m., <b>Sven Brauch</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;">The BackgroundParser::addDocument function is being called whenever the textChanged signal is emitted. This happens a large amount of times if you paste a chunk. Until now, addDocument added an entry to the parse plan whenever it was called, those were only cleared when the background parser actually triggered. The latter happened about 500ms later (the usual background-parser delay). For every call of addDocument, there were serveral places where all entries in the parse plan were scanned and compared. The whole thing thus had ~O(n²) complexity for a large amount of simultaneous insertions, which killed performance. With the suggested changes, there's a) significantly less calls to addDocument and b) no duplicate entries in the parse plan. They will thus reduce complexity to O(n), with a smaller constant factor even.
The asynchronous thing you mentioned is indeed something one might need to think about. However, I don't see a place where this would cause problems yet; because in the next step of the whole parse process, everything is made asynchronous anyways because of the backgroundparser delay. Maybe it would be necessary to check in the timer-called function whether the document has been closed or so? I'm not sure.</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;">Oh, if you're extremely worried about the timer thing we can just remove it and just keep the set change. It'll be slower, but still a major performance boost over the previous solution.</pre>
<br />
<p>- Sven</p>
<br />
<p>On October 30th, 2012, 11:14 p.m., Sven Brauch 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 KDevelop and Milian Wolff.</div>
<div>By Sven Brauch.</div>
<p style="color: grey;"><i>Updated Oct. 30, 2012, 11:14 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;"> Add a 10ms timer to scheduling a document for reparsing, and refactor.
This fixes extreme slowness when pasting large parts of text
into the editor.
Also, make DocumentParsePlan targets a set instead of a list.
I wanted to write a unit test benchmark which verifies this but it turns out to be difficult, since you need a KateTextDocument for inserting text, and it's not being installed (the .h file). I thus didn't do that yet.
You can easily test this manually by pasting > 3000 lines into the editor.</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>language/backgroundparser/backgroundparser.cpp <span style="color: grey">(28a4202)</span></li>
<li>language/backgroundparser/documentchangetracker.h <span style="color: grey">(e3f7f5c)</span></li>
<li>language/backgroundparser/documentchangetracker.cpp <span style="color: grey">(2aab705)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107140/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>