Brackets temporarily hangs with maxed-out CPU when closing file with long lines

[peterflynn]

1. Open src/thirdparty/less-1.3.0.min.js
2. Press Ctrl+W

Result: Brackets hangs for about 30 sec. One CPU core is maxed out for the whole duration.

Expected: Although there are known performance issues on files with very long lines, closing one should be very inexpensive.

Tearing down a vanilla CodeMirror instance containing the same text content is nearly instantaneous, so it's likely that Brackets is doing something else to cause the slowdown.

It also seems a little crazy that it only takes 3 sec to create the editor with this text, yet somehow takes 10x longer to throw it away later.

[peterflynn]

I ran this through the Timelines view and it's not immediately clear what's going on. Native browser operations that occur after tossing out the old editor are just very sluggish for a few seconds. Most "Layout" operations take about 700 ms (it's actually a little suspicious how close in duration they all are). There are lots of parse/style operations too but those complete very quickly.

The GC events are fast, so it doesn't appear to be GC churn getting rid of all the old editor's JS state. But there might be native memory churn in a similar fashion that we're not seeing -- that could explain the native layout operations running slowly.

[peterflynn]

Fwiw, the various places that trigger a browser layout pass are:

• refresh(): fetch scrollbar.scrollHeight -- line 367
• note: lots of little parsing operations go on next: updateDisplay() -> patchDisplay() setting scratch.innerHTML (but they only add up to 100ms)
• updateDisplay(): fetch scroller.clientHeight -- line 1132
• updateGutter(): fetch gutter.offsetWidth -- line 1333
• measureLine(): fetch elt.offsetTop/Left -- line 1914 (called by updateDisplay() -> updateSelection())
• refresh(): fetch scrollbar.scrollHeight -- line 367 (again)
• another batch of little parsing operations
• updateDisplay(): fetch scroller.clientHeight -- line 1129
• updateGutter(): fetch gutter.offsetWidth -- line 1333 (again)
• updateVerticalScroll(): fetch scroller.clientHeight -- line 884
• measureLine(): fetch elt.offsetTop/Left -- line 1914 (called by updateDisplay() -> updateSelection()) (again)
• getScrollInfo(): fetch scroller.scrollLeft/Width & scrollbar.scrollTop/Height -- line 359 (called up _updateScrollerShadow())
• JSLint then runs, kicking off another editor resize -- a call to refresh() and a chain of little parsing operations, but no slow layout

Everything above takes about 8 seconds. After that, there's another 5 seconds of repaints interspersed with GC. They all look quick but there are big gaps in between where nothing happens, so it's possible the true cost of the GC events isn't being recorded right.

Note: there is one obvious optimization we could do here. EditorManager._doShow() calls Editor.setVisible(), which calls refresh(); _doShow() then explicitly calls refresh() again immediately afterward. That should be easy to fix.

[pthiess]

Reviewed, assigned to Peter.

[peterflynn]

Same problem occurs with brackets-shell too

[peterflynn]

The problem entirely goes away if I switch to an experimental branch where the editor isn't contained in a flex-box layout (e.g. pflynn/no-editor-flex-box or Alex's patch in pull #1007). The editor closes essentially instantly in that case.

[gruehle]

Moving to sprint 14.

[peterflynn]

Moving to Sprint 15 -- too risky/disruptive a change to squeeze in now.

[peterflynn]

Talked to Glenn & NJ and decided I'll try to get this landed after I'm back on 10/11 (so still Sprint 15).

The one open question is whether we should try the "new" flex-box syntax first. Should be cheap enough to at least verify whether it fixes this pathological case.

[redmunds]

FBNC to @peterflynn

[peterflynn]

Re-confirmed with latest (merged) version of fix. Closing.