From 4f4336468ce35a68e6d74c1959ceae87518da74f Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 9 Mar 2023 17:34:49 -0800 Subject: [PATCH 1/2] Request output rendering when idle in webview. --- .../browser/diff/notebookDiffEditor.ts | 2 +- .../notebook/browser/notebookEditorWidget.ts | 8 +- .../view/renderers/backLayerWebView.ts | 147 +++++++++++------- .../browser/view/renderers/webviewMessages.ts | 1 + .../browser/view/renderers/webviewPreloads.ts | 75 ++++++++- 5 files changed, 169 insertions(+), 64 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts index ff19fd8b69a7b..06d6049a2d1bb 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts @@ -869,7 +869,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD if (!activeWebview.insetMapping.has(output.source)) { const cellTop = this._list.getAbsoluteTopOfElement(cellDiffViewModel); - await activeWebview.createOutput({ diffElement: cellDiffViewModel, cellHandle: cellViewModel.handle, cellId: cellViewModel.id, cellUri: cellViewModel.uri }, output, cellTop, getOffset(), false); + await activeWebview.createOutput({ diffElement: cellDiffViewModel, cellHandle: cellViewModel.handle, cellId: cellViewModel.id, cellUri: cellViewModel.uri }, output, cellTop, getOffset()); } else { const cellTop = this._list.getAbsoluteTopOfElement(cellDiffViewModel); const outputIndex = cellViewModel.outputsViewModels.indexOf(output.source); diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 9d010e2ea133f..ed49fc8141c87 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -2664,13 +2664,17 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD if (!existingOutput || (!existingOutput.renderer && output.type === RenderOutputType.Extension) ) { - this._webview.createOutput({ cellId: cell.id, cellHandle: cell.handle, cellUri: cell.uri, executionId: cell.internalMetadata.executionId }, output, cellTop, offset, createWhenIdle); + if (createWhenIdle) { + this._webview.requestCreateOutputWhenWebviewIdle({ cellId: cell.id, cellHandle: cell.handle, cellUri: cell.uri, executionId: cell.internalMetadata.executionId }, output, cellTop, offset); + } else { + this._webview.createOutput({ cellId: cell.id, cellHandle: cell.handle, cellUri: cell.uri, executionId: cell.internalMetadata.executionId }, output, cellTop, offset); + } } else if (existingOutput.renderer && output.type === RenderOutputType.Extension && existingOutput.renderer.id !== output.renderer.id) { // switch mimetype this._webview.removeInsets([output.source]); - this._webview.createOutput({ cellId: cell.id, cellHandle: cell.handle, cellUri: cell.uri }, output, cellTop, offset, createWhenIdle); + this._webview.createOutput({ cellId: cell.id, cellHandle: cell.handle, cellUri: cell.uri }, output, cellTop, offset); } else { const outputIndex = cell.outputsViewModels.indexOf(output.source); const outputOffset = cell.getOutputOffset(outputIndex); diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index 5d618d692e45c..b4f7e881a31c3 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -6,7 +6,7 @@ import * as osPath from 'vs/base/common/path'; import { IMouseWheelEvent } from 'vs/base/browser/mouseEvent'; import { coalesce } from 'vs/base/common/arrays'; -import { DeferredPromise, runWhenIdle } from 'vs/base/common/async'; +import { DeferredPromise } from 'vs/base/common/async'; import { decodeBase64 } from 'vs/base/common/buffer'; import { Emitter, Event } from 'vs/base/common/event'; import { getExtensionForMimeType } from 'vs/base/common/mime'; @@ -126,6 +126,7 @@ export class BackLayerWebView extends Themable { element: HTMLElement; webview: IWebviewElement | undefined = undefined; insetMapping: Map> = new Map(); + pendingWebviewIdelInsetCreationRequest: Map> = new Map(); pendingInsetCreationRequest: Map = new Map(); readonly markupPreviewMapping = new Map(); private hiddenInsetMapping: Set = new Set(); @@ -1279,13 +1280,34 @@ export class BackLayerWebView extends Themable { } } - createOutput(cellInfo: T, content: IInsetRenderOutput, cellTop: number, offset: number, createWhenIdle: boolean): void { + requestCreateOutputWhenWebviewIdle(cellInfo: T, content: IInsetRenderOutput, cellTop: number, offset: number) { + if (this._disposed) { + return; + } + + if (this.insetMapping.has(content.source)) { + return; + } + + if (this.pendingWebviewIdelInsetCreationRequest.has(content.source)) { + return; + } + + const { message, renderer } = this._createOutputCreationMessage(cellInfo, content, cellTop, offset, true); + this._sendMessageToWebview(message); + this.pendingWebviewIdelInsetCreationRequest.set(content.source, { outputId: message.outputId, cellInfo: cellInfo, renderer, cachedCreation: message }); + } + + createOutput(cellInfo: T, content: IInsetRenderOutput, cellTop: number, offset: number): void { if (this._disposed) { return; } const cachedInset = this.insetMapping.get(content.source); + // we now request to render the output immediately, so we can remove the pending request + this.pendingWebviewIdelInsetCreationRequest.delete(content.source); + if (cachedInset && this._cachedInsetEqual(cachedInset, content)) { this.hiddenInsetMapping.delete(content.source); this._sendMessageToWebview({ @@ -1300,70 +1322,79 @@ export class BackLayerWebView extends Themable { // create new output const createOutput = () => { - const messageBase = { - type: 'html', - executionId: cellInfo.executionId, - cellId: cellInfo.cellId, - cellTop: cellTop, - outputOffset: offset, - left: 0, - requiredPreloads: [], - } as const; - - let message: ICreationRequestMessage; - let renderer: INotebookRendererInfo | undefined; - if (content.type === RenderOutputType.Extension) { - const output = content.source.model; - renderer = content.renderer; - const first = output.outputs.find(op => op.mime === content.mimeType)!; - - // TODO@jrieken - the message can contain "bytes" and those are transferable - // which improves IPC performance and therefore should be used. However, it does - // means that the bytes cannot be used here anymore - message = { - ...messageBase, - outputId: output.outputId, - rendererId: content.renderer.id, - content: { - type: RenderOutputType.Extension, - outputId: output.outputId, - metadata: output.metadata, - output: { - mime: first.mime, - valueBytes: first.data.buffer, - }, - allOutputs: output.outputs.map(output => ({ mime: output.mime })), - }, - }; - } else { - message = { - ...messageBase, - outputId: UUID.generateUuid(), - content: { - type: content.type, - htmlContent: content.htmlContent, - } - }; - } - + const { message, renderer } = this._createOutputCreationMessage(cellInfo, content, cellTop, offset, false); this._sendMessageToWebview(message); this.insetMapping.set(content.source, { outputId: message.outputId, cellInfo: cellInfo, renderer, cachedCreation: message }); this.hiddenInsetMapping.delete(content.source); this.reversedInsetMapping.set(message.outputId, content.source); }; - if (createWhenIdle) { - this.pendingInsetCreationRequest.get(content.source)?.dispose(); - this.pendingInsetCreationRequest.set(content.source, runWhenIdle(() => { - createOutput(); - this.pendingInsetCreationRequest.delete(content.source); - })); + // if (createWhenIdle) { + // this.pendingInsetCreationRequest.get(content.source)?.dispose(); + // this.pendingInsetCreationRequest.set(content.source, runWhenIdle(() => { + // createOutput(); + // this.pendingInsetCreationRequest.delete(content.source); + // })); + // } else { + // } + this.pendingInsetCreationRequest.get(content.source)?.dispose(); + this.pendingInsetCreationRequest.delete(content.source); + createOutput(); + + } + + private _createOutputCreationMessage(cellInfo: T, content: IInsetRenderOutput, cellTop: number, offset: number, createOnIdle: boolean): { readonly message: ICreationRequestMessage; readonly renderer: INotebookRendererInfo | undefined } { + const messageBase = { + type: 'html', + executionId: cellInfo.executionId, + cellId: cellInfo.cellId, + cellTop: cellTop, + outputOffset: offset, + left: 0, + requiredPreloads: [], + createOnIdle: createOnIdle + } as const; + + let message: ICreationRequestMessage; + let renderer: INotebookRendererInfo | undefined; + if (content.type === RenderOutputType.Extension) { + const output = content.source.model; + renderer = content.renderer; + const first = output.outputs.find(op => op.mime === content.mimeType)!; + + // TODO@jrieken - the message can contain "bytes" and those are transferable + // which improves IPC performance and therefore should be used. However, it does + // means that the bytes cannot be used here anymore + message = { + ...messageBase, + outputId: output.outputId, + rendererId: content.renderer.id, + content: { + type: RenderOutputType.Extension, + outputId: output.outputId, + metadata: output.metadata, + output: { + mime: first.mime, + valueBytes: first.data.buffer, + }, + allOutputs: output.outputs.map(output => ({ mime: output.mime })), + }, + }; } else { - this.pendingInsetCreationRequest.get(content.source)?.dispose(); - this.pendingInsetCreationRequest.delete(content.source); - createOutput(); + message = { + ...messageBase, + outputId: UUID.generateUuid(), + content: { + type: content.type, + htmlContent: content.htmlContent, + } + }; } + return { + message, + renderer + }; } updateOutput(cellInfo: T, content: IInsetRenderOutput, cellTop: number, offset: number): void { @@ -1372,7 +1403,7 @@ export class BackLayerWebView extends Themable { } if (!this.insetMapping.has(content.source)) { - this.createOutput(cellInfo, content, cellTop, offset, false); + this.createOutput(cellInfo, content, cellTop, offset); return; } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewMessages.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewMessages.ts index 9a81a4ea92d8f..a1d281dcd85e2 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewMessages.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewMessages.ts @@ -201,6 +201,7 @@ export interface ICreationRequestMessage { readonly initiallyHidden?: boolean; readonly rendererId?: string | undefined; readonly executionId?: string | undefined; + readonly createOnIdle: boolean; } export interface IContentWidgetTopRequest { diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts index ccdb610f50a81..d297d4ee77b37 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts @@ -80,6 +80,9 @@ interface PreloadContext { readonly isWorkspaceTrusted: boolean; } +declare function requestIdleCallback(callback: (args: IdleDeadline) => void, options?: { timeout: number }): number; +declare function cancelIdleCallback(handle: number): void; + declare function __import(path: string): Promise; async function webviewPreloads(ctx: PreloadContext) { @@ -96,6 +99,43 @@ async function webviewPreloads(ctx: PreloadContext) { delete (globalThis as any).acquireVsCodeApi; const tokenizationStyleElement = document.querySelector('style#vscode-tokenization-styles'); + const runWhenIdle: (callback: (idle: IdleDeadline) => void, timeout?: number) => IDisposable = (typeof requestIdleCallback !== 'function' || typeof cancelIdleCallback !== 'function') + ? (runner) => { + setTimeout(() => { + if (disposed) { + return; + } + const end = Date.now() + 15; // one frame at 64fps + runner(Object.freeze({ + didTimeout: true, + timeRemaining() { + return Math.max(0, end - Date.now()); + } + })); + }); + let disposed = false; + return { + dispose() { + if (disposed) { + return; + } + disposed = true; + } + }; + } + : (runner, timeout?) => { + const handle: number = requestIdleCallback(runner, typeof timeout === 'number' ? { timeout } : undefined); + let disposed = false; + return { + dispose() { + if (disposed) { + return; + } + disposed = true; + cancelIdleCallback(handle); + } + }; + }; const handleInnerClick = (event: MouseEvent) => { if (!event || !event.view || !event.view.document) { @@ -1145,9 +1185,17 @@ async function webviewPreloads(ctx: PreloadContext) { case 'html': { const data = event.data; - outputRunner.enqueue(data.outputId, signal => { - return viewModel.renderOutputCell(data, signal); - }); + if (data.createOnIdle) { + outputRunner.enqueueIdle(data.outputId, signal => { + // cancel the idle callback if it exists + return viewModel.renderOutputCell(data, signal); + }); + } else { + outputRunner.enqueue(data.outputId, signal => { + // cancel the idle callback if it exists + return viewModel.renderOutputCell(data, signal); + }); + } break; } case 'view-scroll': @@ -1491,6 +1539,9 @@ async function webviewPreloads(ctx: PreloadContext) { * ensuring that it's run in-order. */ public enqueue(outputId: string, action: (cancelSignal: AbortSignal) => unknown) { + this.pendingOutputCreationRequest.get(outputId)?.dispose(); + this.pendingOutputCreationRequest.delete(outputId); + const record = this.outputs.get(outputId); if (!record) { const controller = new AbortController(); @@ -1504,10 +1555,24 @@ async function webviewPreloads(ctx: PreloadContext) { } } + private pendingOutputCreationRequest: Map = new Map(); + + public enqueueIdle(outputId: string, action: (cancelSignal: AbortSignal) => unknown) { + this.pendingOutputCreationRequest.get(outputId)?.dispose(); + outputRunner.pendingOutputCreationRequest.set(outputId, runWhenIdle(() => { + outputRunner.enqueue(outputId, action); + outputRunner.pendingOutputCreationRequest.delete(outputId); + })); + } + /** * Cancels the rendering of all outputs. */ public cancelAll() { + // Delete all pending idle requests + this.pendingOutputCreationRequest.forEach(r => r.dispose()); + this.pendingOutputCreationRequest.clear(); + for (const { abort } of this.outputs.values()) { abort.abort(); } @@ -1518,6 +1583,10 @@ async function webviewPreloads(ctx: PreloadContext) { * Cancels any ongoing rendering out an output. */ public cancelOutput(outputId: string) { + // Delete the pending idle request if it exists + this.pendingOutputCreationRequest.get(outputId)?.dispose(); + this.pendingOutputCreationRequest.delete(outputId); + const output = this.outputs.get(outputId); if (output) { output.abort.abort(); From b1a7603e94801d5c12c487beb5e707fa558fa8b5 Mon Sep 17 00:00:00 2001 From: rebornix Date: Sun, 12 Mar 2023 17:38:15 -0700 Subject: [PATCH 2/2] Handle collapse and folding initial state properly for warmup. --- .../view/renderers/backLayerWebView.ts | 46 +++++++++++-------- .../browser/view/renderers/webviewPreloads.ts | 19 ++++++-- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index b4f7e881a31c3..7b7d21d663046 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -126,7 +126,9 @@ export class BackLayerWebView extends Themable { element: HTMLElement; webview: IWebviewElement | undefined = undefined; insetMapping: Map> = new Map(); - pendingWebviewIdelInsetCreationRequest: Map> = new Map(); + pendingWebviewIdleInsetCreationRequest: Map> = new Map(); + private reversedPendingWebviewIdleInsetMapping: Map = new Map(); + pendingInsetCreationRequest: Map = new Map(); readonly markupPreviewMapping = new Map(); private hiddenInsetMapping: Set = new Set(); @@ -580,6 +582,17 @@ export class BackLayerWebView extends Themable { const { cellInfo, output } = resolvedResult; this.notebookEditor.updateOutputHeight(cellInfo, output, height, !!update.init, 'webview#dimension'); this.notebookEditor.scheduleOutputHeightAck(cellInfo, update.id, height); + } else if (update.init) { + // might be idle render request's ack + const outputRequest = this.reversedPendingWebviewIdleInsetMapping.get(update.id); + if (outputRequest) { + const inset = this.pendingWebviewIdleInsetCreationRequest.get(outputRequest)!; + const cellInfo = inset.cellInfo; + this.reversedInsetMapping.set(update.id, outputRequest); + this.insetMapping.set(outputRequest, inset); + this.notebookEditor.updateOutputHeight(cellInfo, outputRequest, height, !!update.init, 'webview#dimension'); + this.notebookEditor.scheduleOutputHeightAck(cellInfo, update.id, height); + } } } else { this.notebookEditor.updateMarkupCellHeight(update.id, height, !!update.init); @@ -1289,13 +1302,14 @@ export class BackLayerWebView extends Themable { return; } - if (this.pendingWebviewIdelInsetCreationRequest.has(content.source)) { + if (this.pendingWebviewIdleInsetCreationRequest.has(content.source)) { return; } - const { message, renderer } = this._createOutputCreationMessage(cellInfo, content, cellTop, offset, true); + const { message, renderer } = this._createOutputCreationMessage(cellInfo, content, cellTop, offset, true, true); this._sendMessageToWebview(message); - this.pendingWebviewIdelInsetCreationRequest.set(content.source, { outputId: message.outputId, cellInfo: cellInfo, renderer, cachedCreation: message }); + this.pendingWebviewIdleInsetCreationRequest.set(content.source, { outputId: message.outputId, cellInfo: cellInfo, renderer, cachedCreation: message }); + this.reversedPendingWebviewIdleInsetMapping.set(message.outputId, content.source); } createOutput(cellInfo: T, content: IInsetRenderOutput, cellTop: number, offset: number): void { @@ -1306,7 +1320,10 @@ export class BackLayerWebView extends Themable { const cachedInset = this.insetMapping.get(content.source); // we now request to render the output immediately, so we can remove the pending request - this.pendingWebviewIdelInsetCreationRequest.delete(content.source); + this.pendingWebviewIdleInsetCreationRequest.delete(content.source); + if (cachedInset) { + this.reversedPendingWebviewIdleInsetMapping.delete(cachedInset.outputId); + } if (cachedInset && this._cachedInsetEqual(cachedInset, content)) { this.hiddenInsetMapping.delete(content.source); @@ -1322,28 +1339,17 @@ export class BackLayerWebView extends Themable { // create new output const createOutput = () => { - const { message, renderer } = this._createOutputCreationMessage(cellInfo, content, cellTop, offset, false); + const { message, renderer } = this._createOutputCreationMessage(cellInfo, content, cellTop, offset, false, false); this._sendMessageToWebview(message); this.insetMapping.set(content.source, { outputId: message.outputId, cellInfo: cellInfo, renderer, cachedCreation: message }); this.hiddenInsetMapping.delete(content.source); this.reversedInsetMapping.set(message.outputId, content.source); }; - // if (createWhenIdle) { - // this.pendingInsetCreationRequest.get(content.source)?.dispose(); - // this.pendingInsetCreationRequest.set(content.source, runWhenIdle(() => { - // createOutput(); - // this.pendingInsetCreationRequest.delete(content.source); - // })); - // } else { - // } - this.pendingInsetCreationRequest.get(content.source)?.dispose(); - this.pendingInsetCreationRequest.delete(content.source); createOutput(); - } - private _createOutputCreationMessage(cellInfo: T, content: IInsetRenderOutput, cellTop: number, offset: number, createOnIdle: boolean): { readonly message: ICreationRequestMessage; readonly renderer: INotebookRendererInfo | undefined } { + private _createOutputCreationMessage(cellInfo: T, content: IInsetRenderOutput, cellTop: number, offset: number, createOnIdle: boolean, initiallyHidden: boolean): { readonly message: ICreationRequestMessage; readonly renderer: INotebookRendererInfo | undefined } { const messageBase = { type: 'html', executionId: cellInfo.executionId, @@ -1379,6 +1385,7 @@ export class BackLayerWebView extends Themable { }, allOutputs: output.outputs.map(output => ({ mime: output.mime })), }, + initiallyHidden: initiallyHidden }; } else { message = { @@ -1387,7 +1394,8 @@ export class BackLayerWebView extends Themable { content: { type: content.type, htmlContent: content.htmlContent, - } + }, + initiallyHidden: initiallyHidden }; } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts index d297d4ee77b37..2f743f12cf6c2 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts @@ -2242,11 +2242,11 @@ async function webviewPreloads(ctx: PreloadContext) { public async renderOutputElement(data: webviewMessages.ICreationRequestMessage, preloadErrors: ReadonlyArray, signal: AbortSignal) { const startTime = Date.now(); - const outputElement = this.createOutputElement(data); + const outputElement /** outputNode */ = this.createOutputElement(data); await outputElement.render(data.content, data.rendererId, preloadErrors, signal); // don't hide until after this step so that the height is right - outputElement.element.style.visibility = data.initiallyHidden ? 'hidden' : ''; + outputElement/** outputNode */.element.style.visibility = data.initiallyHidden ? 'hidden' : ''; if (!!data.executionId && !!data.rendererId) { postNotebookMessage('notebookPerformanceMessage', { cellId: data.cellId, executionId: data.executionId, duration: Date.now() - startTime, rendererId: data.rendererId }); @@ -2295,7 +2295,16 @@ async function webviewPreloads(ctx: PreloadContext) { public updateScroll(request: webviewMessages.IContentWidgetTopRequest) { this.element.style.top = `${request.cellTop}px`; - this.outputElements.get(request.outputId)?.updateScroll(request.outputOffset); + const outputElement = this.outputElements.get(request.outputId); + if (outputElement) { + outputElement.updateScroll(request.outputOffset); + + if (request.forceDisplay && outputElement.outputNode) { + // TODO @rebornix @mjbvz, there is a misalignment here. + // We set output visibility on cell container, other than output container or output node itself. + outputElement.outputNode.element.style.visibility = ''; + } + } if (request.forceDisplay) { this.element.style.visibility = ''; @@ -2309,6 +2318,10 @@ async function webviewPreloads(ctx: PreloadContext) { private _outputNode?: OutputElement; + get outputNode() { + return this._outputNode; + } + constructor( private readonly outputId: string, ) {