From fe6e24fd74ab5d5dba984aab7afe53929b116a93 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 20 Oct 2022 14:34:02 -0700 Subject: [PATCH] DidChangeDecorationsEmitter should fire right away if it is not being deferred (#164065) While debugging a performance issue, I noticed that `DidChangeDecorationsEmitter` will only fire when `endDeferredEmit` is called. This means that if the emitter is not being deferred and someone calls `.fire`, we don't fire anything until someone else comes along and calls `beginDeferredEmit` and `endDeferredEmit` This change makes it so that the event is fired right away if the emitter is not deferred --- src/vs/editor/common/model/textModel.ts | 161 +++++++++++++----------- 1 file changed, 89 insertions(+), 72 deletions(-) diff --git a/src/vs/editor/common/model/textModel.ts b/src/vs/editor/common/model/textModel.ts index 27ee2e71319..5d194284195 100644 --- a/src/vs/editor/common/model/textModel.ts +++ b/src/vs/editor/common/model/textModel.ts @@ -1826,76 +1826,81 @@ export class TextModel extends Disposable implements model.ITextModel, IDecorati const newDecorationsLen = newDecorations.length; let newDecorationIndex = 0; - const result = new Array(newDecorationsLen); - while (oldDecorationIndex < oldDecorationsLen || newDecorationIndex < newDecorationsLen) { + this._onDidChangeDecorations.beginDeferredEmit(); + try { + const result = new Array(newDecorationsLen); + while (oldDecorationIndex < oldDecorationsLen || newDecorationIndex < newDecorationsLen) { - let node: IntervalNode | null = null; + let node: IntervalNode | null = null; - if (oldDecorationIndex < oldDecorationsLen) { - // (1) get ourselves an old node - do { - node = this._decorations[oldDecorationsIds[oldDecorationIndex++]]; - } while (!node && oldDecorationIndex < oldDecorationsLen); + if (oldDecorationIndex < oldDecorationsLen) { + // (1) get ourselves an old node + do { + node = this._decorations[oldDecorationsIds[oldDecorationIndex++]]; + } while (!node && oldDecorationIndex < oldDecorationsLen); + + // (2) remove the node from the tree (if it exists) + if (node) { + if (node.options.after) { + const nodeRange = this._decorationsTree.getNodeRange(this, node); + this._onDidChangeDecorations.recordLineAffectedByInjectedText(nodeRange.endLineNumber); + } + if (node.options.before) { + const nodeRange = this._decorationsTree.getNodeRange(this, node); + this._onDidChangeDecorations.recordLineAffectedByInjectedText(nodeRange.startLineNumber); + } + + this._decorationsTree.delete(node); + + this._onDidChangeDecorations.checkAffectedAndFire(node.options); + } + } + + if (newDecorationIndex < newDecorationsLen) { + // (3) create a new node if necessary + if (!node) { + const internalDecorationId = (++this._lastDecorationId); + const decorationId = `${this._instanceId};${internalDecorationId}`; + node = new IntervalNode(decorationId, 0, 0); + this._decorations[decorationId] = node; + } + + // (4) initialize node + const newDecoration = newDecorations[newDecorationIndex]; + const range = this._validateRangeRelaxedNoAllocations(newDecoration.range); + const options = _normalizeOptions(newDecoration.options); + const startOffset = this._buffer.getOffsetAt(range.startLineNumber, range.startColumn); + const endOffset = this._buffer.getOffsetAt(range.endLineNumber, range.endColumn); + + node.ownerId = ownerId; + node.reset(versionId, startOffset, endOffset, range); + node.setOptions(options); - // (2) remove the node from the tree (if it exists) - if (node) { if (node.options.after) { - const nodeRange = this._decorationsTree.getNodeRange(this, node); - this._onDidChangeDecorations.recordLineAffectedByInjectedText(nodeRange.endLineNumber); + this._onDidChangeDecorations.recordLineAffectedByInjectedText(range.endLineNumber); } if (node.options.before) { - const nodeRange = this._decorationsTree.getNodeRange(this, node); - this._onDidChangeDecorations.recordLineAffectedByInjectedText(nodeRange.startLineNumber); + this._onDidChangeDecorations.recordLineAffectedByInjectedText(range.startLineNumber); } - this._decorationsTree.delete(node); + this._onDidChangeDecorations.checkAffectedAndFire(options); - this._onDidChangeDecorations.checkAffectedAndFire(node.options); + this._decorationsTree.insert(node); + + result[newDecorationIndex] = node.id; + + newDecorationIndex++; + } else { + if (node) { + delete this._decorations[node.id]; + } } } - if (newDecorationIndex < newDecorationsLen) { - // (3) create a new node if necessary - if (!node) { - const internalDecorationId = (++this._lastDecorationId); - const decorationId = `${this._instanceId};${internalDecorationId}`; - node = new IntervalNode(decorationId, 0, 0); - this._decorations[decorationId] = node; - } - - // (4) initialize node - const newDecoration = newDecorations[newDecorationIndex]; - const range = this._validateRangeRelaxedNoAllocations(newDecoration.range); - const options = _normalizeOptions(newDecoration.options); - const startOffset = this._buffer.getOffsetAt(range.startLineNumber, range.startColumn); - const endOffset = this._buffer.getOffsetAt(range.endLineNumber, range.endColumn); - - node.ownerId = ownerId; - node.reset(versionId, startOffset, endOffset, range); - node.setOptions(options); - - if (node.options.after) { - this._onDidChangeDecorations.recordLineAffectedByInjectedText(range.endLineNumber); - } - if (node.options.before) { - this._onDidChangeDecorations.recordLineAffectedByInjectedText(range.startLineNumber); - } - - this._onDidChangeDecorations.checkAffectedAndFire(options); - - this._decorationsTree.insert(node); - - result[newDecorationIndex] = node.id; - - newDecorationIndex++; - } else { - if (node) { - delete this._decorations[node.id]; - } - } + return result; + } finally { + this._onDidChangeDecorations.endDeferredEmit(); } - - return result; } //#endregion @@ -2309,7 +2314,7 @@ export class DidChangeDecorationsEmitter extends Disposable { public readonly event: Event = this._actual.event; private _deferredCnt: number; - private _shouldFire: boolean; + private _shouldFireDeferred: boolean; private _affectsMinimap: boolean; private _affectsOverviewRuler: boolean; private _affectedInjectedTextLines: Set | null = null; @@ -2317,7 +2322,7 @@ export class DidChangeDecorationsEmitter extends Disposable { constructor(private readonly handleBeforeFire: (affectedInjectedTextLines: Set | null) => void) { super(); this._deferredCnt = 0; - this._shouldFire = false; + this._shouldFireDeferred = false; this._affectsMinimap = false; this._affectsOverviewRuler = false; } @@ -2333,17 +2338,8 @@ export class DidChangeDecorationsEmitter extends Disposable { public endDeferredEmit(): void { this._deferredCnt--; if (this._deferredCnt === 0) { - if (this._shouldFire) { - this.handleBeforeFire(this._affectedInjectedTextLines); - - const event: IModelDecorationsChangedEvent = { - affectsMinimap: this._affectsMinimap, - affectsOverviewRuler: this._affectsOverviewRuler - }; - this._shouldFire = false; - this._affectsMinimap = false; - this._affectsOverviewRuler = false; - this._actual.fire(event); + if (this._shouldFireDeferred) { + this.doFire(); } this._affectedInjectedTextLines?.clear(); @@ -2365,13 +2361,34 @@ export class DidChangeDecorationsEmitter extends Disposable { if (!this._affectsOverviewRuler) { this._affectsOverviewRuler = options.overviewRuler && options.overviewRuler.color ? true : false; } - this._shouldFire = true; + this.tryFire(); } public fire(): void { this._affectsMinimap = true; this._affectsOverviewRuler = true; - this._shouldFire = true; + this.tryFire(); + } + + private tryFire() { + if (this._deferredCnt === 0) { + this.doFire(); + } else { + this._shouldFireDeferred = true; + } + } + + private doFire() { + this.handleBeforeFire(this._affectedInjectedTextLines); + + const event: IModelDecorationsChangedEvent = { + affectsMinimap: this._affectsMinimap, + affectsOverviewRuler: this._affectsOverviewRuler + }; + this._shouldFireDeferred = false; + this._affectsMinimap = false; + this._affectsOverviewRuler = false; + this._actual.fire(event); } }