Skip to content

Commit

Permalink
Avoid creating execution state listener for every cell. (#177025)
Browse files Browse the repository at this point in the history
  • Loading branch information
rebornix authored Mar 14, 2023
1 parent 7b81968 commit 93eaf7c
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,8 @@ export class CodeCell extends Disposable {
this.registerDecorations();
this.registerMouseListener();

this._register(notebookExecutionStateService.onDidChangeCellExecution(e => {
if (e.affectsCell(this.viewCell.uri)) {
this.cellParts.updateForExecutionState(this.viewCell, e);
}
this._register(Event.any(this.viewCell.onDidStartExecution, this.viewCell.onDidStopExecution)((e) => {
this.cellParts.updateForExecutionState(this.viewCell, e);
}));

this._register(this.viewCell.onDidChangeState(e => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,17 @@ import { NotebookOptionsChangeEvent } from 'vs/workbench/contrib/notebook/browse
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
import { BaseCellViewModel } from './baseCellViewModel';
import { NotebookLayoutInfo } from 'vs/workbench/contrib/notebook/browser/notebookViewEvents';
import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
import { ICellExecutionStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';

export class CodeCellViewModel extends BaseCellViewModel implements ICellViewModel {
readonly cellKind = CellKind.Code;

protected readonly _onLayoutInfoRead = this._register(new Emitter<void>());
readonly onLayoutInfoRead = this._onLayoutInfoRead.event;

protected readonly _onDidStartExecution = this._register(new Emitter<void>());
protected readonly _onDidStartExecution = this._register(new Emitter<ICellExecutionStateChangedEvent>());
readonly onDidStartExecution = this._onDidStartExecution.event;
protected readonly _onDidStopExecution = this._register(new Emitter<void>());
protected readonly _onDidStopExecution = this._register(new Emitter<ICellExecutionStateChangedEvent>());
readonly onDidStopExecution = this._onDidStopExecution.event;

protected readonly _onDidChangeOutputs = this._register(new Emitter<NotebookCellOutputsSplice>());
Expand Down Expand Up @@ -125,7 +125,6 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
readonly viewContext: ViewContext,
@IConfigurationService configurationService: IConfigurationService,
@INotebookService private readonly _notebookService: INotebookService,
@INotebookExecutionStateService private readonly _notebookExecutionStateService: INotebookExecutionStateService,
@ITextModelService modelService: ITextModelService,
@IUndoRedoService undoRedoService: IUndoRedoService,
@ICodeEditorService codeEditorService: ICodeEditorService
Expand Down Expand Up @@ -154,16 +153,6 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
dispose(removedOutputs);
}));

this._register(this._notebookExecutionStateService.onDidChangeCellExecution(e => {
if (e.affectsCell(model.uri)) {
if (e.changed) {
this._onDidStartExecution.fire();
} else {
this._onDidStopExecution.fire();
}
}
}));

this._outputCollection = new Array(this.model.outputs.length);

this._layoutInfo = {
Expand All @@ -187,6 +176,14 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
};
}

updateExecutionState(e: ICellExecutionStateChangedEvent) {
if (e.changed) {
this._onDidStartExecution.fire(e);
} else {
this._onDidStopExecution.fire(e);
}
}

updateOptions(e: NotebookOptionsChangeEvent) {
if (e.cellStatusBarVisibility || e.insertToolbarPosition || e.cellToolbarLocation) {
this.layoutChange({});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { CellKind, ICell, INotebookSearchOptions, ISelectionState, NotebookCells
import { cellIndexesToRanges, cellRangesToIndexes, ICellRange, reduceCellRanges } from 'vs/workbench/contrib/notebook/common/notebookRange';
import { NotebookLayoutInfo, NotebookMetadataChangedEvent } from 'vs/workbench/contrib/notebook/browser/notebookViewEvents';
import { CellFindMatchModel } from 'vs/workbench/contrib/notebook/browser/contrib/find/findModel';
import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';

const invalidFunc = () => { throw new Error(`Invalid change accessor`); };

Expand Down Expand Up @@ -196,6 +197,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD
@IBulkEditService private readonly _bulkEditService: IBulkEditService,
@IUndoRedoService private readonly _undoService: IUndoRedoService,
@ITextModelService private readonly _textModelService: ITextModelService,
@INotebookExecutionStateService notebookExecutionStateService: INotebookExecutionStateService,
) {
super();

Expand Down Expand Up @@ -318,6 +320,13 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD
}
}));

this._register(notebookExecutionStateService.onDidChangeCellExecution(e => {
const cell = this.getCellByHandle(e.cellHandle);

if (cell instanceof CodeCellViewModel) {
cell.updateExecutionState(e);
}
}));

this._register(this._selectionCollection.onDidChangeSelection(e => {
this._onDidChangeSelection.fire(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ suite('NotebookViewModel', () => {
let undoRedoService: IUndoRedoService;
let modelService: IModelService;
let languageService: ILanguageService;
let notebookExecutionStateService: INotebookExecutionStateService;

suiteSetup(() => {
disposables = new DisposableStore();
Expand All @@ -46,6 +47,7 @@ suite('NotebookViewModel', () => {
undoRedoService = instantiationService.get(IUndoRedoService);
modelService = instantiationService.get(IModelService);
languageService = instantiationService.get(ILanguageService);
notebookExecutionStateService = instantiationService.get(INotebookExecutionStateService);

instantiationService.stub(IConfigurationService, new TestConfigurationService());
instantiationService.stub(IThemeService, new TestThemeService());
Expand All @@ -57,7 +59,7 @@ suite('NotebookViewModel', () => {
const notebook = new NotebookTextModel('notebook', URI.parse('test'), [], {}, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false, cellContentMetadata: {} }, undoRedoService, modelService, languageService);
const model = new NotebookEditorTestModel(notebook);
const viewContext = new ViewContext(new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService)), new NotebookEventDispatcher(), () => ({} as IBaseCellEditorOptions));
const viewModel = new NotebookViewModel('notebook', model.notebook, viewContext, null, { isReadOnly: false }, instantiationService, bulkEditService, undoRedoService, textModelService);
const viewModel = new NotebookViewModel('notebook', model.notebook, viewContext, null, { isReadOnly: false }, instantiationService, bulkEditService, undoRedoService, textModelService, notebookExecutionStateService);
assert.strictEqual(viewModel.viewType, 'notebook');
});

Expand Down

0 comments on commit 93eaf7c

Please sign in to comment.