From e633f3be9df6ada79f6a2f108e14c42434fa6e7b Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Fri, 7 Jul 2017 14:42:06 +0200 Subject: [PATCH] Do not sync large models to web workers (#30180) --- .../editor/browser/widget/diffEditorWidget.ts | 34 ++++++++++++- .../common/services/editorWorkerService.ts | 6 +++ .../services/editorWorkerServiceImpl.ts | 50 +++++++++++++++++-- .../inPlaceReplace/common/inPlaceReplace.ts | 18 ++++--- .../browser/standaloneCodeEditor.ts | 6 ++- .../standalone/browser/standaloneEditor.ts | 4 +- .../electron-browser/dirtydiffDecorator.ts | 4 ++ 7 files changed, 108 insertions(+), 14 deletions(-) diff --git a/src/vs/editor/browser/widget/diffEditorWidget.ts b/src/vs/editor/browser/widget/diffEditorWidget.ts index 99d25e8a316f3..3a28537e6d966 100644 --- a/src/vs/editor/browser/widget/diffEditorWidget.ts +++ b/src/vs/editor/browser/widget/diffEditorWidget.ts @@ -6,10 +6,12 @@ 'use strict'; import 'vs/css!./media/diffEditor'; +import * as nls from 'vs/nls'; import { RunOnceScheduler } from 'vs/base/common/async'; import { Disposable } from 'vs/base/common/lifecycle'; import * as objects from 'vs/base/common/objects'; import * as dom from 'vs/base/browser/dom'; +import Severity from 'vs/base/common/severity'; import { FastDomNode, createFastDomNode } from 'vs/base/browser/fastDomNode'; import { ISashEvent, IVerticalSashLayoutProvider, Sash } from 'vs/base/browser/ui/sash/sash'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; @@ -38,6 +40,8 @@ import { OverviewRulerZone } from 'vs/editor/common/view/overviewZoneManager'; import { IEditorWhitespace } from 'vs/editor/common/viewLayout/whitespaceComputer'; import { ModelDecorationOptions } from 'vs/editor/common/model/textModelWithDecorations'; import { DiffReview } from 'vs/editor/browser/widget/diffReview'; +import URI from 'vs/base/common/uri'; +import { IMessageService } from "vs/platform/message/common/message"; interface IEditorDiffDecorations { decorations: editorCommon.IModelDeltaDecoration[]; @@ -186,6 +190,7 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE protected _contextKeyService: IContextKeyService; private _codeEditorService: ICodeEditorService; private _themeService: IThemeService; + private readonly _messageService: IMessageService; private _reviewPane: DiffReview; @@ -196,7 +201,8 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE @IContextKeyService contextKeyService: IContextKeyService, @IInstantiationService instantiationService: IInstantiationService, @ICodeEditorService codeEditorService: ICodeEditorService, - @IThemeService themeService: IThemeService + @IThemeService themeService: IThemeService, + @IMessageService messageService: IMessageService ) { super(); @@ -205,6 +211,7 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE this._contextKeyService = contextKeyService.createScoped(domElement); this._contextKeyService.createKey('isInDiffEditor', true); this._themeService = themeService; + this._messageService = messageService; this.id = (++DIFF_EDITOR_ID); @@ -826,6 +833,19 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE this._beginUpdateDecorationsTimeout = window.setTimeout(() => this._beginUpdateDecorations(), DiffEditorWidget.UPDATE_DIFF_DECORATIONS_DELAY); } + private _lastOriginalWarning: URI = null; + private _lastModifiedWarning: URI = null; + + private static _equals(a: URI, b: URI): boolean { + if (!a && !b) { + return true; + } + if (!a || !b) { + return false; + } + return (a.toString() === b.toString()); + } + private _beginUpdateDecorations(): void { this._beginUpdateDecorationsTimeout = -1; const currentOriginalModel = this.originalEditor.getModel(); @@ -840,6 +860,18 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE this._diffComputationToken++; let currentToken = this._diffComputationToken; + if (!this._editorWorkerService.canComputeDiff(currentOriginalModel.uri, currentModifiedModel.uri)) { + if ( + !DiffEditorWidget._equals(currentOriginalModel.uri, this._lastOriginalWarning) + || !DiffEditorWidget._equals(currentModifiedModel.uri, this._lastModifiedWarning) + ) { + this._lastOriginalWarning = currentOriginalModel.uri; + this._lastModifiedWarning = currentModifiedModel.uri; + this._messageService.show(Severity.Warning, nls.localize("diff.tooLarge", "Cannot compare files because one file is too large.")); + } + return; + } + this._editorWorkerService.computeDiff(currentOriginalModel.uri, currentModifiedModel.uri, this._ignoreTrimWhitespace).then((result) => { if (currentToken === this._diffComputationToken && currentOriginalModel === this.originalEditor.getModel() diff --git a/src/vs/editor/common/services/editorWorkerService.ts b/src/vs/editor/common/services/editorWorkerService.ts index f2cef5565fd13..430f7b9d1b67f 100644 --- a/src/vs/editor/common/services/editorWorkerService.ts +++ b/src/vs/editor/common/services/editorWorkerService.ts @@ -17,8 +17,14 @@ export var IEditorWorkerService = createDecorator(ID_EDITO export interface IEditorWorkerService { _serviceBrand: any; + canComputeDiff(original: URI, modified: URI): boolean; computeDiff(original: URI, modified: URI, ignoreTrimWhitespace: boolean): TPromise; + + canComputeDirtyDiff(original: URI, modified: URI): boolean; computeDirtyDiff(original: URI, modified: URI, ignoreTrimWhitespace: boolean): TPromise; + computeMoreMinimalEdits(resource: URI, edits: TextEdit[], ranges: IRange[]): TPromise; + + canNavigateValueSet(resource: URI): boolean; navigateValueSet(resource: URI, range: IRange, up: boolean): TPromise; } diff --git a/src/vs/editor/common/services/editorWorkerServiceImpl.ts b/src/vs/editor/common/services/editorWorkerServiceImpl.ts index 62401d3df919b..830c9ea99cdca 100644 --- a/src/vs/editor/common/services/editorWorkerServiceImpl.ts +++ b/src/vs/editor/common/services/editorWorkerServiceImpl.ts @@ -32,9 +32,21 @@ const STOP_SYNC_MODEL_DELTA_TIME_MS = 60 * 1000; */ const STOP_WORKER_DELTA_TIME_MS = 5 * 60 * 1000; +function canSyncModel(modelService: IModelService, resource: URI): boolean { + let model = modelService.getModel(resource); + if (!model) { + return false; + } + if (model.isTooLargeForTokenization()) { + return false; + } + return true; +} + export class EditorWorkerServiceImpl extends Disposable implements IEditorWorkerService { public _serviceBrand: any; + private readonly _modelService: IModelService; private readonly _workerManager: WorkerManager; constructor( @@ -43,25 +55,37 @@ export class EditorWorkerServiceImpl extends Disposable implements IEditorWorker @IModeService modeService: IModeService ) { super(); - this._workerManager = this._register(new WorkerManager(modelService)); + this._modelService = modelService; + this._workerManager = this._register(new WorkerManager(this._modelService)); // todo@joh make sure this happens only once this._register(modes.LinkProviderRegistry.register('*', { provideLinks: (model, token) => { + if (!canSyncModel(this._modelService, model.uri)) { + return TPromise.as([]); // File too large + } return wireCancellationToken(token, this._workerManager.withWorker().then(client => client.computeLinks(model.uri))); } })); - this._register(modes.SuggestRegistry.register('*', new WordBasedCompletionItemProvider(this._workerManager, configurationService, modeService))); + this._register(modes.SuggestRegistry.register('*', new WordBasedCompletionItemProvider(this._workerManager, configurationService, modeService, this._modelService))); } public dispose(): void { super.dispose(); } + public canComputeDiff(original: URI, modified: URI): boolean { + return (canSyncModel(this._modelService, original) && canSyncModel(this._modelService, modified)); + } + public computeDiff(original: URI, modified: URI, ignoreTrimWhitespace: boolean): TPromise { return this._workerManager.withWorker().then(client => client.computeDiff(original, modified, ignoreTrimWhitespace)); } + public canComputeDirtyDiff(original: URI, modified: URI): boolean { + return (canSyncModel(this._modelService, original) && canSyncModel(this._modelService, modified)); + } + public computeDirtyDiff(original: URI, modified: URI, ignoreTrimWhitespace: boolean): TPromise { return this._workerManager.withWorker().then(client => client.computeDirtyDiff(original, modified, ignoreTrimWhitespace)); } @@ -70,10 +94,17 @@ export class EditorWorkerServiceImpl extends Disposable implements IEditorWorker if (!Array.isArray(edits) || edits.length === 0) { return TPromise.as(edits); } else { + if (!canSyncModel(this._modelService, resource)) { + return TPromise.as(edits); // File too large + } return this._workerManager.withWorker().then(client => client.computeMoreMinimalEdits(resource, edits, ranges)); } } + public canNavigateValueSet(resource: URI): boolean { + return (canSyncModel(this._modelService, resource)); + } + public navigateValueSet(resource: URI, range: IRange, up: boolean): TPromise { return this._workerManager.withWorker().then(client => client.navigateValueSet(resource, range, up)); } @@ -84,11 +115,18 @@ class WordBasedCompletionItemProvider implements modes.ISuggestSupport { private readonly _workerManager: WorkerManager; private readonly _configurationService: ITextResourceConfigurationService; private readonly _modeService: IModeService; + private readonly _modelService: IModelService; - constructor(workerManager: WorkerManager, configurationService: ITextResourceConfigurationService, modeService: IModeService) { + constructor( + workerManager: WorkerManager, + configurationService: ITextResourceConfigurationService, + modeService: IModeService, + modelService: IModelService + ) { this._workerManager = workerManager; this._configurationService = configurationService; this._modeService = modeService; + this._modelService = modelService; } provideCompletionItems(model: editorCommon.IModel, position: Position): TPromise { @@ -96,6 +134,9 @@ class WordBasedCompletionItemProvider implements modes.ISuggestSupport { if (!wordBasedSuggestions) { return undefined; } + if (!canSyncModel(this._modelService, model.uri)) { + return undefined; // File too large + } return this._workerManager.withWorker().then(client => client.textualSuggest(model.uri, position)); } } @@ -228,6 +269,9 @@ class EditorModelManager extends Disposable { if (!model) { return; } + if (model.isTooLargeForTokenization()) { + return; + } let modelUrl = resource.toString(); diff --git a/src/vs/editor/contrib/inPlaceReplace/common/inPlaceReplace.ts b/src/vs/editor/contrib/inPlaceReplace/common/inPlaceReplace.ts index 030eff62c2574..3e9084e361a45 100644 --- a/src/vs/editor/contrib/inPlaceReplace/common/inPlaceReplace.ts +++ b/src/vs/editor/contrib/inPlaceReplace/common/inPlaceReplace.ts @@ -75,13 +75,17 @@ class InPlaceReplaceController implements IEditorContribution { var state = new EditorState(this.editor, CodeEditorStateFlag.Value | CodeEditorStateFlag.Position); - this.currentRequest = this.editorWorkerService.navigateValueSet(modelURI, selection, up); - this.currentRequest = this.currentRequest.then((basicResult) => { - if (basicResult && basicResult.range && basicResult.value) { - return basicResult; - } - return null; - }); + if (!this.editorWorkerService.canNavigateValueSet(modelURI)) { + this.currentRequest = TPromise.as(null); + } else { + this.currentRequest = this.editorWorkerService.navigateValueSet(modelURI, selection, up); + this.currentRequest = this.currentRequest.then((basicResult) => { + if (basicResult && basicResult.range && basicResult.value) { + return basicResult; + } + return null; + }); + } return this.currentRequest.then((result: IInplaceReplaceSupportResult) => { diff --git a/src/vs/editor/standalone/browser/standaloneCodeEditor.ts b/src/vs/editor/standalone/browser/standaloneCodeEditor.ts index eba533c0f79f6..4ec8207f3e036 100644 --- a/src/vs/editor/standalone/browser/standaloneCodeEditor.ts +++ b/src/vs/editor/standalone/browser/standaloneCodeEditor.ts @@ -26,6 +26,7 @@ import { MenuId, MenuRegistry, IMenuItem } from 'vs/platform/actions/common/acti import { IDiffEditorOptions, IEditorOptions } from 'vs/editor/common/config/editorOptions'; import { IThemeService } from 'vs/platform/theme/common/themeService'; import * as aria from 'vs/base/browser/ui/aria/aria'; +import { IMessageService } from "vs/platform/message/common/message"; /** * The options to create an editor. @@ -301,14 +302,15 @@ export class StandaloneDiffEditor extends DiffEditorWidget implements IStandalon @IContextViewService contextViewService: IContextViewService, @IEditorWorkerService editorWorkerService: IEditorWorkerService, @ICodeEditorService codeEditorService: ICodeEditorService, - @IStandaloneThemeService themeService: IStandaloneThemeService + @IStandaloneThemeService themeService: IStandaloneThemeService, + @IMessageService messageService: IMessageService ) { options = options || {}; if (typeof options.theme === 'string') { options.theme = themeService.setTheme(options.theme); } - super(domElement, options, editorWorkerService, contextKeyService, instantiationService, codeEditorService, themeService); + super(domElement, options, editorWorkerService, contextKeyService, instantiationService, codeEditorService, themeService, messageService); if (keybindingService instanceof StandaloneKeybindingService) { this._standaloneKeybindingService = keybindingService; diff --git a/src/vs/editor/standalone/browser/standaloneEditor.ts b/src/vs/editor/standalone/browser/standaloneEditor.ts index 5b8a1babae82f..9d42854a03d8b 100644 --- a/src/vs/editor/standalone/browser/standaloneEditor.ts +++ b/src/vs/editor/standalone/browser/standaloneEditor.ts @@ -36,6 +36,7 @@ import { Token } from 'vs/editor/common/core/token'; import { FontInfo, BareFontInfo } from 'vs/editor/common/config/fontInfo'; import * as editorOptions from 'vs/editor/common/config/editorOptions'; import { CursorChangeReason } from 'vs/editor/common/controller/cursorEvents'; +import { IMessageService } from "vs/platform/message/common/message"; /** * @internal @@ -127,7 +128,8 @@ export function createDiffEditor(domElement: HTMLElement, options?: IDiffEditorC services.get(IContextViewService), services.get(IEditorWorkerService), services.get(ICodeEditorService), - services.get(IStandaloneThemeService) + services.get(IStandaloneThemeService), + services.get(IMessageService) ); }); } diff --git a/src/vs/workbench/parts/scm/electron-browser/dirtydiffDecorator.ts b/src/vs/workbench/parts/scm/electron-browser/dirtydiffDecorator.ts index 32b3d1689d286..3aa3675295f21 100644 --- a/src/vs/workbench/parts/scm/electron-browser/dirtydiffDecorator.ts +++ b/src/vs/workbench/parts/scm/electron-browser/dirtydiffDecorator.ts @@ -110,6 +110,10 @@ class DirtyDiffModelDecorator { return winjs.TPromise.as([]); // disposed } + if (!this.editorWorkerService.canComputeDirtyDiff(originalURI, this.model.uri)) { + return winjs.TPromise.as([]); // Files too large + } + return this.editorWorkerService.computeDirtyDiff(originalURI, this.model.uri, true); }); }