Skip to content

Commit 425a6de

Browse files
roblourensroblourens
authored
Fix notebook perf markers (microsoft#155266)
Fixes microsoft#135834
1 parent b40bbdd commit 425a6de

File tree

4 files changed

+26
-43
lines changed

4 files changed

+26
-43
lines changed

‎src/vs/workbench/contrib/notebook/browser/notebookEditor.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import { NotebooKernelActionViewItem } from 'vs/workbench/contrib/notebook/brows
3333
import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
3434
import { NOTEBOOK_EDITOR_ID } from 'vs/workbench/contrib/notebook/common/notebookCommon';
3535
import { NotebookEditorInput } from 'vs/workbench/contrib/notebook/common/notebookEditorInput';
36-
import { clearMarks, getAndClearMarks, mark } from 'vs/workbench/contrib/notebook/common/notebookPerformance';
36+
import { NotebookPerfMarks } from 'vs/workbench/contrib/notebook/common/notebookPerformance';
3737
import { IEditorDropService } from 'vs/workbench/services/editor/browser/editorDropService';
3838
import { GroupsOrder, IEditorGroup, IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
3939
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
@@ -172,8 +172,8 @@ export class NotebookEditor extends EditorPane implements IEditorPaneWithSelecti
172172

173173
override async setInput(input: NotebookEditorInput, options: INotebookEditorOptions | undefined, context: IEditorOpenContext, token: CancellationToken, noRetry?: boolean): Promise<void> {
174174
try {
175-
clearMarks(input.resource);
176-
mark(input.resource, 'startTime');
175+
const perf = new NotebookPerfMarks();
176+
perf.mark('startTime');
177177
const group = this.group!;
178178

179179
this._inputListener.value = input.onDidChangeCapabilities(() => this._onDidChangeInputCapabilities(input));
@@ -203,8 +203,8 @@ export class NotebookEditor extends EditorPane implements IEditorPaneWithSelecti
203203
// only now `setInput` and yield/await. this is AFTER the actual widget is ready. This is very important
204204
// so that others synchronously receive a notebook editor with the correct widget being set
205205
await super.setInput(input, options, context, token);
206-
const model = await input.resolve();
207-
mark(input.resource, 'inputLoaded');
206+
const model = await input.resolve(perf);
207+
perf.mark('inputLoaded');
208208

209209
// Check for cancellation
210210
if (token.isCancellationRequested) {
@@ -230,7 +230,7 @@ export class NotebookEditor extends EditorPane implements IEditorPaneWithSelecti
230230
const viewState = options?.viewState ?? this._loadNotebookEditorViewState(input);
231231

232232
this._widget.value?.setParentContextKeyService(this._contextKeyService);
233-
await this._widget.value!.setModel(model.notebook, viewState);
233+
await this._widget.value!.setModel(model.notebook, viewState, perf);
234234
const isReadOnly = input.hasCapability(EditorInputCapabilities.Readonly);
235235
await this._widget.value!.setOptions({ ...options, isReadOnly });
236236
this._widgetDisposableStore.add(this._widget.value!.onDidFocusWidget(() => this._onDidFocusWidget.fire()));
@@ -240,7 +240,7 @@ export class NotebookEditor extends EditorPane implements IEditorPaneWithSelecti
240240
containsGroup: (group) => this.group?.id === group.id
241241
}));
242242

243-
mark(input.resource, 'editorLoaded');
243+
perf.mark('editorLoaded');
244244

245245
type WorkbenchNotebookOpenClassification = {
246246
owner: 'rebornix';
@@ -266,8 +266,7 @@ export class NotebookEditor extends EditorPane implements IEditorPaneWithSelecti
266266
editorLoaded: number;
267267
};
268268

269-
const perfMarks = getAndClearMarks(input.resource);
270-
269+
const perfMarks = perf.value;
271270
if (perfMarks) {
272271
const startTime = perfMarks['startTime'];
273272
const extensionActivated = perfMarks['extensionActivated'];

‎src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,14 @@ import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/
7777
import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
7878
import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService';
7979
import { NotebookOptions, OutputInnerContainerTopPadding } from 'vs/workbench/contrib/notebook/common/notebookOptions';
80-
import { mark } from 'vs/workbench/contrib/notebook/common/notebookPerformance';
8180
import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange';
8281
import { INotebookRendererMessagingService } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService';
8382
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
8483
import { editorGutterModifiedBackground } from 'vs/workbench/contrib/scm/browser/dirtydiffDecorator';
8584
import { IWebview } from 'vs/workbench/contrib/webview/browser/webview';
8685
import { EditorExtensionsRegistry } from 'vs/editor/browser/editorExtensions';
8786
import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
87+
import { NotebookPerfMarks } from 'vs/workbench/contrib/notebook/common/notebookPerformance';
8888

8989
const $ = DOM.$;
9090

@@ -1080,12 +1080,12 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
10801080
this.scopedContextKeyService.updateParent(parentContextKeyService);
10811081
}
10821082

1083-
async setModel(textModel: NotebookTextModel, viewState: INotebookEditorViewState | undefined): Promise<void> {
1083+
async setModel(textModel: NotebookTextModel, viewState: INotebookEditorViewState | undefined, perf?: NotebookPerfMarks): Promise<void> {
10841084
if (this.viewModel === undefined || !this.viewModel.equal(textModel)) {
10851085
const oldTopInsertToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType);
10861086
const oldBottomToolbarDimensions = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType);
10871087
this._detachModel();
1088-
await this._attachModel(textModel, viewState);
1088+
await this._attachModel(textModel, viewState, perf);
10891089
const newTopInsertToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType);
10901090
const newBottomToolbarDimensions = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType);
10911091

@@ -1389,7 +1389,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
13891389
this._list.attachWebview(this._webview.element);
13901390
}
13911391

