<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>
<p>On October 31st, 2012, 1:02 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;">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>
</blockquote>
<p>On October 31st, 2012, 2:58 p.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;">Do you know why pasting doing a single paste causes multiple textChanged signals? Does this maybe only happen when having Kate configured to reformat source code pasted from the clipboard? To me it seems illogical/wrong that textChanged is emitted more than once for a single change like pasting something and I'd rather try to fix that.</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;">No, I don't. And no, I have the indent code for pastes disabled, so it probably doesn't have anything to do with it...
And I agree that it seems weird... but code for, for example, code folding seems to rely on things like "for each line inserted, a "line inserted" signal is emitted".
Maybe we should ask the kate people about that? I'll see if I can reach anyone in IRC.
Anyways, the thing with the set should go in in any case, since it makes absolutely no sense that the parseplan contains two identical items.</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>