Skip to content

Commit

Permalink
Speed up transfer of outputs to notebook webviews (#178719)
Browse files Browse the repository at this point in the history
Speed an transfer of outputs to notebook webviews

For microsoft/vscode-jupyter#11031

The VS Buffer for the output items has a short byte length but a very large backing buffer (`Uint8Array` which is actually a `Buffer`)

When sending outputs to the webview, we ended up transfering the huge backing buffer instead of the much smaller actual data

We can fix this by creating a Uint8Array copy of the data and then transfering it to the webview
  • Loading branch information
mjbvz authored Mar 30, 2023
1 parent b9202b6 commit 9fd77b4
Showing 1 changed file with 17 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1340,8 +1340,8 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable {
}

this.pendingWebviewIdleCreationRequest.set(content.source, runWhenIdle(() => {
const { message, renderer } = this._createOutputCreationMessage(cellInfo, content, cellTop, offset, true, true);
this._sendMessageToWebview(message);
const { message, renderer, transfer: transferable } = this._createOutputCreationMessage(cellInfo, content, cellTop, offset, true, true);
this._sendMessageToWebview(message, transferable);
this.pendingWebviewIdleInsetMapping.set(content.source, { outputId: message.outputId, cellInfo: cellInfo, renderer, cachedCreation: message });
this.reversedPendingWebviewIdleInsetMapping.set(message.outputId, content.source);
this.pendingWebviewIdleCreationRequest.delete(content.source);
Expand Down Expand Up @@ -1380,8 +1380,8 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable {

// create new output
const createOutput = () => {
const { message, renderer } = this._createOutputCreationMessage(cellInfo, content, cellTop, offset, false, false);
this._sendMessageToWebview(message);
const { message, renderer, transfer: transferable } = this._createOutputCreationMessage(cellInfo, content, cellTop, offset, false, false);
this._sendMessageToWebview(message, transferable);
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);
Expand All @@ -1390,7 +1390,7 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable {
createOutput();
}

private _createOutputCreationMessage(cellInfo: T, content: IInsetRenderOutput, cellTop: number, offset: number, createOnIdle: boolean, initiallyHidden: 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; transfer: readonly ArrayBuffer[] } {
const messageBase = {
type: 'html',
executionId: cellInfo.executionId,
Expand All @@ -1402,16 +1402,20 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable {
createOnIdle: createOnIdle
} as const;

const transfer: ArrayBuffer[] = [];

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

// Copy the underlying buffer so we only send over the data we need
const valueBytes = new Uint8Array(first.data.buffer);
transfer.push(valueBytes.buffer);

message = {
...messageBase,
outputId: output.outputId,
Expand All @@ -1422,7 +1426,7 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable {
metadata: output.metadata,
output: {
mime: first.mime,
valueBytes: first.data.buffer,
valueBytes: valueBytes,
},
allOutputs: output.outputs.map(output => ({ mime: output.mime })),
},
Expand All @@ -1442,7 +1446,8 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable {

return {
message,
renderer
renderer,
transfer,
};
}

Expand Down Expand Up @@ -1694,12 +1699,12 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable {
});
}

private _sendMessageToWebview(message: ToWebviewMessage) {
private _sendMessageToWebview(message: ToWebviewMessage, transfer?: readonly ArrayBuffer[]) {
if (this._disposed) {
return;
}

this.webview?.postMessage(message);
this.webview?.postMessage(message, transfer);
}

override dispose() {
Expand Down

0 comments on commit 9fd77b4

Please sign in to comment.