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

[core-ai-bot]

Issue by peterflynn
Thursday Aug 09, 2012 at 06:42 GMT
Originally opened as adobe/brackets#1354

---

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.

[core-ai-bot]

Comment by peterflynn
Thursday Aug 09, 2012 at 19:58 GMT

---

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.

[core-ai-bot]

Comment by peterflynn
Thursday Aug 09, 2012 at 22:23 GMT

---

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.

[core-ai-bot]

Comment by pthiess
Tuesday Aug 14, 2012 at 17:28 GMT

---

Reviewed, assigned to Peter.

[core-ai-bot]

Comment by peterflynn
Tuesday Aug 14, 2012 at 17:43 GMT

---

Same problem occurs with brackets-shell too

[core-ai-bot]

Comment by peterflynn
Thursday Aug 16, 2012 at 00:01 GMT

---

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.

[core-ai-bot]

Comment by gruehle
Tuesday Aug 28, 2012 at 17:58 GMT

---

Moving to sprint 14.

[core-ai-bot]

Comment by peterflynn
Friday Sep 07, 2012 at 19:01 GMT

---

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

[core-ai-bot]

Comment by peterflynn
Monday Oct 01, 2012 at 18:05 GMT

---

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.

[core-ai-bot]

Comment by redmunds
Wednesday Oct 17, 2012 at 00:51 GMT

---

FBNC to@peterflynn

[core-ai-bot]

Comment by peterflynn
Wednesday Oct 17, 2012 at 01:00 GMT

---

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