Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't clear execution order immediately when execution starts #155357

Merged
merged 2 commits into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ class ExecutionStateCellStatusBarItem extends Disposable {

private _currentItemIds: string[] = [];

private _currentExecutingStateTimer: IDisposable | undefined;
private _showedExecutingStateTime: number | undefined;
private _clearExecutingStateTimer: IDisposable | undefined;

constructor(
private readonly _notebookViewModel: INotebookViewModel,
Expand Down Expand Up @@ -123,23 +124,27 @@ class ExecutionStateCellStatusBarItem extends Disposable {
*/
private _getItemsForCell(): INotebookCellStatusBarItem[] | undefined {
const runState = this._executionStateService.getCellExecution(this._cell.uri);
if (this._currentExecutingStateTimer && !runState?.isPaused) {
return;
}

const item = this._getItemForState(runState, this._cell.internalMetadata);

// Show the execution spinner for a minimum time
if (runState?.state === NotebookCellExecutionState.Executing) {
this._currentExecutingStateTimer = this._register(disposableTimeout(() => {
const runState = this._executionStateService.getCellExecution(this._cell.uri);
this._currentExecutingStateTimer = undefined;
if (runState?.state !== NotebookCellExecutionState.Executing) {
this._update();
if (runState?.state === NotebookCellExecutionState.Executing && typeof this._showedExecutingStateTime !== 'number') {
this._showedExecutingStateTime = Date.now();
} else if (runState?.state !== NotebookCellExecutionState.Executing && typeof this._showedExecutingStateTime === 'number') {
const timeUntilMin = ExecutionStateCellStatusBarItem.MIN_SPINNER_TIME - (Date.now() - this._showedExecutingStateTime);
if (timeUntilMin > 0) {
if (!this._clearExecutingStateTimer) {
this._clearExecutingStateTimer = disposableTimeout(() => {
this._showedExecutingStateTime = undefined;
this._clearExecutingStateTimer = undefined;
this._update();
}, timeUntilMin);
}
}, ExecutionStateCellStatusBarItem.MIN_SPINNER_TIME));

return undefined;
} else {
this._showedExecutingStateTime = undefined;
}
}

const item = this._getItemForState(runState, this._cell.internalMetadata);
return item ? [item] : [];
}

Expand Down Expand Up @@ -203,12 +208,16 @@ export class TimerCellStatusBarContrib extends Disposable implements INotebookEd
}
registerNotebookContribution(TimerCellStatusBarContrib.id, TimerCellStatusBarContrib);

const UPDATE_TIMER_GRACE_PERIOD = 200;

class TimerCellStatusBarItem extends Disposable {
private static UPDATE_INTERVAL = 100;
private _currentItemIds: string[] = [];

private _scheduler: RunOnceScheduler;

private _deferredUpdate: IDisposable | undefined;

constructor(
private readonly _notebookViewModel: INotebookViewModel,
private readonly _cell: ICellViewModel,
Expand Down Expand Up @@ -243,7 +252,18 @@ class TimerCellStatusBarItem extends Disposable {
}

const items = item ? [item] : [];
this._currentItemIds = this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this._cell.handle, items }]);
if (!items.length && !!runState) {
if (!this._deferredUpdate) {
this._deferredUpdate = disposableTimeout(() => {
this._deferredUpdate = undefined;
this._currentItemIds = this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this._cell.handle, items }]);
}, UPDATE_TIMER_GRACE_PERIOD);
}
} else {
this._deferredUpdate?.dispose();
this._deferredUpdate = undefined;
this._currentItemIds = this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this._cell.handle, items }]);
}
}

