From @sbatten:

I investigated the perf of the new gridlayout. I can attribute at least 20ms difference, sometimes near measured 40ms, to the grid layout. There are several steps in the layout where its just 1ms or 2ms slower which can add a bit overall, this will be harder to shrink.

However, a large chunk of the slowdown is due the SerializableGrid.deserialize call. In this call, the grid adds and lays out views multiple times costing a lot of time. ~15ms difference in the analogous legacy version.

I tried an experiment where I deserialize the grid but make their layout functions noops until deserialization is complete and that alone cut 10ms off the time.

@joao do we need to call layout so much in deserialize?

From @jrieken:

Thanks for investigating!
Yeah, maybe something callback-based would help. The editor does that a lot, e.g. like changeViewZones taking a callback with does all modifications and once the callback is done the editor updates layout etc. Or something explicit, like startRestore matched by stopRestore

@sbatten

Upon investigating this, I noticed a SerializableGrid being constructed twice for the workbench... I found I'm always hitting this... which means the grid is never restored from serialization. You can't call getNeighborViews without ever calling layout, there's just no way that could work.

So:

• I've made getNeighborViews throw if called before layout. c429b41
• I've deleted that check code and I've changed the way how we can detect where views are in the grid. d0a5544

This was for sure responsible for most of the cost, since the grid would instantiate twice. Always.

I also like the idea of suspending layout calls until deserialization is complete, so I've also pushed this. For example, this reduced the status bar layout calls on startup from 15 to 5. 600c7db

@jrieken How can we get new numbers?

@jrieken How can we get new numbers?

You run and measure locally? For the perf bot: it runs twice a day (morning & afternoon) but only when there is new insiders available.

Yes I mean the perf bot, since the first post mentions an increase on that setup. Let's wait for tomorrow then.

@joaomoreno that code setting undefined was just checked in last night, so it may have been causing a new perf issue but I don't think it was the original issue.

@sbatten OK. Once I removed it I saw startup duration cut by almost half, so I assumed it was that. Let's wait for da numbas.

Perf machine reports ~1780ms which is roughly at the level of the first regression, confirming what @sbatten said here: #77856 (comment)

Moving to August since it's not impacting endgame.

@sbatten Great job! I can see, using breakpoints, that the editor is only called layout once. I think algorithmically, we're in a great spot. We can now start looking into bad decisions in terms of code patterns and DOM usage.

I found something. On my beefy machine, given a specific workbench size and layout, I can measure the CPU profile of these two calls taking roughly 50ms: bad.cpuprofile.txt

There's a lengthy layout call which is very suspicious. It turns out that there's a forced DOM reflow due to a417a06 which introduced changes to scrollTop and scrollLeft during layout! When I comment out those changes, I get that number down to 15ms instead: good.cpuprofile.txt

Since #78167 was pushed out to September, I'll just revert that fix, since it isn't super important anyway.

Yep, by reverting the changes from #78167, I can bring the average of workbench ready down by another ~30ms on my machine. Pushing it! 💪

@jrieken Closing this for now, let me know what the bot says tomorrow!

Not sure how to verify this. Startup was slow compared to old runs but @jrieken attributed that to Windows bloat.

💩Summary: BEST `1774ms`, VERSION `bffbe2ea304a6a4cff23388703b0963f1dadec36`, APP `Code - Insiders.exe`

SLOW 2136    Code - Insiders    bffbe2ea30    52dd11ff-31cc-447b-9249-ed2b032caef11567069843821    standard_start 
SLOW 1809    Code - Insiders    bffbe2ea30    c0e9031c-91d7-40a0-ad6d-837137f285e21567069890756    standard_start 
SLOW 1810    Code - Insiders    bffbe2ea30    af9ed370-7b31-4e46-abbc-5ecdfe2931fa1567069921039    standard_start 
SLOW 1813    Code - Insiders    bffbe2ea30    fc327391-064d-4c08-9cb7-b58359e51ab31567069977230    standard_start 
SLOW 1816    Code - Insiders    bffbe2ea30    8bcfb10e-9cc5-4cd8-919c-339affbc3eee1567070027415    standard_start 
SLOW 1788    Code - Insiders    bffbe2ea30    49bd75d6-c2a6-4531-95eb-c12c2e2f71721567070059584    standard_start 
SLOW 1830    Code - Insiders    bffbe2ea30    14aa9329-50f9-4d88-90db-940deb6253dc1567070089797    standard_start 
SLOW 1812    Code - Insiders    bffbe2ea30    4fd698e9-e468-40fe-b04c-f72cf534dc261567070123982    standard_start 
SLOW 1774    Code - Insiders    bffbe2ea30    dd43781a-f9ea-43a9-91bf-baab5bd51c781567070173151    standard_start 
SLOW 1808    Code - Insiders    bffbe2ea30    3f7fef8c-edf4-4a5f-b485-9915490ad28e1567070210408    standard_start

To verify, preferably with a clean install or with extensions disabled, use the grid layout setting to try both configurations for several runs. Compare startup perf using the command Developer: Startup Performance and look at overall workbench load and workbench ready times. Testing todays insiders averaged over 5 runs, I actually see us better than with grid disabled:

Here are my numbers, seems overall good:

grid

453 941
453 937
455 939
475 965
473 958

legacy 

441 930
478 966
438 928
440 924
441 930