ENB: An old performance bug in Leo's colorizer

33 views
Skip to first unread message

Edward K. Ream

unread,
Nov 4, 2024, 3:37:51 AM11/4/24
to leo-editor

This Engineering Notebook post discusses possible fixes for a long-standing performance bug in Leo's syntax coloring code. This bug increases in severity the more lines p.b contains.


However, nobody seems to have noticed! I only discovered this bug while working on PR #4145. Nevertheless, I shall fix this bug next. It's infuriating to have so much good work partially ruined.


Background


The JEditColorizer (jedit) class drives the QSyntaxHighlighter (qsh) class. The qsh maintains an integer state for each line of text, with -1 meaning no state. The jedit class uses a string state internally, converting to and from int states as needed.


There is a ton of good code in the jedit class and discovering the bug is good news. I foresee that Leo's syntax coloring will soon become super fast!


The bug


The qsh class calls jedit.recolor(s) to recolor a single line. The qsh coordinates with the QTextBrowser class to optimize these calls.


The qsh should usually call jedit.recolor(s) only once per typed character. For continued strings and comments, the qsh should call recolor(s) only until the string or comment closes. Instead, the botch causes the qsh to recolor every line of the text on every keystroke. Ouch!


Enhanced states


The current code requests a full redraw from the qsh class in several situations. This policy is not the cause of the bug. But perhaps this policy is misguided. It may be possible to use incremental coloring at all times, even when changing nodes or cutting or pasting text!


Recall that the jedit class uses string states internally. These string states have been an unqualified success. They encode the language in effect and the qsh's restart state. Such restart states keep track of comments and strings the span lines. The so-called restarter pattern matchers maintain the restart state. The details are hairy. Happily, the details don't matter here.


Could we enhance string states by adding the line itself to the state? The increased size doesn't matter to Python. Hmm. The number of states will greatly increase. Suddenly we have to worry about overflowing ints.


In any event, let's consider such enhanced states as a thought experiment. Perhaps we can get the same effect without creating huge numbers of states.


Now you can see where I am going. Using enhanced states, the jedit class might never need to request a full redraw from the qsh. The states themselves would tell the qsh when to do a full recolor!


Goals


- Create internal checks to ensure the bug never happens again!

- Simplify the calling sequences. Now is the time to clean the code!

- Use enhanced states to avoid all full redraws.


Summary


Leo's syntax-coloring code has long suffered a performance bug that degrades the performance of large text.


A smallish fix might be possible, but I have a bigger ambition, namely to avoid all full redraws, even when changing nodes or cutting or pasting text.


Edward

Edward K. Ream

unread,
Nov 4, 2024, 7:52:12 AM11/4/24
to leo-e...@googlegroups.com
On Mon, Nov 4, 2024 at 2:37 AM Edward K. Ream <edre...@gmail.com> wrote:

Goals


- Create internal checks to ensure the bug never happens again!

- Simplify the calling sequences. Now is the time to clean the code!

- Use enhanced states to avoid all full redraws.


My next steps:

- Completely diagnose the bug.
- Find a minimal solution, cleaning code only as necessary.
- Insert checking code. This code will be a form of continuous unit testing.

Only after these steps will I consider more "interesting" ideas.

Edward

Edward K. Ream

unread,
Nov 4, 2024, 5:21:28 PM11/4/24
to leo-editor
On Monday, November 4, 2024 at 6:52:12 AM UTC-6 Edward K. Ream wrote:

My next steps:
- Completely diagnose the bug.
- Find a minimal solution, cleaning code only as necessary.

 Done in PR #4147. See that PR for details.

The changes were simple enough so that there is no need to consider enhanced states. Hurray!

Edward
Reply all
Reply to author
Forward
0 new messages