Coloring exhibits quadratic behavior and blocks the SWT thread
Closed this issue · 15 comments
Opening a medium[1] sized JSON file in Eclipse might block the UI seemingly indefinitely due to AbstractModelLines.getOrNull which attempts query a linked list by index.
Relevant part of the stack:
"main" #1 [259] prio=6 os_prio=31 cpu=255346.03ms elapsed=871.90s tid=0x00007fadbe021400 nid=259 runnable [0x0000000205a14000]
java.lang.Thread.State: RUNNABLE
at java.util.LinkedList.node(java.base@20.0.1/LinkedList.java:575)
at java.util.LinkedList.get(java.base@20.0.1/LinkedList.java:481)
at org.eclipse.tm4e.core.model.AbstractModelLines.getOrNull(AbstractModelLines.java:149)
- locked <0x0000000769f00080> (a java.util.LinkedList)
at org.eclipse.tm4e.core.model.TMModel.getLineTokens(TMModel.java:284)
at org.eclipse.tm4e.ui.text.TMPresentationReconciler.colorize(TMPresentationReconciler.java:540)
at org.eclipse.tm4e.ui.text.TMPresentationReconciler$InternalListener.textChanged(TMPresentationReconciler.java:336)
at org.eclipse.jface.text.TextViewer.updateTextListeners(TextViewer.java:2785)
at org.eclipse.jface.text.TextViewer.invalidateTextPresentation(TextViewer.java:3405)
at org.eclipse.jface.text.TextViewer.initializeWidgetContents(TextViewer.java:3453)
at org.eclipse.jface.text.TextViewer.setVisibleDocument(TextViewer.java:3492)
[1] To quantify this: 800k lines, 22MB; we might call it a big JSON file.
It shall be noted that a quick glance over AbstractModelLines.java indicates that almost all of the usages of the internal linked list are index based. It might warrant a refactoring of the entire class to avoid performance penalty that is imposed on all methods with the current data structure.
What can be a good remediation? Just switching to ArrayList? Freel free to submit PRs for possible improvements.
I am investigating that.
Please try the latest snapshot from https://download.eclipse.org/tm4e/snapshots/ it contains the performance improvements I merged.
Issue has been solved in latest snapshot build (confirmed by other parties).
@sebthom I installed the latest nightly snapshot and still see a lot of time is spent here:
"main" #1 [259] prio=6 os_prio=31 cpu=141396.40ms elapsed=533.04s tid=0x00007fa2ed017000 nid=259 runnable [0x0000000209d21000]
java.lang.Thread.State: RUNNABLE
at org.eclipse.tm4e.core.model.AbstractModelLines.replaceLines(AbstractModelLines.java:118)
- locked <0x0000000758f6b1a0> (a java.util.ArrayList)
at org.eclipse.tm4e.ui.internal.model.DocumentModelLines.documentChanged(DocumentModelLines.java:72)
at org.eclipse.jface.text.AbstractDocument.doFireDocumentChanged2(AbstractDocument.java:748)
at org.eclipse.jface.text.AbstractDocument.doFireDocumentChanged(AbstractDocument.java:717)
at org.eclipse.jface.text.AbstractDocument.doFireDocumentChanged(AbstractDocument.java:701)
at org.eclipse.jface.text.AbstractDocument.fireDocumentChanged(AbstractDocument.java:775)
at org.eclipse.jface.text.AbstractDocument.replace(AbstractDocument.java:1100)
Steps:
- Open large file
- Select all
- Press Delete to empty the file
If you're patient enough, a subsequent Undo will do basically the same:
Because of #544 I am currently in the process of (probably) rewriting big parts of the model package. Once I got a handle on that either these issues are solved or I will have a second look at performance.
Anyways, you should already see considerable improvements in the performance using the snapshot build. can you confirm that?
Yes, I can confirm that opening editors for bigger jsons is a lot better. Editing larger chunks of data is still slow, though. Instead of doing the big rewrite, do you mind implementing the suggestion in #543 (comment) ?
The changes you suggested are not necessary in that form in my rewrite. The UI thread will just need to add the information about the number of line changed to a queue. The insertion of ModelLine instances into the lines ArrayList (which can be expensive for large files), will then occur asynchronously in the tokenizer thread.
@szarnekow please give the latest snapshot from download.eclipse.org/tm4e/snapshots a try.
As suggested by you I changed the list modifying code to use List.subList() for deleting multiple lines at once. See https://github.com/eclipse/tm4e/blob/9b03a29dd214f5390aaf4f9fd05ed40d3a7b13ba/org.eclipse.tm4e.core/src/main/java/org/eclipse/tm4e/core/model/TMModel.java#L343
@szarnekow please give the latest snapshot from download.eclipse.org/tm4e/snapshots a try.
I tried to, but the installation failed:
An error occurred while collecting items to be installed
session context was:(profile=<redacted>, phase=org.eclipse.equinox.internal.p2.engine.phases.Collect, operand=, action=).
Artifact not found: https://download.eclipse.org/tm4e/snapshots/plugins/org.eclipse.tm4e.ui_0.7.0.202306211001.jar.
https://download.eclipse.org/tm4e/snapshots/plugins/org.eclipse.tm4e.ui_0.7.0.202306211001.jar
Artifact not found: https://download.eclipse.org/tm4e/snapshots/features/org.eclipse.tm4e.feature_0.5.6.202306211001.jar.
https://download.eclipse.org/tm4e/snapshots/features/org.eclipse.tm4e.feature_0.5.6.202306211001.jar
Strange, well I bumped some versions and ran a rebuild. Can you try again please?
Strange, well I bumped some versions and ran a rebuild. Can you try again please?
@sebthom I managed to install the new version, but it looks like it's quite broken. Please watch the attached screen recording. Two notable things:
- functionally the highlighting is broken after edits
- non-functionally the latency is quite bad. I was continuously typing and you can see the cursor / editing actions to be extremely laggy. Note how the cursors doesn't even blink most of the time
Screen.Recording.2023-06-24.at.15.27.30.mov
@szarnekow thanks for taking the time to try out the release. I unfortunately cannot reproduce the issue. JSON editing works just fine for me even for large files such as https://github.com/json-iterator/test-data/blob/master/large-file.json.
Two things:
a) can you provide me that json file you are experiencing the rendering issue?
b) can you verify that you have these plugin versions installed:
@sebthom the versions here show qualifier without an actual qualifier, which means they are not built versions and thus most likely not the artifacts that are available in the snapshots repo but your local working copy of those bundles.
Ok, this are the released vesions. Editing works fine for me.
