Only cal updateView() in visibleRange() when endPos() is invalid.

Authored by sars on Nov 29 2018, 8:26 PM.



visibleRange() has a side-effect that the visible range is updated every time the function is called. This is a big problem in createHighlights().

I created a dummy XML file that has an almost 4096 characters long line. I then used the search plugin to replace >< with >\n< to split the line.

Without the patch on my computer it took 1 min 30s to split the lines.
With this patch the time went down to 48s.

The time is still bad, but much better ;)

I wonder if the comment on line 3318 "//ensure that the view is up-to-date, otherwise 'endPos()' might fail!" warns about some corner case that I have not encountered...

Diff Detail

R39 KTextEditor
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
sars created this revision.Nov 29 2018, 8:26 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 29 2018, 8:26 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
sars requested review of this revision.Nov 29 2018, 8:26 PM
sars added a comment.Nov 29 2018, 8:44 PM

Note: this is especially bad when the "on the fly spellchecking" is enabled as visibleRange() is used to "optimize" and only highlight what is currently visible. It is called multiple times.

Hmm, in principle the change might make sense, to avoid un-needed recomputations.
Thought no idea how to see if that regresses something without just trying it.

sars updated this revision to Diff 46937.Dec 6 2018, 6:39 AM
sars edited the summary of this revision. (Show Details)

Remove unneeded temporary

sars added a comment.Dec 6 2018, 6:45 AM

OK, I have used this patch actively for a week now, and I have not noticed any regression... ;)

I also now removed the temporary as it did not provide noticeable improvements.

cullmann accepted this revision.Dec 6 2018, 8:28 AM

Then lets try that.

This revision is now accepted and ready to land.Dec 6 2018, 8:28 AM
This revision was automatically updated to reflect the committed changes.