1392-
private async _attachModel(textModel: NotebookTextModel, viewState: INotebookEditorViewState | undefined) {
1392+
private async _attachModel(textModel: NotebookTextModel, viewState: INotebookEditorViewState | undefined, perf?: NotebookPerfMarks) {
13931393
await this._createWebview(this.getId(), textModel.uri);
13941394
this.viewModel = this.instantiationService.createInstance(NotebookViewModel, textModel.viewType, textModel, this._viewContext, this.getLayoutInfo(), { isReadOnly: this._readOnly });
13951395
this._viewContext.eventDispatcher.emit([new NotebookLayoutChangedEvent({ width: true, fontInfo: true }, this.getLayoutInfo())]);
@@ -1472,7 +1472,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
14721472
// init rendering
14731473
await this._warmupWithMarkdownRenderer(this.viewModel, viewState);
14741474

1475-
mark(textModel.uri, 'customMarkdownLoaded');
1475+
perf?.mark('customMarkdownLoaded');
14761476

14771477
// model attached
14781478
this._localCellStateListeners = this.viewModel.viewCells.map(cell => this._bindCellListener(cell));

‎src/vs/workbench/contrib/notebook/common/notebookEditorInput.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@ import { IDisposable, IReference } from 'vs/base/common/lifecycle';
1616
import { CellEditType, IResolvedNotebookEditorModel } from 'vs/workbench/contrib/notebook/common/notebookCommon';
1717
import { ILabelService } from 'vs/platform/label/common/label';
1818
import { Schemas } from 'vs/base/common/network';
19-
import { mark } from 'vs/workbench/contrib/notebook/common/notebookPerformance';
2019
import { FileSystemProviderCapabilities, IFileService } from 'vs/platform/files/common/files';
2120
import { AbstractResourceEditorInput } from 'vs/workbench/common/editor/resourceEditorInput';
2221
import { IResourceEditorInput } from 'vs/platform/editor/common/editor';
2322
import { onUnexpectedError } from 'vs/base/common/errors';
2423
import { VSBuffer } from 'vs/base/common/buffer';
2524
import { IWorkingCopyIdentifier } from 'vs/workbench/services/workingCopy/common/workingCopy';
2625
import { NotebookProviderInfo } from 'vs/workbench/contrib/notebook/common/notebookProvider';
26+
import { NotebookPerfMarks } from 'vs/workbench/contrib/notebook/common/notebookPerformance';
2727

2828
export interface NotebookEditorInputOptions {
2929
startDirty?: boolean;
@@ -231,12 +231,12 @@ export class NotebookEditorInput extends AbstractResourceEditorInput {
231231
}
232232
}
233233

234-
override async resolve(): Promise<IResolvedNotebookEditorModel | null> {
234+
override async resolve(perf?: NotebookPerfMarks): Promise<IResolvedNotebookEditorModel | null> {
235235
if (!await this._notebookService.canResolve(this.viewType)) {
236236
return null;
237237
}
238238

239-
mark(this.resource, 'extensionActivated');
239+
perf?.mark('extensionActivated');
240240

241241
// we are now loading the notebook and don't need to listen to
242242
// "other" loading anymore

‎src/vs/workbench/contrib/notebook/common/notebookPerformance.ts

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,39 +3,23 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { URI } from 'vs/base/common/uri';
7-
86
export type PerfName = 'startTime' | 'extensionActivated' | 'inputLoaded' | 'webviewCommLoaded' | 'customMarkdownLoaded' | 'editorLoaded';
97

108
type PerformanceMark = { [key in PerfName]?: number };
119

12-
const perfMarks = new Map<string, PerformanceMark>();
10+
export class NotebookPerfMarks {
11+
private _marks: PerformanceMark = {};
12+
13+
get value(): PerformanceMark {
14+
return { ...this._marks };
15+
}
1316

14-
export function mark(resource: URI, name: PerfName): void {
15-
const key = resource.toString();
16-
if (!perfMarks.has(key)) {
17-
const perfMark: PerformanceMark = {};
18-
perfMark[name] = Date.now();
19-
perfMarks.set(key, perfMark);
20-
} else {
21-
if (perfMarks.get(key)![name]) {
17+
mark(name: PerfName): void {
18+
if (this._marks[name]) {
2219
console.error(`Skipping overwrite of notebook perf value: ${name}`);
2320
return;
2421
}
25-
perfMarks.get(key)![name] = Date.now();
26-
}
27-
}
2822

29-
export function clearMarks(resource: URI): void {
30-
const key = resource.toString();
31-
32-
perfMarks.delete(key);
33-
}
34-
35-
export function getAndClearMarks(resource: URI): PerformanceMark | null {
36-
const key = resource.toString();
37-
38-
const perfMark = perfMarks.get(key) || null;
39-
perfMarks.delete(key);
40-
return perfMark;
23+
this._marks[name] = Date.now();
24+
}
4125
}

0 commit comments

Comments
 (0)