Highlight Active Line makes repetitive up/down arrow navigation jerky.

[lkcampbell]

Spun off of discussion in issue #3191.

OS: Windows 7

Build: sprint 30 development build 0.30.0-0 (master 60dc6b7)

Background: This issue shares some similarities to issues #4862 and #4778, but it looks at repetitive up and down arrow navigation (i.e. holding the arrow key down, no selection involved) with, and without, Highlight Active Line enabled. Also, like issues #4862 and #4778, this issue appears to be new with Sprint 30. In Sprint 29, repetitive up/down arrow navigation is smooth in both cases.

Repro Steps:

1. Open src/editor/Editor.js in Brackets.
2. Make sure Highlight Active Line is on.
3. Goto line 1, then attempt to navigate to a higher line (say, 150 or 200) by holding down the down arrow key, causing repetitive down arrow key strokes.
4. As a comparison, turn off the Highlight Active Line setting and repeat step 3.

Observed Behavior:
Navigation with Highlight Active Line on is much more jumpy than when Highlight Active Line is off. It may not even be possible to accurate navigate to the higher line number in this manner.

Expected Behavior:
Navigation should be smooth in both cases. No difference between having Highlight Active Line turned on or off.

[jasonsanjose]

Congrats on issue 5000! 🍻

[larz0]

🎂

[lkcampbell]

Thanks!

I like to take a moment to thank issues 1-4999, without which none of this would have been possible...

[njx]

You should really be thanking us for writing all the buggy code that led to people filing those issues :)

[njx]

(And thanking yourself too, except I don't know if there have been any bugs in your code...)

[peterflynn]

Not to ruin the #5000 party or anything, but any reason to keep this open given that #4862 seems to be a superset? I think we can close this as a dupe...

[lkcampbell]

@peterflynn, issue #4862 is about the performance difference, when selecting text, between Sprint 29 and Sprint 30. This bug is about the performance difference between arrow navigation with Highlight Active Line on versus Highlight Active Line off.

To summarize...

There appear to be four main problems, which have been spread out a little unevenly among several issues. To add to the confusion, the discussions in these issues have weaved in and out of the different problems as well.

The first two problems are performance differences between Sprint 29 and Sprint 30 when looking at arrow selection, and arrow navigation, respectively. These problems are most likely caused by the CEF change going from Sprint 29 to Sprint 30, and are primarily presented and discussed together in issue #4862.

The second two problems are performance differences between Highlight Active Line being on and Highlight Active Line being off, when looking at arrow selection, and arrow navigation, respectively. The arrow selection problem is primarily presented and discussed in issue #4778. This issue should be addressed, at least indirectly, by pull request #4878. The arrow navigation problem is represented by this issue.

[njx]

Opening to @RaymondLim, low priority. It seems like this might be Win only since we're not seeing it on Mac.

[andremw]

@njx, I'm also experiencing this problem in Ubuntu 13.04. I got 30- FPS
The problem is that the clientWidth property is being retrieved many times inside findCachedMeasurement and it forces reflow.

These two links below explains it better.
http://developer.nokia.com/Community/Wiki/JavaScript_Performance_Best_Practices
http://www.stubbornella.org/content/2009/03/27/reflows-repaints-css-performance-making-your-javascript-slow/#comment-13157

[njx]

That's interesting. I found some related issues while trying to optimize scrolling, where style/layout info was being invalidated when it didn't need to be. In this case, it's a bit surprising that changing the active line highlight should invalidate the clientWidth, but there could be any number of reasons why that happens. Thanks for the clue!

[njx]

Bumping to medium priority since we've heard a number of reports of this (I believe).

[andremw]

Great! But just correct the tag.. It's not "Win only" anymore.

And with the shift key pressed (as mentioned in another issue) the FPS are much better.. I will take a look at it later and post here if I find something new.

[redmunds]

Removed Win-only tag.

[peterflynn]

There should be a noticeable performance improvement coming in our next CodeMirror update (Sprint 36, in 2014) due to codemirror/codemirror5#2007. Adding tracking tag.

[njx]

We've just had another report of Highlight Active Line seriously impacting performance: #6365. I'm going to nominate this for sprint 37.

[peterflynn]

@njx Sprint 36 will have the CM fix/improvement, so we should get everyone to test a newer build as soon as it's available. If it works well enough we might even be able to call this closed at that point.

[njx]

Oh, I didn't see your last comment.

[lkcampbell]

@njx and @peterflynn, I can already tell you that the performance on my Mac is much better than it was before the CM improvements. I could close this bug as fixed based on my Mac performance now. I see no difference in the repetitive navigation speed with having the highlight on versus having the highlight off. However, since I originally filed this issue on my older Windows 7 laptop, I should probably test it on that platform as well.

Since my Windows 7 machine no longer has a build environment on it, I will test an install of Sprint 36 on it when Sprint 36 becomes available.

[lkcampbell]

Also, even though it is not directly related to this bug, the keyboard navigation on my Mac is fairly smooth even with the Show Whitespace extension activated as well. That's pretty impressive because I consider the Show Whitespace extension to be the ultimate scrolling benchmark. So, yeah, it is definitely a lot better on my Mac.

[lkcampbell]

@RaymondLim, @njx, @peterflynn, I tested Sprint 36 on my new Mac, my wife's really old Windows machine, and the original Windows 7 laptop I was using when I filed this issue.

All three work well, they scroll smoothly with or without active line highlighted.

As far as I am concerned, Sprint 36 fixes this bug and I can close it.

Did you guys want to have some other people test it? I'm keeping it open just in case you want to test it more. Otherwise, close this bug as fixed.

[njx]

Sounds good. We did get a report from someone on Linux who still seems to be having this issue: #6837. But it's possible that's Linux-specific, so I'm good closing this one and just keeping that one open to investigate further.

Closing.