From 80eed34534699cf5dc45ab173fb52378eba9692d Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Fri, 10 Sep 2021 15:23:03 -0600 Subject: [PATCH] Use `_private` instead of `#private` properties The updates to the template compiler made in support of strict mode introduced a number of uses of private class fields, some of them in hot paths. Although all supported versions of Node support private class fields, `tsc` transpiles private class fields to `WeakMap`s for all values of `target` lower than `ESNEXT`. As a result, the use of private class fields results in dramatically higher memory usage and garbage collection than in the previous version of the template compiler. This in turn causes at least a large portion of the regression noted in [emberjs/ember.js#19750][1]. [1]: https://github.com/emberjs/ember.js/issues/19750 Here, we replace *all* private class fields with `_` private fields, which substantially (thought not 100%) closed the gap with the original. Co-authored-by: Brenden Palmer --- .../lib/passes/1-normalization/context.ts | 16 +++++------ .../passes/1-normalization/keywords/impl.ts | 12 ++++---- .../@glimmer/syntax/lib/source/loc/match.ts | 24 ++++++++-------- .../@glimmer/syntax/lib/source/loc/offset.ts | 18 ++++++------ .../@glimmer/syntax/lib/source/loc/span.ts | 28 +++++++++---------- .../@glimmer/syntax/lib/source/span-list.ts | 12 ++++---- packages/@glimmer/syntax/lib/symbol-table.ts | 6 ++-- 7 files changed, 58 insertions(+), 58 deletions(-) diff --git a/packages/@glimmer/compiler/lib/passes/1-normalization/context.ts b/packages/@glimmer/compiler/lib/passes/1-normalization/context.ts index 4e2b541187..d379a005c1 100644 --- a/packages/@glimmer/compiler/lib/passes/1-normalization/context.ts +++ b/packages/@glimmer/compiler/lib/passes/1-normalization/context.ts @@ -9,29 +9,29 @@ import { VISIT_STMTS } from './visitors/statements'; * This is the mutable state for this compiler pass. */ export class NormalizationState { - #currentScope: SymbolTable; - #cursorCount = 0; + _currentScope: SymbolTable; + _cursorCount = 0; constructor(block: SymbolTable, readonly isStrict: boolean) { - this.#currentScope = block; + this._currentScope = block; } generateUniqueCursor(): string { - return `%cursor:${this.#cursorCount++}%`; + return `%cursor:${this._cursorCount++}%`; } get scope(): SymbolTable { - return this.#currentScope; + return this._currentScope; } visitBlock(block: ASTv2.Block): Result> { - let oldBlock = this.#currentScope; - this.#currentScope = block.scope; + let oldBlock = this._currentScope; + this._currentScope = block.scope; try { return VISIT_STMTS.visitList(block.body, this); } finally { - this.#currentScope = oldBlock; + this._currentScope = oldBlock; } } } diff --git a/packages/@glimmer/compiler/lib/passes/1-normalization/keywords/impl.ts b/packages/@glimmer/compiler/lib/passes/1-normalization/keywords/impl.ts index b8da4c1216..dd9019b9a2 100644 --- a/packages/@glimmer/compiler/lib/passes/1-normalization/keywords/impl.ts +++ b/packages/@glimmer/compiler/lib/passes/1-normalization/keywords/impl.ts @@ -168,18 +168,18 @@ function getCalleeExpression( export class Keywords = never> implements Keyword> { - #keywords: Keyword[] = []; - #type: K; + _keywords: Keyword[] = []; + _type: K; constructor(type: K) { - this.#type = type; + this._type = type; } kw( name: S, delegate: KeywordDelegate ): Keywords> { - this.#keywords.push(keyword(name, this.#type, delegate)); + this._keywords.push(keyword(name, this._type, delegate)); return this; } @@ -188,7 +188,7 @@ export class Keywords = ne node: KeywordCandidates[K], state: NormalizationState ): Result> | null { - for (let keyword of this.#keywords) { + for (let keyword of this._keywords) { let result = keyword.translate(node, state) as Result>; if (result !== null) { return result; @@ -200,7 +200,7 @@ export class Keywords = ne if (path && path.type === 'Path' && path.ref.type === 'Free' && isKeyword(path.ref.name)) { let { name } = path.ref; - let usedType = this.#type; + let usedType = this._type; let validTypes = KEYWORDS_TYPES[name]; if (validTypes.indexOf(usedType) === -1) { diff --git a/packages/@glimmer/syntax/lib/source/loc/match.ts b/packages/@glimmer/syntax/lib/source/loc/match.ts index f441011997..41b6020f1e 100644 --- a/packages/@glimmer/syntax/lib/source/loc/match.ts +++ b/packages/@glimmer/syntax/lib/source/loc/match.ts @@ -28,14 +28,14 @@ export type IsInvisible = 'IS_INVISIBLE'; type Pattern = OffsetKind | IsInvisible | MatchAny; class WhenList { - #whens: When[]; + _whens: When[]; constructor(whens: When[]) { - this.#whens = whens; + this._whens = whens; } first(kind: OffsetKind): Out | null { - for (let when of this.#whens) { + for (let when of this._whens) { let value = when.match(kind); if (isPresent(value)) { return value[0]; @@ -47,10 +47,10 @@ class WhenList { } class When { - #map: Map = new Map(); + _map: Map = new Map(); get(pattern: Pattern, or: () => Out): Out { - let value = this.#map.get(pattern); + let value = this._map.get(pattern); if (value) { return value; @@ -58,13 +58,13 @@ class When { value = or(); - this.#map.set(pattern, value); + this._map.set(pattern, value); return value; } add(pattern: Pattern, out: Out): void { - this.#map.set(pattern, out); + this._map.set(pattern, out); } match(kind: OffsetKind): Out[] { @@ -72,8 +72,8 @@ class When { let out: Out[] = []; - let exact = this.#map.get(pattern); - let fallback = this.#map.get(MatchAny); + let exact = this._map.get(pattern); + let fallback = this._map.get(MatchAny); if (exact) { out.push(exact); @@ -105,7 +105,7 @@ export function match(callback: (m: Matcher) => ExhaustiveMatcher } class Matcher { - #whens: When Out>> = new When(); + _whens: When Out>> = new When(); /** * You didn't exhaustively match all possibilities. @@ -118,7 +118,7 @@ class Matcher { left: OffsetKind, right: OffsetKind ): (left: PositionData, right: PositionData) => Out { - let nesteds = this.#whens.match(left); + let nesteds = this._whens.match(left); assert( isPresent(nesteds), @@ -179,7 +179,7 @@ class Matcher { // eslint-disable-next-line @typescript-eslint/no-explicit-any callback: (left: any, right: any) => Out ): Matcher | ExhaustiveMatcher { - this.#whens.get(left, () => new When()).add(right, callback); + this._whens.get(left, () => new When()).add(right, callback); return this; } diff --git a/packages/@glimmer/syntax/lib/source/loc/offset.ts b/packages/@glimmer/syntax/lib/source/loc/offset.ts index 8cd9091969..76dd62ac85 100644 --- a/packages/@glimmer/syntax/lib/source/loc/offset.ts +++ b/packages/@glimmer/syntax/lib/source/loc/offset.ts @@ -162,7 +162,7 @@ export class CharPosition implements PositionData { readonly kind = OffsetKind.CharPosition; /** Computed from char offset */ - #locPos: HbsPosition | BROKEN | null = null; + _locPos: HbsPosition | BROKEN | null = null; constructor(readonly source: Source, readonly charPos: number) {} @@ -206,15 +206,15 @@ export class CharPosition implements PositionData { * computing the `HbsPosition` should be a one-time operation. */ toHbsPos(): HbsPosition | null { - let locPos = this.#locPos; + let locPos = this._locPos; if (locPos === null) { let hbsPos = this.source.hbsPosFor(this.charPos); if (hbsPos === null) { - this.#locPos = locPos = BROKEN; + this._locPos = locPos = BROKEN; } else { - this.#locPos = locPos = new HbsPosition(this.source, hbsPos, this.charPos); + this._locPos = locPos = new HbsPosition(this.source, hbsPos, this.charPos); } } @@ -225,14 +225,14 @@ export class CharPosition implements PositionData { export class HbsPosition implements PositionData { readonly kind = OffsetKind.HbsPosition; - #charPos: CharPosition | BROKEN | null; + _charPos: CharPosition | BROKEN | null; constructor( readonly source: Source, readonly hbsPos: SourcePosition, charPos: number | null = null ) { - this.#charPos = charPos === null ? null : new CharPosition(source, charPos); + this._charPos = charPos === null ? null : new CharPosition(source, charPos); } /** @@ -244,15 +244,15 @@ export class HbsPosition implements PositionData { * @implements {PositionData} */ toCharPos(): CharPosition | null { - let charPos = this.#charPos; + let charPos = this._charPos; if (charPos === null) { let charPosNumber = this.source.charPosFor(this.hbsPos); if (charPosNumber === null) { - this.#charPos = charPos = BROKEN; + this._charPos = charPos = BROKEN; } else { - this.#charPos = charPos = new CharPosition(this.source, charPosNumber); + this._charPos = charPos = new CharPosition(this.source, charPosNumber); } } diff --git a/packages/@glimmer/syntax/lib/source/loc/span.ts b/packages/@glimmer/syntax/lib/source/loc/span.ts index e230fae344..ef4a351d06 100644 --- a/packages/@glimmer/syntax/lib/source/loc/span.ts +++ b/packages/@glimmer/syntax/lib/source/loc/span.ts @@ -313,7 +313,7 @@ type AnySpan = HbsSpan | CharPositionSpan | InvisibleSpan; class CharPositionSpan implements SpanData { readonly kind = OffsetKind.CharPosition; - #locPosSpan: HbsSpan | BROKEN | null = null; + _locPosSpan: HbsSpan | BROKEN | null = null; constructor( readonly source: Source, @@ -350,16 +350,16 @@ class CharPositionSpan implements SpanData { } toHbsSpan(): HbsSpan | null { - let locPosSpan = this.#locPosSpan; + let locPosSpan = this._locPosSpan; if (locPosSpan === null) { let start = this.charPositions.start.toHbsPos(); let end = this.charPositions.end.toHbsPos(); if (start === null || end === null) { - locPosSpan = this.#locPosSpan = BROKEN; + locPosSpan = this._locPosSpan = BROKEN; } else { - locPosSpan = this.#locPosSpan = new HbsSpan(this.source, { + locPosSpan = this._locPosSpan = new HbsSpan(this.source, { start, end, }); @@ -390,17 +390,17 @@ class CharPositionSpan implements SpanData { export class HbsSpan implements SpanData { readonly kind = OffsetKind.HbsPosition; - #charPosSpan: CharPositionSpan | BROKEN | null = null; + _charPosSpan: CharPositionSpan | BROKEN | null = null; // the source location from Handlebars + AST Plugins -- could be wrong - #providedHbsLoc: SourceLocation | null; + _providedHbsLoc: SourceLocation | null; constructor( readonly source: Source, readonly hbsPositions: { start: HbsPosition; end: HbsPosition }, providedHbsLoc: SourceLocation | null = null ) { - this.#providedHbsLoc = providedHbsLoc; + this._providedHbsLoc = providedHbsLoc; } serialize(): SerializedConcreteSourceSpan { @@ -413,13 +413,13 @@ export class HbsSpan implements SpanData { } private updateProvided(pos: SourcePosition, edge: 'start' | 'end') { - if (this.#providedHbsLoc) { - this.#providedHbsLoc[edge] = pos; + if (this._providedHbsLoc) { + this._providedHbsLoc[edge] = pos; } // invalidate computed character offsets - this.#charPosSpan = null; - this.#providedHbsLoc = { + this._charPosSpan = null; + this._providedHbsLoc = { start: pos, end: pos, }; @@ -466,19 +466,19 @@ export class HbsSpan implements SpanData { } toCharPosSpan(): CharPositionSpan | null { - let charPosSpan = this.#charPosSpan; + let charPosSpan = this._charPosSpan; if (charPosSpan === null) { let start = this.hbsPositions.start.toCharPos(); let end = this.hbsPositions.end.toCharPos(); if (start && end) { - charPosSpan = this.#charPosSpan = new CharPositionSpan(this.source, { + charPosSpan = this._charPosSpan = new CharPositionSpan(this.source, { start, end, }); } else { - charPosSpan = this.#charPosSpan = BROKEN; + charPosSpan = this._charPosSpan = BROKEN; return null; } } diff --git a/packages/@glimmer/syntax/lib/source/span-list.ts b/packages/@glimmer/syntax/lib/source/span-list.ts index 31d66f9087..f65cff1d59 100644 --- a/packages/@glimmer/syntax/lib/source/span-list.ts +++ b/packages/@glimmer/syntax/lib/source/span-list.ts @@ -16,22 +16,22 @@ export class SpanList { return new SpanList(span.map(loc)).getRangeOffset(fallback); } - #span: SourceSpan[]; + _span: SourceSpan[]; constructor(span: SourceSpan[] = []) { - this.#span = span; + this._span = span; } add(offset: SourceSpan): void { - this.#span.push(offset); + this._span.push(offset); } getRangeOffset(fallback: SourceSpan): SourceSpan { - if (this.#span.length === 0) { + if (this._span.length === 0) { return fallback; } else { - let first = this.#span[0]; - let last = this.#span[this.#span.length - 1]; + let first = this._span[0]; + let last = this._span[this._span.length - 1]; return first.extend(last); } diff --git a/packages/@glimmer/syntax/lib/symbol-table.ts b/packages/@glimmer/syntax/lib/symbol-table.ts index a5fce6039f..a692d9770e 100644 --- a/packages/@glimmer/syntax/lib/symbol-table.ts +++ b/packages/@glimmer/syntax/lib/symbol-table.ts @@ -47,18 +47,18 @@ export class ProgramSymbolTable extends SymbolTable { private blocks = dict(); private usedTemplateLocals: string[] = []; - #hasEval = false; + _hasEval = false; getUsedTemplateLocals(): string[] { return this.usedTemplateLocals; } setHasEval(): void { - this.#hasEval = true; + this._hasEval = true; } get hasEval(): boolean { - return this.#hasEval; + return this._hasEval; } has(name: string): boolean {