Notebook Performance Improvements to Cell Editing/Output Changes/Execution Count Changes (#6867)

* edit perf

* Save multiline source in notebooks

* More merges

* Single, multi line works needs work

* Works with single + multi and recomputes active

* Actual perf improvements this time

* code cleanup

* Calculating output position on the fly

* Hmm can we use brackets to make this simpler?

* monday progress

* output working. lots of improvements.

* First tests working

* 10 tests now, fixed bugs

* Cleanup, add output test

* More fixes

* Need to still fix execution count bug

* Tests pass, added comments

* Cleanup

* PR comments round 1

* Deal with merge issues from master, layering

* Deleting duplicate file

* More PR Comments

* PR Comments
This commit is contained in:
Chris LaFreniere
2019-08-26 10:17:58 -07:00
committed by GitHub
parent 4afa282ef9
commit 84b3e876d7
19 changed files with 1157 additions and 73 deletions

View File

@@ -221,6 +221,7 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
this._register(this._editorInput);
this._register(this._editorModel.onDidChangeContent(e => {
this._editor.setHeightToScrollHeight();
this.cellModel.modelContentChangedEvent = e;
this.cellModel.source = this._editorModel.getValue();
this.onContentChanged.emit();
this.checkForLanguageMagics();

View File

@@ -30,7 +30,7 @@ export class CodeCellComponent extends CellView implements OnInit, OnChanges {
@HostListener('document:keydown.escape', ['$event'])
handleKeyboardEvent() {
this.cellModel.active = false;
this._model.activeCell = undefined;
this._model.updateActiveCell(undefined);
}
private _model: NotebookModel;

View File

@@ -16,7 +16,6 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti
import { ITextModelService } from 'vs/editor/common/services/resolverService';
import { INotebookModel, IContentManager, NotebookContentChange } from 'sql/workbench/parts/notebook/common/models/modelInterfaces';
import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel';
import { Range } from 'vs/editor/common/core/range';
import { UntitledEditorModel } from 'vs/workbench/common/editor/untitledEditorModel';
import { Schemas } from 'vs/base/common/network';
import { ITextFileService, ISaveOptions, StateChange } from 'vs/workbench/services/textfile/common/textfiles';
@@ -26,12 +25,17 @@ import { UntitledEditorInput } from 'vs/workbench/common/editor/untitledEditorIn
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
import { IDisposable } from 'vs/base/common/lifecycle';
import { NotebookChangeType } from 'sql/workbench/parts/notebook/common/models/contracts';
import { Deferred } from 'sql/base/common/promise';
import { NotebookTextFileModel } from 'sql/workbench/parts/notebook/common/models/notebookTextFileModel';
export type ModeViewSaveHandler = (handle: number) => Thenable<boolean>;
export class NotebookEditorModel extends EditorModel {
private dirty: boolean;
private _dirty: boolean;
private _changeEventsHookedUp: boolean = false;
private _notebookTextFileModel: NotebookTextFileModel = new NotebookTextFileModel();
private readonly _onDidChangeDirty: Emitter<void> = this._register(new Emitter<void>());
private _lastEditFullReplacement: boolean;
constructor(public readonly notebookUri: URI,
private textEditorModel: TextFileEditorModel | UntitledEditorModel,
@INotebookService private notebookService: INotebookService,
@@ -41,9 +45,17 @@ export class NotebookEditorModel extends EditorModel {
this._register(this.notebookService.onNotebookEditorAdd(notebook => {
if (notebook.id === this.notebookUri.toString()) {
// Hook to content change events
notebook.modelReady.then(() => {
this._register(notebook.model.kernelChanged(e => this.updateModel()));
this._register(notebook.model.contentChanged(e => this.updateModel(e)));
notebook.modelReady.then((model) => {
if (!this._changeEventsHookedUp) {
this._changeEventsHookedUp = true;
this._register(model.kernelChanged(e => this.updateModel(undefined, NotebookChangeType.KernelChanged)));
this._register(model.contentChanged(e => this.updateModel(e, e.changeType)));
this._register(notebook.model.onActiveCellChanged((cell) => {
if (cell) {
this._notebookTextFileModel.activeCellGuid = cell.cellGuid;
}
}));
}
}, err => undefined);
}
}));
@@ -58,7 +70,7 @@ export class NotebookEditorModel extends EditorModel {
}
}));
}
this.dirty = this.textEditorModel.isDirty();
this._dirty = this.textEditorModel.isDirty();
}
public get contentString(): string {
@@ -66,15 +78,19 @@ export class NotebookEditorModel extends EditorModel {
return model.getValue();
}
public get lastEditFullReplacement(): boolean {
return this._lastEditFullReplacement;
}
isDirty(): boolean {
return this.textEditorModel.isDirty();
}
public setDirty(dirty: boolean): void {
if (this.dirty === dirty) {
if (this._dirty === dirty) {
return;
}
this.dirty = dirty;
this._dirty = dirty;
this._onDidChangeDirty.fire();
}
@@ -92,7 +108,8 @@ export class NotebookEditorModel extends EditorModel {
}
}
public updateModel(contentChange?: NotebookContentChange): void {
public updateModel(contentChange?: NotebookContentChange, type?: NotebookChangeType): void {
this._lastEditFullReplacement = false;
if (contentChange && contentChange.changeType === NotebookChangeType.Saved) {
// We send the saved events out, so ignore. Otherwise we double-count this as a change
// and cause the text to be reapplied
@@ -104,22 +121,42 @@ export class NotebookEditorModel extends EditorModel {
// Request serialization so trusted state is preserved but don't update the model
this.sendNotebookSerializationStateChange();
} else {
// For all other changes, update the backing model with the latest contents
let notebookModel = this.getNotebookModel();
let editAppliedSuccessfully = false;
if (notebookModel && this.textEditorModel && this.textEditorModel.textEditorModel) {
let content = JSON.stringify(notebookModel.toJSON(), undefined, ' ');
let model = this.textEditorModel.textEditorModel;
let endLine = model.getLineCount();
let endCol = model.getLineMaxColumn(endLine);
this.textEditorModel.textEditorModel.applyEdits([{
range: new Range(1, 1, endLine, endCol),
text: content
}]);
if (contentChange && contentChange.cells && contentChange.cells[0]) {
if (type === NotebookChangeType.CellSourceUpdated) {
if (this._notebookTextFileModel.transformAndApplyEditForSourceUpdate(contentChange, this.textEditorModel)) {
editAppliedSuccessfully = true;
}
} else if (type === NotebookChangeType.CellOutputUpdated) {
if (this._notebookTextFileModel.transformAndApplyEditForOutputUpdate(contentChange, this.textEditorModel)) {
editAppliedSuccessfully = true;
}
} else if (type === NotebookChangeType.CellOutputCleared) {
if (this._notebookTextFileModel.transformAndApplyEditForClearOutput(contentChange, this.textEditorModel)) {
editAppliedSuccessfully = true;
}
} else if (type === NotebookChangeType.CellExecuted) {
if (this._notebookTextFileModel.transformAndApplyEditForCellUpdated(contentChange, this.textEditorModel)) {
editAppliedSuccessfully = true;
}
}
}
// If edit was already applied, skip replacing entire text model
if (editAppliedSuccessfully) {
return;
}
this.replaceEntireTextEditorModel(notebookModel, type);
this._lastEditFullReplacement = true;
}
}
}
public replaceEntireTextEditorModel(notebookModel: INotebookModel, type: NotebookChangeType) {
this._notebookTextFileModel.replaceEntireTextEditorModel(notebookModel, type, this.textEditorModel);
}
private sendNotebookSerializationStateChange(): void {
let notebookModel = this.getNotebookModel();
if (notebookModel) {
@@ -142,6 +179,10 @@ export class NotebookEditorModel extends EditorModel {
get onDidChangeDirty(): Event<void> {
return this._onDidChangeDirty.event;
}
get editorModel() {
return this.textEditorModel;
}
}
export class NotebookInput extends EditorInput {
@@ -161,6 +202,8 @@ export class NotebookInput extends EditorInput {
private _providersLoaded: Promise<void>;
private _dirtyListener: IDisposable;
private _notebookEditorOpenedTimestamp: number;
private _modelResolveInProgress: boolean = false;
private _modelResolved: Deferred<void> = new Deferred<void>();
constructor(private _title: string,
private resource: URI,
@@ -283,6 +326,12 @@ export class NotebookInput extends EditorInput {
}
async resolve(): Promise<NotebookEditorModel> {
if (!this._modelResolveInProgress) {
this._modelResolveInProgress = true;
} else {
await this._modelResolved;
return this._model;
}
if (this._model) {
return Promise.resolve(this._model);
} else {
@@ -296,6 +345,7 @@ export class NotebookInput extends EditorInput {
}
this._model = this.instantiationService.createInstance(NotebookEditorModel, this.resource, textOrUntitledEditorModel);
this.hookDirtyListener(this._model.onDidChangeDirty, () => this._onDidChangeDirty.fire());
this._modelResolved.resolve();
return this._model;
}
}

View File

@@ -176,14 +176,8 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
if (event) {
event.stopPropagation();
}
if (cell !== this.model.activeCell) {
if (this.model.activeCell) {
this.model.activeCell.active = false;
}
this._model.activeCell = cell;
this._model.activeCell.active = true;
this.detectChanges();
}
this.model.updateActiveCell(cell);
this.detectChanges();
}
//Saves scrollTop value on scroll change
@@ -192,10 +186,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
}
public unselectActiveCell() {
if (this.model && this.model.activeCell) {
this.model.activeCell.active = false;
this.model.activeCell = undefined;
}
this.model.updateActiveCell(undefined);
this.detectChanges();
}
@@ -311,10 +302,10 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
editorLoadedTimestamp: this._notebookParams.input.editorOpenedTimestamp
}, this.profile, this.logService, this.notificationService, this.telemetryService);
let trusted = await this.notebookService.isNotebookTrustCached(this._notebookParams.notebookUri, this.isDirty());
model.onError((errInfo: INotification) => this.handleModelError(errInfo));
model.contentChanged((change) => this.handleContentChanged(change));
model.onProviderIdChange((provider) => this.handleProviderIdChanged(provider));
model.kernelChanged((kernelArgs) => this.handleKernelChanged(kernelArgs));
this._register(model.onError((errInfo: INotification) => this.handleModelError(errInfo)));
this._register(model.contentChanged((change) => this.handleContentChanged()));
this._register(model.onProviderIdChange((provider) => this.handleProviderIdChanged(provider)));
this._register(model.kernelChanged((kernelArgs) => this.handleKernelChanged(kernelArgs)));
this._model = this._register(model);
await this._model.loadContents(trusted);
this.setLoading(false);
@@ -382,7 +373,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
this.notificationService.notify(notification);
}
private handleContentChanged(change: NotebookContentChange) {
private handleContentChanged() {
// Note: for now we just need to set dirty state and refresh the UI.
this.detectChanges();
}