Cache outline headers & ref count outline provider #210213
Conversation
| this._disposables.add(_configurationService.onDidChangeConfiguration(e => { | ||
| if (e.affectsConfiguration(NotebookSetting.outlineShowMarkdownHeadersOnly) || | ||
| e.affectsConfiguration(NotebookSetting.outlineShowCodeCells) || | ||
| e.affectsConfiguration(NotebookSetting.outlineShowCodeCellSymbols) || | ||
| e.affectsConfiguration(NotebookSetting.breadcrumbsShowCodeCells) | ||
| ) { | ||
| this._recomputeState(); | ||
| delayedRecompute(); |
There was a problem hiding this comment.
Debouncing in outline provider resulted in significant improvements
From 60s to run a cell to 2-3s.
(this was already fixed in a previous PR, but this now adds the same debouncing for typing in editor).
When ever we make changes in editor (typing), we trigger outline changes synchronously
As a result, ever key stroke results in outline calculation.
With a large notebook (100 markdown cells and each markdown cell having 10-20 lines), then it takes around 50ms to calculate the outline.
This is ignoring code cells in outline.
Thus every key stroke results in a 50ms delay in renderer.
Solution = debounce
| getOrCreate(editor: INotebookEditor, target: OutlineTarget): IReference<NotebookCellOutlineProvider> | ||
| } | ||
|
|
||
| export class NotebookCellOutlineProviderFactory implements INotebookCellOutlineProviderFactory { |
There was a problem hiding this comment.
3 outline providers are created.
Meaning each key stroke in cell editors results in outlines being calculated 3 times.
Assume we take 50ms to calculate it, then we spend 50+50+50ms in total for each keystroke
Now it will be only one calculation.
There was a problem hiding this comment.
Did you actually see the provider for the goto symbols being calculated on each keystroke? I thought that one was only called when you open that widget
There was a problem hiding this comment.
Yes,
Open the widget, and every keystroke will result in outline being calculated (3 times)
There was a problem hiding this comment.
only called when you open that widget
Given the fact taht this is also used by other widgets, its possible it will re-compute it,
E.g. when the Sticky scroll feature is eanbled, we re-compute on every key stroke even if the outline widget is not visible.
| const fullContent = cell.getText().substring(0, 10000); | ||
| for (const { depth, text } of getMarkdownHeadersInCell(fullContent)) { | ||
| const headers = cache?.alternativeId === cell.getAlternativeId() ? cache.headers : Array.from(getMarkdownHeadersInCell(fullContent)); |
There was a problem hiding this comment.
This caching results in saving 50% of the time used to calculate outlines.
Down from 50ms to 26
I.e. this PR brings down the total time in keystokes from 150ms to 0 (when debounced) and when user stops typing, then we spend 20ms.
We should see some significant perf gains here.
| } | ||
| } | ||
|
|
||
| private init() { | ||
| this.notebookOutline.init(); |
There was a problem hiding this comment.
We no longer need to call init.
| if (!this.compareStickyLineMaps(recompute, this.currentStickyLines)) { | ||
| this.updateContent(recompute); | ||
| } | ||
| })); | ||
|
|
||
| this._disposables.add(this.notebookEditor.onDidAttachViewModel(() => { | ||
| this.notebookOutline.init(); |
There was a problem hiding this comment.
No more calling of init
There was a problem hiding this comment.
I fact init has been removed, it seemed to be a work around.
VS Code Stable (typing around 20 chars in quick succession)
VS Code OSS (typing around 40 chars in quick succession)The changes are not yet in VS Code insiders.
Duplicate event (before & after)
|
For #209787