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]: emberjs/ember.js#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 <[email protected]>
glimmerjs · Sep 10, 2021 · 80eed34 · 80eed34
1 parent 56f5c1e
commit 80eed34
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 58 deletions.
diff --git 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<OptionalList<mir.Statement>> {
-    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
@@ -168,18 +168,18 @@ function getCalleeExpression(
 
 export class Keywords<K extends KeywordType, KeywordList extends Keyword<K> = never>
   implements Keyword<K, OutFor<KeywordList>> {
-  #keywords: Keyword[] = [];
-  #type: K;
+  _keywords: Keyword[] = [];
+  _type: K;
 
   constructor(type: K) {
-    this.#type = type;
+    this._type = type;
   }
 
   kw<S extends string = string, Out = unknown>(
     name: S,
     delegate: KeywordDelegate<KeywordMatches[K], unknown, Out>
   ): Keywords<K, KeywordList | Keyword<K, Out>> {
-    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<K extends KeywordType, KeywordList extends Keyword<K> = ne
     node: KeywordCandidates[K],
     state: NormalizationState
   ): Result<OutFor<KeywordList>> | null {
-    for (let keyword of this.#keywords) {
+    for (let keyword of this._keywords) {
       let result = keyword.translate(node, state) as Result<OutFor<KeywordList>>;
       if (result !== null) {
         return result;
@@ -200,7 +200,7 @@ export class Keywords<K extends KeywordType, KeywordList extends Keyword<K> = 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
@@ -28,14 +28,14 @@ export type IsInvisible = 'IS_INVISIBLE';
 type Pattern = OffsetKind | IsInvisible | MatchAny;
 
 class WhenList<Out> {
-  #whens: When<Out>[];
+  _whens: When<Out>[];
 
   constructor(whens: When<Out>[]) {
-    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,33 +47,33 @@ class WhenList<Out> {
 }
 
 class When<Out> {
-  #map: Map<Pattern, Out> = new Map();
+  _map: Map<Pattern, Out> = new Map();
 
   get(pattern: Pattern, or: () => Out): Out {
-    let value = this.#map.get(pattern);
+    let value = this._map.get(pattern);
 
     if (value) {
       return value;
     }
 
     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[] {
     let pattern = patternFor(kind);
 
     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<Out>(callback: (m: Matcher<Out>) => ExhaustiveMatcher<Out>
 }
 
 class Matcher<Out, M extends Matches = Matches> {
-  #whens: When<When<(left: PositionData, right: PositionData) => Out>> = new When();
+  _whens: When<When<(left: PositionData, right: PositionData) => Out>> = new When();
 
   /**
    * You didn't exhaustively match all possibilities.
@@ -118,7 +118,7 @@ class Matcher<Out, M extends Matches = Matches> {
     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<Out, M extends Matches = Matches> {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     callback: (left: any, right: any) => Out
   ): Matcher<Out, Matches> | ExhaustiveMatcher<Out> {
-    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
@@ -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
@@ -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
@@ -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
@@ -47,18 +47,18 @@ export class ProgramSymbolTable extends SymbolTable {
   private blocks = dict<number>();
   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 {