From 11659c12d15aec8702819ab91a77ded53db04f5a Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Mon, 27 Jul 2020 14:48:22 -0700 Subject: [PATCH] [Refactor] Use a map to dedupe constants instead of indexOf This PR updates the constants pool to use a Map for deduping rather than `indexOf`, which can get very expensive in larger applications. It also caches reified string arrays, so we don't re-reify for every component invocation, and refactors arguments so that we don't mutate those arrays when currying. --- .../bundle-compiler/lib/debug-constants.ts | 10 +- .../lib/suites/bundle-compiler.ts | 18 +-- packages/@glimmer/interfaces/lib/program.d.ts | 7 +- packages/@glimmer/program/lib/constants.ts | 147 ++++++------------ packages/@glimmer/runtime/lib/vm/arguments.ts | 11 +- 5 files changed, 70 insertions(+), 123 deletions(-) diff --git a/packages/@glimmer/bundle-compiler/lib/debug-constants.ts b/packages/@glimmer/bundle-compiler/lib/debug-constants.ts index 365ab772a1..e51975a473 100644 --- a/packages/@glimmer/bundle-compiler/lib/debug-constants.ts +++ b/packages/@glimmer/bundle-compiler/lib/debug-constants.ts @@ -4,11 +4,11 @@ import { RuntimeConstants } from '@glimmer/interfaces'; export default class DebugConstants extends WriteOnlyConstants implements RuntimeConstants { getNumber(value: number): number { - return this.numbers[value]; + return this.values[value] as number; } getString(value: number): string { - return this.strings[value]; + return this.values[value] as string; } getStringArray(value: number): string[] { @@ -24,7 +24,7 @@ export default class DebugConstants extends WriteOnlyConstants implements Runtim } getArray(value: number): number[] { - return (this.arrays as number[][])[value]; + return this.values[value] as number[]; } resolveHandle(s: number): T { @@ -33,7 +33,7 @@ export default class DebugConstants extends WriteOnlyConstants implements Runtim } getSerializable(s: number): unknown { - return JSON.parse(this.strings[s]); + return JSON.parse(this.values[s] as string); } getTemplateMeta(m: number): unknown { @@ -41,6 +41,6 @@ export default class DebugConstants extends WriteOnlyConstants implements Runtim } getOther(s: number): unknown { - return this.others[s]; + return this.values[s]; } } diff --git a/packages/@glimmer/integration-tests/lib/suites/bundle-compiler.ts b/packages/@glimmer/integration-tests/lib/suites/bundle-compiler.ts index 3b5e8ed2ed..81edf68103 100644 --- a/packages/@glimmer/integration-tests/lib/suites/bundle-compiler.ts +++ b/packages/@glimmer/integration-tests/lib/suites/bundle-compiler.ts @@ -20,9 +20,9 @@ export class BundleCompilerEmberTests extends EmberishComponentTests { module: 'ui/components/main', name: 'default', }); - let { strings } = this.delegate.getConstants(); - this.assert.equal(strings.indexOf(ALocator), -1); - this.assert.equal(strings.indexOf(MainLocator), -1); + let values = this.delegate.getConstants(); + this.assert.equal(values.indexOf(ALocator), -1); + this.assert.equal(values.indexOf(MainLocator), -1); this.assertHTML('B 1 B 2 B 3 B 4'); this.assertStableRerender(); } @@ -39,9 +39,9 @@ export class BundleCompilerEmberTests extends EmberishComponentTests { let MainLocator = JSON.stringify({ locator: { module: 'ui/components/main', name: 'default' }, }); - let { strings } = this.delegate.constants!.toPool(); - this.assert.equal(strings.indexOf(ALocator), -1); - this.assert.equal(strings.indexOf(MainLocator), -1); + let values = this.delegate.constants!.toPool(); + this.assert.equal(values.indexOf(ALocator), -1); + this.assert.equal(values.indexOf(MainLocator), -1); this.assertHTML('B 1 B 1'); this.assertStableRerender(); } @@ -56,9 +56,9 @@ export class BundleCompilerEmberTests extends EmberishComponentTests { module: 'ui/components/main', name: 'default', }); - let { strings } = this.delegate.constants!.toPool(); - this.assert.ok(strings.indexOf(ALocator) > -1, 'Has locator for "A"'); - this.assert.equal(strings.indexOf(MainLocator), -1); + let values = this.delegate.constants!.toPool(); + this.assert.ok(values.indexOf(ALocator) > -1, 'Has locator for "A"'); + this.assert.equal(values.indexOf(MainLocator), -1); this.assertHTML('B 1'); this.assertStableRerender(); } diff --git a/packages/@glimmer/interfaces/lib/program.d.ts b/packages/@glimmer/interfaces/lib/program.d.ts index e2d19a9bd8..dc0767701f 100644 --- a/packages/@glimmer/interfaces/lib/program.d.ts +++ b/packages/@glimmer/interfaces/lib/program.d.ts @@ -85,12 +85,7 @@ export interface TemplateCompilationContext { export type EMPTY_ARRAY = Array>; -export interface ConstantPool { - strings: string[]; - arrays: number[][] | EMPTY_ARRAY; - handles: number[]; - numbers: number[]; -} +export type ConstantPool = unknown[]; /** * Constants are interned values that are referenced as numbers in the program. diff --git a/packages/@glimmer/program/lib/constants.ts b/packages/@glimmer/program/lib/constants.ts index 9636e8966d..ac8a267e0f 100644 --- a/packages/@glimmer/program/lib/constants.ts +++ b/packages/@glimmer/program/lib/constants.ts @@ -1,12 +1,4 @@ -import { - SymbolTable, - CompileTimeConstants, - EMPTY_ARRAY, - ConstantPool, - RuntimeConstants, -} from '@glimmer/interfaces'; - -const UNRESOLVED = {}; +import { CompileTimeConstants, ConstantPool, RuntimeConstants } from '@glimmer/interfaces'; export const WELL_KNOWN_EMPTY_ARRAY_POSITION = 0; const WELL_KNOW_EMPTY_ARRAY = Object.freeze([]); @@ -14,26 +6,27 @@ const WELL_KNOW_EMPTY_ARRAY = Object.freeze([]); export class WriteOnlyConstants implements CompileTimeConstants { // `0` means NULL - protected strings: string[] = []; - protected arrays: number[][] | EMPTY_ARRAY = [WELL_KNOW_EMPTY_ARRAY]; - protected tables: SymbolTable[] = []; - protected handles: number[] = []; - protected resolved: unknown[] = []; - protected numbers: number[] = []; - protected others: unknown[] = []; + protected values: unknown[] = [WELL_KNOW_EMPTY_ARRAY]; + protected indexMap: Map = new Map(); + + protected value(value: unknown) { + let indexMap = this.indexMap; + let index = indexMap.get(value); + + if (index === undefined) { + index = this.values.push(value) - 1; + indexMap.set(value, index); + } + + return index; + } other(other: unknown): number { - return this.others.push(other) - 1; + return this.value(other); } string(value: string): number { - let index = this.strings.indexOf(value); - - if (index > -1) { - return index; - } - - return this.strings.push(value) - 1; + return this.value(value); } stringArray(strings: string[]): number { @@ -51,23 +44,13 @@ export class WriteOnlyConstants implements CompileTimeConstants { return WELL_KNOWN_EMPTY_ARRAY_POSITION; } - let index = (this.arrays as number[][]).indexOf(values); - - if (index > -1) { - return index; - } - - return (this.arrays as number[][]).push(values) - 1; + return this.value(values); } serializable(value: unknown): number { let str = JSON.stringify(value); - let index = this.strings.indexOf(str); - if (index > -1) { - return index; - } - return this.strings.push(str) - 1; + return this.value(str); } templateMeta(value: unknown): number { @@ -75,46 +58,27 @@ export class WriteOnlyConstants implements CompileTimeConstants { } number(number: number): number { - let index = this.numbers.indexOf(number); - - if (index > -1) { - return index; - } - - return this.numbers.push(number) - 1; + return this.value(number); } toPool(): ConstantPool { - return { - strings: this.strings, - arrays: this.arrays, - handles: this.handles, - numbers: this.numbers, - }; + return this.values; } } export class RuntimeConstantsImpl implements RuntimeConstants { - protected strings: string[]; - protected arrays: number[][] | EMPTY_ARRAY; - protected handles: number[]; - protected numbers: number[]; - protected others: unknown[]; + protected values: unknown[]; constructor(pool: ConstantPool) { - this.strings = pool.strings; - this.arrays = pool.arrays; - this.handles = pool.handles; - this.numbers = pool.numbers; - this.others = []; + this.values = pool; } getString(value: number): string { - return this.strings[value]; + return this.values[value] as string; } getNumber(value: number): number { - return this.numbers[value]; + return this.values[value] as number; } getStringArray(value: number): string[] { @@ -130,11 +94,11 @@ export class RuntimeConstantsImpl implements RuntimeConstants { } getArray(value: number): number[] { - return (this.arrays as number[][])[value]; + return this.values[value] as number[]; } getSerializable(s: number): T { - return JSON.parse(this.strings[s]) as T; + return JSON.parse(this.values[s] as string) as T; } getTemplateMeta(m: number): T { @@ -142,69 +106,60 @@ export class RuntimeConstantsImpl implements RuntimeConstants { } getOther(value: number): T { - return this.others[value] as T; + return this.values[value] as T; } } export class JitConstants extends WriteOnlyConstants implements RuntimeConstants { - protected metas: unknown[] = []; - - constructor(pool?: ConstantPool) { - super(); - - if (pool) { - this.strings = pool.strings; - this.arrays = pool.arrays; - this.handles = pool.handles; - this.resolved = this.handles.map(() => UNRESOLVED); - this.numbers = pool.numbers; - } - - this.others = []; - } + protected reifiedStringArrs: string[][] = [WELL_KNOW_EMPTY_ARRAY as any]; templateMeta(meta: unknown): number { - let index = this.metas.indexOf(meta); - if (index > -1) { - return index; - } + return this.value(meta); + } - return this.metas.push(meta) - 1; + getValue(index: number) { + return this.values[index] as T; } getNumber(value: number): number { - return this.numbers[value]; + return this.getValue(value); } getString(value: number): string { - return this.strings[value]; + return this.getValue(value); } getStringArray(value: number): string[] { - let names = this.getArray(value); - let _names: string[] = new Array(names.length); + let reifiedStringArrs = this.reifiedStringArrs; + let reified = reifiedStringArrs[value]; - for (let i = 0; i < names.length; i++) { - let n = names[i]; - _names[i] = this.getString(n); + if (reified === undefined) { + let names = this.getArray(value); + reified = new Array(names.length); + + for (let i = 0; i < names.length; i++) { + reified[i] = this.getValue(names[i]); + } + + reifiedStringArrs[value] = reified; } - return _names; + return reified; } getArray(value: number): number[] { - return (this.arrays as number[][])[value]; + return this.getValue(value); } getSerializable(s: number): T { - return JSON.parse(this.strings[s]) as T; + return JSON.parse(this.getValue(s)) as T; } getTemplateMeta(m: number): T { - return this.metas[m] as T; + return this.getValue(m); } getOther(value: number): T { - return this.others[value] as T; + return this.getValue(value); } } diff --git a/packages/@glimmer/runtime/lib/vm/arguments.ts b/packages/@glimmer/runtime/lib/vm/arguments.ts index 976646e294..f701760d51 100644 --- a/packages/@glimmer/runtime/lib/vm/arguments.ts +++ b/packages/@glimmer/runtime/lib/vm/arguments.ts @@ -360,24 +360,21 @@ export class NamedArgumentsImpl implements NamedArguments { if (extras > 0) { let { names, length, stack } = this; let { names: extraNames } = other; - - if (Object.isFrozen(names) && names.length === 0) { - names = []; - } + let newNames = names.slice(); for (let i = 0; i < extras; i++) { let name = extraNames[i]; - let idx = names.indexOf(name); + let idx = newNames.indexOf(name); if (idx === -1) { - length = names.push(name); + length = newNames.push(name); stack.push(other.references[i]); } } this.length = length; this._references = null; - this._names = names; + this._names = newNames; this._atNames = null; } }