Skip to content

Notebook dependent-cell diagnostic refresh is serialized at 500 ms per cell #1837

Description

Problem

An edit to one notebook cell can invalidate diagnostics in every other cell.
The changed cell is pulled immediately, but other affected cells enter the
generic background document scheduler, which waits 500 ms before each pull.

This means a notebook with 10 dependent cells can take approximately 5 seconds
to show correct diagnostics; 100 dependent cells can take approximately 50
seconds. There is no exponential timer in the implementation: the observed
problem is a fixed per-cell delay that accumulates linearly.

This is also an issue because it delays diagnostic pulls for other documents also (but, arguably, you're not supposed to import notebooks 😆).

Minimal reproduction

First enable the already-supported dependency capability in
client-node-tests/src/servers/fullNotebookServer.ts:

 diagnosticProvider: {
     identifier: 'diagnostic-provider',
     documentSelector: null,
-    interFileDependencies: false,
+    interFileDependencies: true,
     workspaceDiagnostics: false
 }

Then add this test to the existing suite('Full notebook tests', ...) in
client-node-tests/src/integration.test.ts:

test('Notebook dependent cells refresh without a per-cell delay', async () => {
  const dependentCellCount = 10;
  const notebook = await vscode.workspace.openNotebookDocument(
    'jupyter-notebook',
    new vscode.NotebookData([
      new vscode.NotebookCellData(vscode.NotebookCellKind.Code, 'value = 1', 'python'),
      ...Array.from({ length: dependentCellCount }, () =>
        new vscode.NotebookCellData(vscode.NotebookCellKind.Code, 'use(value)', 'python'),
      ),
    ]),
  );

  const initialPulls = new Set<string>();
  const dependentPullTimes = new Map<string, number>();
  let editStartedAt: number | undefined;

  client.middleware.provideDiagnostics = async (document, previousResultId, token, next) => {
    const uri = document instanceof vscode.Uri ? document : document.uri;
    const key = uri.toString();
    if (editStartedAt === undefined) {
      initialPulls.add(key);
    } else if (key !== notebook.cellAt(0).document.uri.toString()) {
      dependentPullTimes.set(key, Date.now() - editStartedAt);
    }
    return next(document, previousResultId, token);
  };

  try {
    await vscode.window.showNotebookDocument(notebook);

    for (let attempt = 0; attempt < 100; attempt++) {
      if (initialPulls.size === notebook.cellCount) {
        break;
      }
      await new Promise<void>((resolve) => setTimeout(resolve, 20));
    }
    assert.strictEqual(initialPulls.size, notebook.cellCount);

    editStartedAt = Date.now();
    const edit = new vscode.WorkspaceEdit();
    edit.insert(notebook.cellAt(0).document.uri, new vscode.Position(0, 0), '# change\n');
    assert.strictEqual(await vscode.workspace.applyEdit(edit), true);

    for (let attempt = 0; attempt < 150; attempt++) {
      if (dependentPullTimes.size === dependentCellCount) {
        break;
      }
      await new Promise<void>((resolve) => setTimeout(resolve, 50));
    }

    assert.strictEqual(
      dependentPullTimes.size,
      dependentCellCount,
      'Every dependent notebook cell should be refreshed',
    );

    const elapsed = Math.max(...dependentPullTimes.values());
    assert.ok(
      elapsed < 1000,
      `Refreshing ${dependentCellCount} dependent cells took ${elapsed} ms: ` +
        JSON.stringify([...dependentPullTimes.values()]),
    );
  } finally {
    client.middleware.provideDiagnostics = undefined;
    await revertAllDirty();
  }
}).timeout(10000);

Expected

After a cell edit, diagnostic pulls for other cells in the same notebook are
coalesced or scheduled promptly as a group.

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions