highlighter: autostop co-routine when not needed

[jgmdev]

The highlighter creates a coroutine for incremental tokenization, but each time a document is reset, re-opened, a command view executed, etc... a new co-routine is added and the previous one never stopped. Found this issue while changing the rainbowparen.lua plugin, here a video with print debugging 😄, notice that at startup three co-routines for highlight are created for docs that don't even have name, some one else more familiarized with the related code should investigate it, also each time you close and open a file or commandview a new co-routine is created which isn't nice for the cpu usage:

co-leak.mp4

Don't know if this PR is the best solution, I don't see much gain on that incremental highlighter, maybe it is doing yield incorrectly since the highlighting on files which syntax has lots of patterns is taking too much to load and scroll, should be performed more lazily (inside the for loop instead of outside). Also disabling the incremental highlighter makes performance worse than original lite.

[jgmdev]

Don't know if this PR is the best solution, I don't see much gain on that incremental highlighter, maybe it is doing yield incorrectly since the highlighting on files which syntax has lots of patterns is taking too much to load and scroll, should be performed more lazily

to be more specific this issues can be noticed on mardown files that have long lines like the readme on lite-xl-plugins which have the table with really long lines. Maybe something needs to be yielded when tokenizing a single line or something related to sub syntaxes.

[jgmdev]

This change improves the responsiveness of this readme but there is something really wrong with long lines since even loading the file and saving takes some time....

diff --git a/data/core/doc/highlighter.lua b/data/core/doc/highlighter.lua
index c8d9d47..2ca5806 100644
--- a/data/core/doc/highlighter.lua
+++ b/data/core/doc/highlighter.lua
@@ -5,6 +5,9 @@ local tokenizer = require "core.tokenizer"
 local Object = require "core.object"
 
 
+local long_line = 255
+
+
 local Highlighter = Object:extend()
 
 
@@ -18,13 +21,12 @@ function Highlighter:new(doc)
 end
 
 
---- init incremental syntax highlighting
 function Highlighter:start()
   if self.running then return end
   self.running = true
   core.add_thread(function()
     while not self.exit do
-      
+
       -- for some strange reason some docs are created at startup without any
       -- name so we should also exit if doc has no name and not in core.docs
       if not self.doc.filename then
@@ -52,6 +54,10 @@ function Highlighter:start()
           if not (line and line.init_state == state and line.text == self.doc.lines[i]) then
             self.lines[i] = self:tokenize_line(i, state)
           end
+          if #self.doc.lines[i] > long_line then
+            core.redraw = true
+            coroutine.yield()
+          end
         end
 
         self.first_invalid_line = max + 1
@@ -116,9 +122,18 @@ end
 function Highlighter:get_line(idx)
   local line = self.lines[idx]
   if not line or line.text ~= self.doc.lines[idx] then
-    local prev = self.lines[idx - 1]
-    line = self:tokenize_line(idx, prev and prev.state)
-    self.lines[idx] = line
+    if #self.doc.lines[idx] > long_line then
+      line = {
+        tokens = {
+          "normal",
+          self.doc.lines[idx]
+        }
+      }
+    else
+      local prev = self.lines[idx - 1]
+      line = self:tokenize_line(idx, prev and prev.state)
+      self.lines[idx] = line
+    end
   end
   self.max_wanted_line = math.max(self.max_wanted_line, idx)
   return line

[jgmdev]

Note: After some profiling one of the performance degraders is detectindent which is also using the tokenizer at startup with the same syntax of the document which seems too much patterns for what we want to check, this should be simplified with a more basic and performant solution.

[jgmdev]

As an update to this issue, the problem was really apparent on luajit. But, after some discussion with Guldoman the circular weak ref used by the co-routine on Highlighter is properly collected by lua non jit version when the document is closed after a couple of seconds, but some plugins like lint+ that may have loose references to the doc will prevent garbage collection and leave the co-routine running until the document is re-opened which then looses the old references (maybe not) and creates new ones.

So with lua5.4 the major issue is plugins that not properly release references to a loaded doc, which would mean that opening documents and closing will leave these co-routines running. This could be negligible when the amount of plugins that could introduce such an issue is small otherwise it turns into a witch hunt when trying to fix the plugin culprit.

[franko]

May I ask you to adapt this fix for master-2.0 branch ? I may publish a 2.0.6 release with some minor fixes.

[jgmdev]

May I ask you to adapt this fix for master-2.0 branch ? I may publish a 2.0.6 release with some minor fixes.

Sorry that I haven't done anything on this, I did fixed the lint+ plugin that was causing the co-routine leakage, so at least when using lua 5.4 it should not happen at least with that plugin which so far seems to be the only one that presented the issue on non jit lua. Still, this fix would benefit the luajit target (if releasing luajit builds is still of interest) which doesn't properly handles weak references, but I wanted to first get #885 in and then rebase this PR in top of it and maybe have a better discussion about it, but surely this change would benefit the luajit branch.

[franko]

Thank you for your answer. Not everything is completely clear to me, I didn't know that luajit had problems with weak references.

I may do some investigations to see if for the master-2.0 branch with luajit there is a real coroutine leak.

[jgmdev]

I may do some investigations to see if for the master-2.0 branch with luajit there is a real coroutine leak.

Let me know how your testing goes

[jgmdev]

Turns out we really need this since there isn't an elegant way to stop the highlighter coroutine on elements that inherit from the Doc like the widgets TextBox and don't need the highlighter to run. Also command view is running a highlighter co-routine without any need, here a screenshots of all the unnecessary highlighter co-routines running when the settings gui plugin is active since it instantiates a couple of TextBoxes that uselessly kick the highlighter.

When applying on widgets TextBox the same logic as in CommandView with this PR applied the reduction of useless co-routines comes with a noticeable response on plugins that really need to run a co-routine like the LSP one.

[jgmdev]

After some discussion with @Guldoman he came with the idea of auto stopping the highlighter co-routine automatically when not needed which is actually a better solution on the resources/performance department since now having a bunch of documents open means that unnecessary highlighting co-routines will not be running on the background. So this solution should be an overall win,