private _getTimeItem(startTime: number, endTime: number, adjustment: number = 0): INotebookCellStatusBarItem {
Expand All @@ -258,6 +278,7 @@ class TimerCellStatusBarItem extends Disposable {
override dispose() {
super.dispose();

this._deferredUpdate?.dispose();
this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this._cell.handle, items: [] }]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo
if (!exe) {
exe = this._createNotebookCellExecution(notebook, cellHandle);
notebookExecutionMap.set(cellHandle, exe);
exe.initialize();
this._onDidChangeCellExecution.fire(new NotebookExecutionEvent(notebookUri, cellHandle, exe));
}

Expand Down Expand Up @@ -305,6 +306,9 @@ class CellExecution extends Disposable implements INotebookCellExecution {
) {
super();
this._logService.debug(`CellExecution#ctor ${this.getCellLog()}`);
}

initialize() {
const startExecuteEdit: ICellEditOperation = {
editType: CellEditType.PartialInternalMetadata,
handle: this.cellHandle,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,23 @@
*--------------------------------------------------------------------------------------------*/

import * as DOM from 'vs/base/browser/dom';
import { disposableTimeout } from 'vs/base/common/async';
import { DisposableStore } from 'vs/base/common/lifecycle';
import { ICellViewModel, INotebookEditorDelegate } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { CellViewModelStateChangeEvent } from 'vs/workbench/contrib/notebook/browser/notebookViewEvents';
import { CellPart } from 'vs/workbench/contrib/notebook/browser/view/cellPart';
import { NotebookCellInternalMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';

const UPDATE_EXECUTION_ORDER_GRACE_PERIOD = 200;

export class CellExecutionPart extends CellPart {
private kernelDisposables = this._register(new DisposableStore());

constructor(
private readonly _notebookEditor: INotebookEditorDelegate,
private readonly _executionOrderLabel: HTMLElement
private readonly _executionOrderLabel: HTMLElement,
@INotebookExecutionStateService private readonly _notebookExecutionStateService: INotebookExecutionStateService
) {
super();

Expand All @@ -37,11 +42,22 @@ export class CellExecutionPart extends CellPart {
}

protected override didRenderCell(element: ICellViewModel): void {
this.updateExecutionOrder(element.internalMetadata);
this.updateExecutionOrder(element.internalMetadata, true);
}

private updateExecutionOrder(internalMetadata: NotebookCellInternalMetadata): void {
private updateExecutionOrder(internalMetadata: NotebookCellInternalMetadata, forceClear = false): void {
if (this._notebookEditor.activeKernel?.implementsExecutionOrder) {
// If the executionOrder was just cleared, and the cell is executing, wait just a bit before clearing the view to avoid flashing
if (typeof internalMetadata.executionOrder !== 'number' && !forceClear && !!this._notebookExecutionStateService.getCellExecution(this.currentCell!.uri)) {
const renderingCell = this.currentCell;
this.cellDisposables.add(disposableTimeout(() => {
if (this.currentCell === renderingCell) {
this.updateExecutionOrder(this.currentCell!.internalMetadata, true);
}
}, UPDATE_EXECUTION_ORDER_GRACE_PERIOD));
return;
}

const executionOrderLabel = typeof internalMetadata.executionOrder === 'number' ?
`[${internalMetadata.executionOrder}]` :
'[ ]';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende
templateDisposables.add(scopedInstaService.createInstance(RunToolbar, this.notebookEditor, contextKeyService, container, runButtonContainer)),
templateDisposables.add(new CellDecorations(rootContainer, decorationContainer)),
templateDisposables.add(scopedInstaService.createInstance(CellComments, this.notebookEditor, cellCommentPartContainer)),
templateDisposables.add(new CellExecutionPart(this.notebookEditor, executionOrderLabel)),
templateDisposables.add(scopedInstaService.createInstance(CellExecutionPart, this.notebookEditor, executionOrderLabel)),
templateDisposables.add(scopedInstaService.createInstance(CollapsedCellOutput, this.notebookEditor, cellOutputCollapsedContainer)),
templateDisposables.add(new CollapsedCellInput(this.notebookEditor, cellInputCollapsedContainer)),
templateDisposables.add(new CellFocusPart(container, focusSinkElement, this.notebookEditor)),
Expand Down