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

Milian Wolff mail at milianw.de
Thu Nov 1 13:59:26 UTC 2012


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


You do not instantiate a KateDocument, instead you ask the KTextEditor instance to create a document (of type KTextEditor::Document) for you. See e.g. our code completion tests in cpp for how to do that. Please do add a benchmark and share before/after numbers.

Furthermore, I dislike the 10ms timer. We might integrate it temporarily as a quick performance fix, but I would really like to see someone fixing the actual issues where they are created. This means at one hand KatePart shouldn't emit tons of such signals. Please write to kwrite-devel and ask there for their feedback.

I furthermore think that this whole 500ms delay in the background parser is wrongly placed. It should cover the cases where an open document gets changed all the time, i.e. the user is happily hacking away. Then we shouldn't trigger tons of parse jobs. But for all other use cases I can think of, namely adding project files to the background parser or also unit tests, we do not want the delay. So imo, we should rip out the delay from the background parser and add it to the place where we addDocument - according to your patch that would be the change tracker.

So, tl;dr; version: Write a benchmark and get the QSet patch into kdevplatform, then improve KatePart and/or refactor the background parser + change tracker.

Cheers

- Milian Wolff


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/20121101/0eb1e438/attachment.html>


More information about the KDevelop-devel mailing list