Review Request: Add a 10ms timer to scheduling a document for reparsing, and refactor -- fixes slowness when inserting text

[Sven Brauch]

> On Oct. 31, 2012, 7:55 a.m., Andreas Pakulat wrote:
> > 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.
> 
> Sven Brauch wrote:
>     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.

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.


- Sven


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107140/#review21190
-----------------------------------------------------------


On Oct. 30, 2012, 11:14 p.m., Sven Brauch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107140/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2012, 11:14 p.m.)
> 
> 
> Review request for KDevelop and Milian Wolff.
> 
> 
> Description
> -------
> 
>     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.
> 
> 
> Diffs
> -----
> 
>   language/backgroundparser/backgroundparser.cpp 28a4202 
>   language/backgroundparser/documentchangetracker.h e3f7f5c 
>   language/backgroundparser/documentchangetracker.cpp 2aab705 
> 
> Diff: http://git.reviewboard.kde.org/r/107140/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sven Brauch
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20121031/842d286c/attachment.html>