Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets performance of the editor’s LineHeightsManager by reducing the cost of removing staged custom line-height updates, addressing jank when many custom line heights are present (issue #298719).
Changes:
- Introduces
_stagedDecorationIdToLinesto avoid O(n) scans/splices when removing staged inserts for a decoration. - Switches staged-removal behavior from array splicing to marking staged
CustomLineentries asdeletedand skipping them during flush. - Adds timing
console.loginstrumentation around commit/flush (appears to be debug instrumentation).
Comments suppressed due to low confidence (2)
src/vs/editor/common/viewLayout/lineHeights.ts:225
- _flushStagedDecorationChanges can enter an effectively infinite loop when
stagedInsertscontains only entries markeddeleted. In that case the insertion loop skips everything,_invalidIndexremainsInfinity, and the subsequentfor (let i = 0; i < this._invalidIndex; i++)never terminates (and will also read past_orderedCustomLines). Consider short-circuiting after processingstagedInsertswhen_invalidIndexis stillInfinity(i.e. no actual inserts/deletes affected the ordered array).
private _flushStagedDecorationChanges(stagedInserts: CustomLine[]): void {
if (stagedInserts.length === 0 && this._invalidIndex === Infinity) {
return;
}
const flushStart = Date.now();
for (const pendingChange of stagedInserts) {
if (pendingChange.deleted) {
continue;
}
const candidateInsertionIndex = this._binarySearchOverOrderedCustomLinesArray(pendingChange.lineNumber);
const insertionIndex = candidateInsertionIndex >= 0 ? candidateInsertionIndex : -(candidateInsertionIndex + 1);
this._orderedCustomLines.splice(insertionIndex, 0, pendingChange);
this._invalidIndex = Math.min(this._invalidIndex, insertionIndex);
}
stagedInserts.length = 0;
this._stagedDecorationIdToLines.clear();
const newDecorationIDToSpecialLine = new ArrayMap<string, CustomLine>();
const newOrderedSpecialLines: CustomLine[] = [];
for (let i = 0; i < this._invalidIndex; i++) {
const customLine = this._orderedCustomLines[i];
newOrderedSpecialLines.push(customLine);
newDecorationIDToSpecialLine.add(customLine.decorationId, customLine);
}
src/vs/editor/common/viewLayout/lineHeights.ts:174
_doRemoveCustomLineHeightno longer uses thestagedInsertsparameter. To avoid confusion (and potential lint noise), consider removing the parameter and updating call sites, or renaming it to_stagedInsertsif it must remain for signature compatibility.
private _doRemoveCustomLineHeight(decorationID: string, stagedInserts: CustomLine[]): void {
const customLines = this._decorationIDToCustomLine.get(decorationID);
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #298719
I added date.now() around the commit call, and with this PR, the commit reduces to:
Without this PR, the commit call takes: