Bad performance on files with a long line - LinesYardstick::clientRectForRange

[gandm]

Prerequisites

• Can you reproduce the problem in safe mode?
• Are you running the latest version of Atom?
• Did you check the debugging guide?
• Did you check the FAQs on Discuss?
• Are you reporting to the correct repository?
• Did you perform a cursory search to see if your bug or enhancement is already reported?

For more information on how to write a good bug report or enhancement request, see the CONTRIBUTING guide.

Description

Files that have a long lines make the interace un responsive

Steps to Reproduce

As an example this test case of 9000 chars json line using plain text grammar and the code line at this gist repeatedly calls to LinesYardStick::clientRectForRange() with startIndex always 0 and the EndIndex starting at 0 moving incrementally over the range of the line. e.g. (0,1) (0,20) (0,21) (0,37)..... On my machine this takes 7 seconds to complete, during which time the interface is frozen! A line 3 times that length takes over a minute to unlock the interface O(n2)?

Expected behavior: [What you expected to happen]

Something sensible!

Actual behavior: [What actually happened]

Atom becomes unusable.

Versions

You can get this information from executing atom --version and apm --version at the command line. Also, please include the OS and what version of the OS you're running.

Atom : 1.10.2
Electron: 0.37.8
Chrome : 49.0.2623.75
Node : 5.10.0

apm 1.12.5
npm 3.10.5
node 4.4.5
python 2.7.9
git 2.8.1.windows.1
visual studio 2013

[winstliu]

Directly related to #979. /cc @nathansobo @as-cii

[nathansobo]

@as-cii Interesting, I wouldn't have thought we would measure at every point in the line, only the point where the cursor is. Any idea why this would happen?

[gandm]

@nathansobo @as-cii Actually, it's not every point, but certain points, it moves through the line at what appears to be a random ascending order. I can't quite see why the points are chosen because there aren't any DOM tags intervening. However, it does choose a large number of points to inspect with reference to the leftmost 0 index. At first I thought it was the spell checker underlining certain words but disabling it didn't have any effect.

[nathansobo]

We are forced to measure anywhere we absolutely position something. Currently that's cursors, highlights, and overlay decorations. We use a 0 index because we need to know how far right in pixel terms these locations are, which ends up being the width of the rect when you measure from 0 to the index in question. I'm not sure if it would be faster to measure a narrower area and just take its left position. That would require special handling at the end of the line because I'm not sure if zero-width regions work. There may be other reasons we didn't do it that way.

[gandm]

Well I disabled the spell checker, used the plain text grammar and copied the file and it still exhibits this behaviour. I can't see any markers or obvious overlays on the screen. And a DOM inspection just shows a single span with the data?

[gandm]

It does appear to be the Atom spell-checker causing this. However when disabling it, although the editors spell check markers aren't visible, the system still attempts to index at the old points. You have to disable it and either close the old editors and open a new editor panel or restart Atom.

The spell-checker doesn't appear to clean up after being disabled on existing documents. Even worse when a new line or a single character is added elsewhere in the doc the system seems to reindex all the lines giving the original slowdown, so it takes 7 seconds to enter a single character!

In my view both the spell-checker and Atoms re-indexing of pixel positions on lines that haven't changed need looking at. Maybe other packages also generate copious markers?

[as-cii]

Thanks for the investigation, @gandm! 👏 I have just opened atom/spell-check#159 to ensure all the markers get destroyed when disabling the spell-check package. There was also a subtle caching issue in our measurement code: I have fixed it in #12730 although admittedly it won't affect performance dramatically.

As for LinesYardstick being slow on long lines with lots of highlight decorations, with the current approach we are forced to measure all the points where there is an highlight decoration; also, whenever a change on that line occurs, we need to remeasure all the other points as well, because a single character could potentially affect the position of subsequent characters in non-trivial ways.

@nathansobo and I have discussed potential ways of fixing this and we think we might probably create and use a new API (which has been briefly discussed in #11787) that basically wraps misspelled text within other HTML tags (i.e. <span>s): this would allow us to not measure text at all for spell-check, thus letting the browser perform all the positioning on its own.

Another idea could be to keep the current approach, but avoid measuring text that's not currently visible on screen; although non-trivial, this would probably be a more holistic approach as it would solve the slowness caused by other packages that create lots of highlight decorations. We would add a fixed cost to search the first/last visible column (or we might just calculate an approximation via @baseCharacterWidth. This API looks promising too: https://developer.mozilla.org/en-US/docs/Web/API/Document/caretRangeFromPoint), and ignore measurements for all other ranges that affect only an invisible part of the screen.

Thoughts?

[as-cii]

Anyways, we are aware of the problem and we're planning to work on this as part of our ongoing and future efforts to improve performance in Atom. This may not happen in the immediate future because long lines are problematic also for other reasons and there are other performance improvements we need to make before addressing this.

This is definitely on our radar, though, and we will post updates on this issue as soon as we start working on it. Thanks again for the report, @gandm! 🙏

[gandm]

@as-cii Thanks for the speedy and thorough response. I haven't had time to think about this too much but hopefully I'll take another look.

From what I saw if a large number of markers appear on row 0 then, no matter where text is inserted or changed in the document, it is always slow. If the same line appears on row > 1, then insertions etc seem to work at full speed. I maybe wrong but I don't think you PR changes this behaviour.

Some issues I've had with long lines are where a regex in a grammar goes cause oniguruma to go into near infinite back tracking. I've also had some Git issues slowing the system. Others issues appear to have been fixed with 1.11 beta's.

Are the changes mentioned above going to be applied to a beta soon? If so I will take a look.

[as-cii]

From what I saw if a large number of markers appear on row 0 then, no matter where text is inserted or changed in the document, it is always slow. If the same line appears on row > 1, then insertions etc seem to work at full speed. I maybe wrong but I don't think you PR changes this behaviour.

Thanks for reporting this, @gandm! This will be fixed in #12745 and will go out in the next beta. As mentioned previously, there are still other factors contributing to slowness with long lines, but that pull-request should at least mitigate it when not editing such lines.

[lock]

This issue has been automatically locked since there has not been any recent activity after it was closed. If you can still reproduce this issue in Safe Mode then please open a new issue and fill out the entire issue template to ensure that we have enough information to address your issue. Thanks!