Skip to content

Commit

Permalink
Merge pull request #4080 from Tyriar/decoration_gc
Browse files Browse the repository at this point in the history 
Optimize gc from decoration-related hot code paths
  • Loading branch information
@Tyriar
Tyriar authored Aug 28, 2022
2 parents 823f734 + 1b64a09 commit eb31850
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 56 deletions.
11 changes: 5 additions & 6 deletions addons/xterm-addon-canvas/src/BaseRenderLayer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,12 +404,11 @@ export abstract class BaseRenderLayer implements IRenderLayer {

// Don't try cache the glyph if it uses any decoration foreground/background override.
let hasOverrides = false;
for (const d of this._decorationService.getDecorationsAtCell(x, y)) {
this._decorationService.forEachDecorationAtCell(x, y, undefined, d => {
if (d.backgroundColorRGB || d.foregroundColorRGB) {
hasOverrides = true;
break;
}
}
});

const atlasDidDraw = hasOverrides ? false : this._charAtlas?.draw(this._ctx, this._currentGlyphIdentifier, x * this._scaledCellWidth + this._scaledCharLeft, y * this._scaledCellHeight + this._scaledCharTop);

Expand Down Expand Up @@ -519,9 +518,9 @@ export abstract class BaseRenderLayer implements IRenderLayer {
let bgOverride: number | undefined;
let fgOverride: number | undefined;
let isTop = false;
for (const d of this._decorationService.getDecorationsAtCell(x, y)) {
this._decorationService.forEachDecorationAtCell(x, y, undefined, d => {
if (d.options.layer !== 'top' && isTop) {
continue;
return;
}
if (d.backgroundColorRGB) {
bgOverride = d.backgroundColorRGB.rgba;
Expand All @@ -530,7 +529,7 @@ export abstract class BaseRenderLayer implements IRenderLayer {
fgOverride = d.foregroundColorRGB.rgba;
}
isTop = d.options.layer === 'top';
}
});

// Apply selection foreground if applicable
if (!isTop) {
Expand Down
6 changes: 3 additions & 3 deletions addons/xterm-addon-canvas/src/TextRenderLayer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,15 @@ export class TextRenderLayer extends BaseRenderLayer {
// Get any decoration foreground/background overrides, this must be fetched before the early
// exist but applied after inverse
let isTop = false;
for (const d of this._decorationService.getDecorationsAtCell(x, this._bufferService.buffer.ydisp + y)) {
this._decorationService.forEachDecorationAtCell(x, this._bufferService.buffer.ydisp + y, undefined, d => {
if (d.options.layer !== 'top' && isTop) {
continue;
return;
}
if (d.backgroundColorRGB) {
nextFillStyle = d.backgroundColorRGB.css;
}
isTop = d.options.layer === 'top';
}
});

if (prevFillStyle === null) {
// This is either the first iteration, or the default background was set. Either way, we
Expand Down
83 changes: 52 additions & 31 deletions addons/xterm-addon-webgl/src/WebglRenderer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { WebglCharAtlas } from './atlas/WebglCharAtlas';
import { RectangleRenderer } from './RectangleRenderer';
import { IWebGL2RenderingContext } from './Types';
import { RenderModel, COMBINED_CHAR_BIT_MASK, RENDER_MODEL_BG_OFFSET, RENDER_MODEL_FG_OFFSET, RENDER_MODEL_EXT_OFFSET, RENDER_MODEL_INDICIES_PER_CELL } from './RenderModel';
import { Disposable, toDisposable } from 'common/Lifecycle';
import { Disposable } from 'common/Lifecycle';
import { Attributes, BgFlags, Content, FgFlags, NULL_CELL_CHAR, NULL_CELL_CODE } from 'common/buffer/Constants';
import { Terminal, IEvent } from 'xterm';
import { IRenderLayer } from './renderLayer/Types';
Expand All @@ -26,6 +26,15 @@ import { CharData, ICellData } from 'common/Types';
import { AttributeData } from 'common/buffer/AttributeData';
import { ICoreService, IDecorationService } from 'common/services/Services';

/** Work variables to avoid garbage collection. */
const w: { fg: number, bg: number, hasFg: boolean, hasBg: boolean, isSelected: boolean } = {
fg: 0,
bg: 0,
hasFg: false,
hasBg: false,
isSelected: false
};

export class WebglRenderer extends Disposable implements IRenderer {
private _renderLayers: IRenderLayer[];
private _charAtlas: WebglCharAtlas | undefined;
Expand Down Expand Up @@ -404,79 +413,91 @@ export class WebglRenderer extends Disposable implements IRenderer {
this._workColors.ext = this._workCell.bg & BgFlags.HAS_EXTENDED ? this._workCell.extended.ext : 0;
// Get any foreground/background overrides, this happens on the model to avoid spreading
// override logic throughout the different sub-renderers
let bgOverride: number | undefined;
let fgOverride: number | undefined;
let isSelected: boolean = false;

// Reset overrides work variables
w.bg = 0;
w.fg = 0;
w.hasBg = false;
w.hasFg = false;
w.isSelected = false;

// Apply decorations on the bottom layer
for (const d of this._decorationService.getDecorationsAtCell(x, y, 'bottom')) {
this._decorationService.forEachDecorationAtCell(x, y, 'bottom', d => {
if (d.backgroundColorRGB) {
bgOverride = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF;
w.bg = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF;
w.hasBg = true;
}
if (d.foregroundColorRGB) {
fgOverride = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF;
w.fg = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF;
w.hasFg = true;
}
}
});

// Apply the selection color if needed
isSelected = this._isCellSelected(x, y);
if (isSelected) {
bgOverride = (this._coreBrowserService.isFocused ? this._colors.selectionBackgroundOpaque : this._colors.selectionInactiveBackgroundOpaque).rgba >> 8 & 0xFFFFFF;
w.isSelected = this._isCellSelected(x, y);
if (w.isSelected) {
w.bg = (this._coreBrowserService.isFocused ? this._colors.selectionBackgroundOpaque : this._colors.selectionInactiveBackgroundOpaque).rgba >> 8 & 0xFFFFFF;
w.hasBg = true;
if (this._colors.selectionForeground) {
fgOverride = this._colors.selectionForeground.rgba >> 8 & 0xFFFFFF;
w.fg = this._colors.selectionForeground.rgba >> 8 & 0xFFFFFF;
w.hasFg = true;
}
}

// Apply decorations on the top layer
for (const d of this._decorationService.getDecorationsAtCell(x, y, 'top')) {
this._decorationService.forEachDecorationAtCell(x, y, 'top', d => {
if (d.backgroundColorRGB) {
bgOverride = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF;
w.bg = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF;
w.hasBg = true;
}
if (d.foregroundColorRGB) {
fgOverride = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF;
w.fg = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF;
w.hasFg = true;
}
}
});

// Convert any overrides from rgba to the fg/bg packed format. This resolves the inverse flag
// ahead of time in order to use the correct cache key
if (bgOverride !== undefined) {
if (isSelected) {
if (w.hasBg) {
if (w.isSelected) {
// Non-RGB attributes from model + force non-dim + override + force RGB color mode
bgOverride = (this._workCell.bg & ~Attributes.RGB_MASK & ~BgFlags.DIM) | bgOverride | Attributes.CM_RGB;
w.bg = (this._workCell.bg & ~Attributes.RGB_MASK & ~BgFlags.DIM) | w.bg | Attributes.CM_RGB;
} else {
// Non-RGB attributes from model + override + force RGB color mode
bgOverride = (this._workCell.bg & ~Attributes.RGB_MASK) | bgOverride | Attributes.CM_RGB;
w.bg = (this._workCell.bg & ~Attributes.RGB_MASK) | w.bg | Attributes.CM_RGB;
}
}
if (fgOverride !== undefined) {
if (w.hasFg) {
// Non-RGB attributes from model + force disable inverse + override + force RGB color mode
fgOverride = (this._workCell.fg & ~Attributes.RGB_MASK & ~FgFlags.INVERSE) | fgOverride | Attributes.CM_RGB;
w.fg = (this._workCell.fg & ~Attributes.RGB_MASK & ~FgFlags.INVERSE) | w.fg | Attributes.CM_RGB;
}

// Handle case where inverse was specified by only one of bgOverride or fgOverride was set,
// Handle case where inverse was specified by only one of bg override or fg override was set,
// resolving the other inverse color and setting the inverse flag if needed.
if (this._workColors.fg & FgFlags.INVERSE) {
if (bgOverride !== undefined && fgOverride === undefined) {
if (w.hasBg && !w.hasFg) {
// Resolve bg color type (default color has a different meaning in fg vs bg)
if ((this._workColors.bg & Attributes.CM_MASK) === Attributes.CM_DEFAULT) {
fgOverride = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | ((this._colors.background.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB;
w.fg = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | ((this._colors.background.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB;
} else {
fgOverride = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | this._workColors.bg & (Attributes.RGB_MASK | Attributes.CM_MASK);
w.fg = (this._workColors.fg & ~(Attributes.RGB_MASK | FgFlags.INVERSE | Attributes.CM_MASK)) | this._workColors.bg & (Attributes.RGB_MASK | Attributes.CM_MASK);
}
w.hasFg = true;
}
if (bgOverride === undefined && fgOverride !== undefined) {
if (!w.hasBg && w.hasFg) {
// Resolve bg color type (default color has a different meaning in fg vs bg)
if ((this._workColors.fg & Attributes.CM_MASK) === Attributes.CM_DEFAULT) {
bgOverride = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | ((this._colors.foreground.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB;
w.bg = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | ((this._colors.foreground.rgba >> 8 & 0xFFFFFF) & Attributes.RGB_MASK) | Attributes.CM_RGB;
} else {
bgOverride = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | this._workColors.fg & (Attributes.RGB_MASK | Attributes.CM_MASK);
w.bg = (this._workColors.bg & ~(Attributes.RGB_MASK | Attributes.CM_MASK)) | this._workColors.fg & (Attributes.RGB_MASK | Attributes.CM_MASK);
}
w.hasBg = true;
}
}

// Use the override if it exists
this._workColors.bg = bgOverride ?? this._workColors.bg;
this._workColors.fg = fgOverride ?? this._workColors.fg;
this._workColors.bg = w.hasBg ? w.bg : this._workColors.bg;
this._workColors.fg = w.hasFg ? w.fg : this._workColors.fg;
}

private _isCellSelected(x: number, y: number): boolean {
Expand Down
6 changes: 3 additions & 3 deletions src/browser/renderer/dom/DomRendererRowFactory.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,9 @@ export class DomRendererRowFactory {
let bgOverride: IColor | undefined;
let fgOverride: IColor | undefined;
let isTop = false;
for (const d of this._decorationService.getDecorationsAtCell(x, row)) {
this._decorationService.forEachDecorationAtCell(x, row, undefined, d => {
if (d.options.layer !== 'top' && isTop) {
continue;
return;
}
if (d.backgroundColorRGB) {
bgColorMode = Attributes.CM_RGB;
Expand All @@ -219,7 +219,7 @@ export class DomRendererRowFactory {
fgOverride = d.foregroundColorRGB;
}
isTop = d.options.layer === 'top';
}
});

// Apply selection foreground if applicable
const isInSelection = this._isCellInSelection(x, row);
Expand Down
25 changes: 22 additions & 3 deletions src/common/SortedList.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
* @license MIT
*/

// Work variables to avoid garbage collection.
let i = 0;

/**
* A generic list that is maintained in sorted order and allows values with duplicate keys. This
* list is based on binary search and as such locating a key will take O(log n) amortized, this
Expand All @@ -25,7 +28,7 @@ export class SortedList<T> {
this._array.push(value);
return;
}
const i = this._search(this._getKey(value), 0, this._array.length - 1);
i = this._search(this._getKey(value), 0, this._array.length - 1);
this._array.splice(i, 0, value);
}

Expand All @@ -37,7 +40,7 @@ export class SortedList<T> {
if (key === undefined) {
return false;
}
let i = this._search(key, 0, this._array.length - 1);
i = this._search(key, 0, this._array.length - 1);
if (i === -1) {
return false;
}
Expand All @@ -57,7 +60,7 @@ export class SortedList<T> {
if (this._array.length === 0) {
return;
}
let i = this._search(key, 0, this._array.length - 1);
i = this._search(key, 0, this._array.length - 1);
if (i < 0 || i >= this._array.length) {
return;
}
Expand All @@ -69,6 +72,22 @@ export class SortedList<T> {
} while (++i < this._array.length && this._getKey(this._array[i]) === key);
}

public forEachByKey(key: number, callback: (value: T) => void): void {
if (this._array.length === 0) {
return;
}
i = this._search(key, 0, this._array.length - 1);
if (i < 0 || i >= this._array.length) {
return;
}
if (this._getKey(this._array[i]) !== key) {
return;
}
do {
callback(this._array[i]);
} while (++i < this._array.length && this._getKey(this._array[i]) === key);
}

public values(): IterableIterator<T> {
return this._array.values();
}
Expand Down
3 changes: 1 addition & 2 deletions src/common/TestUtils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ export class MockDecorationService implements IDecorationService {
public onDecorationRemoved = new EventEmitter<IInternalDecoration>().event;
public registerDecoration(decorationOptions: IDecorationOptions): IDecoration | undefined { return undefined; }
public reset(): void { }
public *getDecorationsAtLine(line: number): IterableIterator<IInternalDecoration> { }
public *getDecorationsAtCell(x: number, line: number): IterableIterator<IInternalDecoration> { }
public forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void { }
public dispose(): void { }
}
20 changes: 16 additions & 4 deletions src/common/services/DecorationService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ import { SortedList } from 'common/SortedList';
import { IColor } from 'common/Types';
import { IDecorationOptions, IDecoration, IMarker, IEvent } from 'xterm';

/** Work variables to avoid garbage collection. */
const w = {
xmin: 0,
xmax: 0
};

export class DecorationService extends Disposable implements IDecorationService {
public serviceBrand: any;

Expand Down Expand Up @@ -56,10 +62,6 @@ export class DecorationService extends Disposable implements IDecorationService
this._decorations.clear();
}

public *getDecorationsAtLine(line: number): IterableIterator<IInternalDecoration> {
return this._decorations.getKeyIterator(line);
}

public *getDecorationsAtCell(x: number, line: number, layer?: 'bottom' | 'top'): IterableIterator<IInternalDecoration> {
let xmin = 0;
let xmax = 0;
Expand All @@ -72,6 +74,16 @@ export class DecorationService extends Disposable implements IDecorationService
}
}

public forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void {
this._decorations.forEachByKey(line, d => {
w.xmin = d.options.x ?? 0;
w.xmax = w.xmin + (d.options.width ?? 1);
if (x >= w.xmin && x < w.xmax && (!layer || (d.options.layer ?? 'bottom') === layer)) {
callback(d);
}
});
}

public dispose(): void {
for (const d of this._decorations.values()) {
this._onDecorationRemoved.fire(d);
Expand Down
9 changes: 5 additions & 4 deletions src/common/services/Services.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,11 @@ export interface IDecorationService extends IDisposable {
readonly onDecorationRemoved: IEvent<IInternalDecoration>;
registerDecoration(decorationOptions: IDecorationOptions): IDecoration | undefined;
reset(): void;
/** Iterates over the decorations at a line (in no particular order). */
getDecorationsAtLine(line: number): IterableIterator<IInternalDecoration>;
/** Iterates over the decorations at a cell (in no particular order). */
getDecorationsAtCell(x: number, line: number, layer?: 'bottom' | 'top'): IterableIterator<IInternalDecoration>;
/**
* Trigger a callback over the decoration at a cell (in no particular order). This uses a callback
* instead of an iterator as it's typically used in hot code paths.
*/
forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void;
}
export interface IInternalDecoration extends IDecoration {
readonly options: IDecorationOptions;
Expand Down

0 comments on commit eb31850

Please sign in to comment.