TabsTitleControl.layout() slows down editor resize

[alexdima]

• open a diff editor (side-by-side)
• resize via sash:
  

---

• now open two editors side-by-side
• resize via sash:
  

---

Most of the time is spent in two things:

• the find widget: filled Find widget's checkEditorWidth does synchronous layout on editor resize #34744
• TabsTitleControl.layout:

[alexdima]

Note: the time TabsTitleControl.layout takes is significantly larger when using the steps in #34716

i.e. 3K+ code lenses.

[bpasero]

@alexandrudima looks like most time is spend in this call:

I am happy for advice how to avoid that. The tab title control is a display:flex container that lays out all tabs. I have seen performance issues before at this location and it seems worse the more tabs are opened.

I need the offsetWidth to update the scrollbar properly that shows when tabs overflow.

Maybe one idea is to buffer the layout calls and not run them sync from the mouse move listener in the sash widget.

/cc @joaomoreno

[alexdima]

@bpasero I use various tricks to avoid sync forced layout:

• measuring unknown width/height things at creation time. i.e. if a new tab is added with a custom label, the tab width can be measured when the tab is created/added. If a tab is modified to have a different label, the width can be measured at the modification time.
• keeping track of known widths. If the workbench gets resized, the width of the tabs area can be saved in memory and the DOM would not need to be queried.

IMHO those two bullet points should cover the computation of scrollWidth (sum of all tabs width) and width (this is something the workbench knows)

[joaomoreno]

keeping track of known widths

Yeah, the width should actually only be measured at the top level of a layout() call chain. Every step down that chain should split that width up accordingly. The TabsTitleControl.layout() method should take a dimension as an argument. And its caller, EditorGroupsControl, should pass in the correct dimensions when calling layout, since he already knows those dimensions.

[bpasero]

The expensive part is indeed the call to scrollWidth where Chrome has to compute the size of each tab. Knowing the size of the container is not enough so it turns out to be a matter of caching the sizes for each tab when the tab changes.

[jrieken]

This is really bad. When opening/reloading the workbench with just one tab, ~50ms are spend. That makes it the most expensive single call after code loading and garbage collection.

title.cpuprofile.txt

[bpasero]

@jrieken interestingly as soon as I move that layout call into a setTimeout(0) the code is much faster. So just the fact that we force a layout in Chrome (by asking for tabsContainer.scrollWidth) during window startup causes a performance of 50ms vs. 0.xms in the other case...

[jrieken]

The timeline tool tells me that it's causing re-layouts. So with some setTimeout-tricks you might be lucky and get a layout-ed state but I believe the problem is some html-properties are accessed that cause the browser to relayout the whole page.

[jrieken]

Should we consider something like https://github.com/wilsonpage/fastdom?

[bpasero]

@jrieken I think fastdom is the equivalent of our scheduleAtNextAnimationFrame where you can batch operations to run on the next animation frame.

[bpasero]

Pushed 2 changes:

• for startup: scheduling the layout at next animation frame to avoid the critical path (f54d16c)
• found an issue where within the layout method I would force 2 full layouts because the way code was ordered (8142724)

The fix of avoiding a second layout call brings some nice performance boost for resizing of editors.

Previously

Now

The remaining performance issue with tabs is the fact that we access offsetWidth as well as scroll via scrollLeft. The only fix here is to:

• scroll via absolute positioning (negative left)
• remove flex and implement absolute pixel layout for the tabs
• implement custom auto-scrolling behaviour (e.g. when doing DND and reaching tabs that are scrolled away)

I am not a big fan of going away from flex, so I will stop here for now.

@jrieken I extracted the startup perf issue into #38404 so that we have something to close and verify for November. I will keep this issue open and restore it how it was before.

[jrieken]

I will keep this issue open and restore it how it was before.

@bpasero Did we make further improvements to this or why is this closed?

[bpasero]

Did not want to close this actually, the performance is pretty much the same as before grid land:

[bpasero]

Talked with Alex and he is 👍 to close this issue given the changes I have made so far, esp. c35e63f has helped in my measurement.

[alexdima]

Yes, the layout is a lot faster for me now: