Skip to content

Commit fe6e24f

Browse files
authored
DidChangeDecorationsEmitter should fire right away if it is not being deferred (microsoft#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
1 parent e341419 commit fe6e24f

File tree

1 file changed

+87
-70
lines changed

1 file changed

+87
-70
lines changed

src/vs/editor/common/model/textModel.ts

Lines changed: 87 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1826,76 +1826,81 @@ export class TextModel extends Disposable implements model.ITextModel, IDecorati
18261826
const newDecorationsLen = newDecorations.length;
18271827
let newDecorationIndex = 0;
18281828

1829-
const result = new Array<string>(newDecorationsLen);
1830-
while (oldDecorationIndex < oldDecorationsLen || newDecorationIndex < newDecorationsLen) {
1831-
1832-
let node: IntervalNode | null = null;
1829+
this._onDidChangeDecorations.beginDeferredEmit();
1830+
try {
1831+
const result = new Array<string>(newDecorationsLen);
1832+
while (oldDecorationIndex < oldDecorationsLen || newDecorationIndex < newDecorationsLen) {
1833+
1834+
let node: IntervalNode | null = null;
1835+
1836+
if (oldDecorationIndex < oldDecorationsLen) {
1837+
// (1) get ourselves an old node
1838+
do {
1839+
node = this._decorations[oldDecorationsIds[oldDecorationIndex++]];
1840+
} while (!node && oldDecorationIndex < oldDecorationsLen);
1841+
1842+
// (2) remove the node from the tree (if it exists)
1843+
if (node) {
1844+
if (node.options.after) {
1845+
const nodeRange = this._decorationsTree.getNodeRange(this, node);
1846+
this._onDidChangeDecorations.recordLineAffectedByInjectedText(nodeRange.endLineNumber);
1847+
}
1848+
if (node.options.before) {
1849+
const nodeRange = this._decorationsTree.getNodeRange(this, node);
1850+
this._onDidChangeDecorations.recordLineAffectedByInjectedText(nodeRange.startLineNumber);
1851+
}
18331852

1834-
if (oldDecorationIndex < oldDecorationsLen) {
1835-
// (1) get ourselves an old node
1836-
do {
1837-
node = this._decorations[oldDecorationsIds[oldDecorationIndex++]];
1838-
} while (!node && oldDecorationIndex < oldDecorationsLen);
1853+
this._decorationsTree.delete(node);
18391854

1840-
// (2) remove the node from the tree (if it exists)
1841-
if (node) {
1842-
if (node.options.after) {
1843-
const nodeRange = this._decorationsTree.getNodeRange(this, node);
1844-
this._onDidChangeDecorations.recordLineAffectedByInjectedText(nodeRange.endLineNumber);
1855+
this._onDidChangeDecorations.checkAffectedAndFire(node.options);
18451856
}
1846-
if (node.options.before) {
1847-
const nodeRange = this._decorationsTree.getNodeRange(this, node);
1848-
this._onDidChangeDecorations.recordLineAffectedByInjectedText(nodeRange.startLineNumber);
1849-
}
1850-
1851-
this._decorationsTree.delete(node);
1852-
1853-
this._onDidChangeDecorations.checkAffectedAndFire(node.options);
18541857
}
1855-
}
18561858

1857-
if (newDecorationIndex < newDecorationsLen) {
1858-
// (3) create a new node if necessary
1859-
if (!node) {
1860-
const internalDecorationId = (++this._lastDecorationId);
1861-
const decorationId = `${this._instanceId};${internalDecorationId}`;
1862-
node = new IntervalNode(decorationId, 0, 0);
1863-
this._decorations[decorationId] = node;
1864-
}
1859+
if (newDecorationIndex < newDecorationsLen) {
1860+
// (3) create a new node if necessary
1861+
if (!node) {
1862+
const internalDecorationId = (++this._lastDecorationId);
1863+
const decorationId = `${this._instanceId};${internalDecorationId}`;
1864+
node = new IntervalNode(decorationId, 0, 0);
1865+
this._decorations[decorationId] = node;
1866+
}
18651867

1866-
// (4) initialize node
1867-
const newDecoration = newDecorations[newDecorationIndex];
1868-
const range = this._validateRangeRelaxedNoAllocations(newDecoration.range);
1869-
const options = _normalizeOptions(newDecoration.options);
1870-
const startOffset = this._buffer.getOffsetAt(range.startLineNumber, range.startColumn);
1871-
const endOffset = this._buffer.getOffsetAt(range.endLineNumber, range.endColumn);
1868+
// (4) initialize node
1869+
const newDecoration = newDecorations[newDecorationIndex];
1870+
const range = this._validateRangeRelaxedNoAllocations(newDecoration.range);
1871+
const options = _normalizeOptions(newDecoration.options);
1872+
const startOffset = this._buffer.getOffsetAt(range.startLineNumber, range.startColumn);
1873+
const endOffset = this._buffer.getOffsetAt(range.endLineNumber, range.endColumn);
18721874

1873-
node.ownerId = ownerId;
1874-
node.reset(versionId, startOffset, endOffset, range);
1875-
node.setOptions(options);
1875+
node.ownerId = ownerId;
1876+
node.reset(versionId, startOffset, endOffset, range);
1877+
node.setOptions(options);
18761878

1877-
if (node.options.after) {
1878-
this._onDidChangeDecorations.recordLineAffectedByInjectedText(range.endLineNumber);
1879-
}
1880-
if (node.options.before) {
1881-
this._onDidChangeDecorations.recordLineAffectedByInjectedText(range.startLineNumber);
1882-
}
1879+
if (node.options.after) {
1880+
this._onDidChangeDecorations.recordLineAffectedByInjectedText(range.endLineNumber);
1881+
}
1882+
if (node.options.before) {
1883+
this._onDidChangeDecorations.recordLineAffectedByInjectedText(range.startLineNumber);
1884+
}
18831885

1884-
this._onDidChangeDecorations.checkAffectedAndFire(options);
1886+
this._onDidChangeDecorations.checkAffectedAndFire(options);
18851887

1886-
this._decorationsTree.insert(node);
1888+
this._decorationsTree.insert(node);
18871889

1888-
result[newDecorationIndex] = node.id;
1890+
result[newDecorationIndex] = node.id;
18891891

1890-
newDecorationIndex++;
1891-
} else {
1892-
if (node) {
1893-
delete this._decorations[node.id];
1892+
newDecorationIndex++;
1893+
} else {
1894+
if (node) {
1895+
delete this._decorations[node.id];
1896+
}
18941897
}
18951898
}
1896-
}
18971899

1898-
return result;
1900+
return result;
1901+
} finally {
1902+
this._onDidChangeDecorations.endDeferredEmit();
1903+
}
18991904
}
19001905

19011906
//#endregion
@@ -2309,15 +2314,15 @@ export class DidChangeDecorationsEmitter extends Disposable {
23092314
public readonly event: Event<IModelDecorationsChangedEvent> = this._actual.event;
23102315

23112316
private _deferredCnt: number;
2312-
private _shouldFire: boolean;
2317+
private _shouldFireDeferred: boolean;
23132318
private _affectsMinimap: boolean;
23142319
private _affectsOverviewRuler: boolean;
23152320
private _affectedInjectedTextLines: Set<number> | null = null;
23162321

23172322
constructor(private readonly handleBeforeFire: (affectedInjectedTextLines: Set<number> | null) => void) {
23182323
super();
23192324
this._deferredCnt = 0;
2320-
this._shouldFire = false;
2325+
this._shouldFireDeferred = false;
23212326
this._affectsMinimap = false;
23222327
this._affectsOverviewRuler = false;
23232328
}
@@ -2333,17 +2338,8 @@ export class DidChangeDecorationsEmitter extends Disposable {
23332338
public endDeferredEmit(): void {
23342339
this._deferredCnt--;
23352340
if (this._deferredCnt === 0) {
2336-
if (this._shouldFire) {
2337-
this.handleBeforeFire(this._affectedInjectedTextLines);
2338-
2339-
const event: IModelDecorationsChangedEvent = {
2340-
affectsMinimap: this._affectsMinimap,
2341-
affectsOverviewRuler: this._affectsOverviewRuler
2342-
};
2343-
this._shouldFire = false;
2344-
this._affectsMinimap = false;
2345-
this._affectsOverviewRuler = false;
2346-
this._actual.fire(event);
2341+
if (this._shouldFireDeferred) {
2342+
this.doFire();
23472343
}
23482344

23492345
this._affectedInjectedTextLines?.clear();
@@ -2365,13 +2361,34 @@ export class DidChangeDecorationsEmitter extends Disposable {
23652361
if (!this._affectsOverviewRuler) {
23662362
this._affectsOverviewRuler = options.overviewRuler && options.overviewRuler.color ? true : false;
23672363
}
2368-
this._shouldFire = true;
2364+
this.tryFire();
23692365
}
23702366

23712367
public fire(): void {
23722368
this._affectsMinimap = true;
23732369
this._affectsOverviewRuler = true;
2374-
this._shouldFire = true;
2370+
this.tryFire();
2371+
}
2372+
2373+
private tryFire() {
2374+
if (this._deferredCnt === 0) {
2375+
this.doFire();
2376+
} else {
2377+
this._shouldFireDeferred = true;
2378+
}
2379+
}
2380+
2381+
private doFire() {
2382+
this.handleBeforeFire(this._affectedInjectedTextLines);
2383+
2384+
const event: IModelDecorationsChangedEvent = {
2385+
affectsMinimap: this._affectsMinimap,
2386+
affectsOverviewRuler: this._affectsOverviewRuler
2387+
};
2388+
this._shouldFireDeferred = false;
2389+
this._affectsMinimap = false;
2390+
this._affectsOverviewRuler = false;
2391+
this._actual.fire(event);
23752392
}
23762393
}
23772394

0 commit comments

Comments
 (0)