Notebook slow scrolling caused by update layer tree

[rebornix]

Originally reported by @digitarald

Expensive layout is the cuplrit, even warned as layout shift so content really seems to move on every frame while scrolling – cased by a wheel event handler calling setScrollPositionNow.

After digging into the perf log, it seems there is a long update layer tree (~10ms) for every scroll.

[rebornix]

@digitarald can you try below when you get a chance to reproduce the issue

• run vscode with --disable-gpu and see if you can reproduce
• check if you have a large code cell (with a lot of lines), if so, try to split the cell to smaller cells and see if you still see the slowness

[claudiaregio]

+1

[claudiaregio]

+1

[alceballosa]

Hi @claudiaregio , I tried making a new notebook (using the disable-gpu flag) with only a few cells of code and one markdown cell, and the behavior is the same. Are there any other options I can test? To me it seems that it's not slow at all, only that the sensitivity of the mouse scroll goes up for those cells.

Thank you!

[rebornix]

@alceballosa do you have editor.mouseWheelScrollSensitivity for the text editor?

[alceballosa]

Hi @rebornix , that's right.

[rebornix]

Did quite some experiments with scrolling last week, there are multiple ways to reduce the creation time of a cell or reducing the browser rendering time. Having all the experiments doesn't necessarily lead to better perceived user experience so we would need to fine tune each improvement and cherry-pick them to main branch one by one. The experiments and findings:

• Fewer operations on cell view init creation
  • We used to register event listeners on cell view model. It will trigger false event listener mem leak alert (which prints call stack) and cause significant slow down. Better way to handle this is registering a single listener on NotebookViewModel and then redirect the events to each cell.
• Reduce forced layout
  • Previously when we render a cell output, we always check its offsetHeight first and then register a resize observer on the DOM element. The offsetHeight check is used to generate an init height but it's actually not necessary as resize observer can help with that can avoid forced layout by a ton. It does lead to a layout change on each cell but it's better than forced reflow.
  • If we receive the height change from resize observer before the list view's dynamic height check, we can probably populate the new height to list view other than relayout the cell in next frame
• Reduced layout shift
  • use transform instead of setting top for cell components (cell toolbar, focus indicator, status bar, etc)
  • we disabled transform on the list view as it will cut off the editor widgets (see Fix widget in cell editor cutoff by the titlebar. #94195). To re-enable this, we would need overflowWidgetsDomNode and onDidBlurEditorWidget don't work together #105346 to be properly implemented first.
• Reduce monaco editor creation
  • Instead of caching monaco editors on list view templates, we can have a dedicated DOM for all monaco editors and reuse them on cell rendering.
• Move costly operations to web worker
  • When model changes, we will recalculate outline, which is currently computed in main thread, it will block the UI when the document is large.

Perf log before and after the experiments

[roblourens]

I don't understand how the resize observer avoids a forced layout, wouldn't it have to do that to know the size? Is the benefit just that we get to delay that a little bit?

[rebornix]

I don't understand how the resize observer avoids a forced layout, wouldn't it have to do that to know the size? Is the benefit just that we get to delay that a little bit?

The resize observer doesn't avoid forced layout, but stop checking offsetHeight does. The assumption here is resize observer will give us the init height.

[rebornix]

Reduced forced reflow after changes in #133571

[rebornix]

Let's continue the following work in #117644