Skip to content

Commit 386464f

Browse files
NullVoxPopuliNullVoxPopuli
authored
Merge pull request #21571 from emberjs/nvp/perf/destroyable-slot-widen
improve batch rendering performance by moving VM blocks out of the DESTRYABLE_META
2 parents 182ae47 + 9a9b36c commit 386464f

10 files changed

Lines changed: 104 additions & 18 deletions

File tree

‎package.json‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "ember-source",
3-
"version": "7.4.0-alpha.1",
3+
"version": "7.4.0-alpha.1.nvp-perf-destroyable-slot-widen+b92b228d",
44
"description": "A JavaScript framework for creating ambitious web applications",
55
"keywords": [
66
"ember-addon"
@@ -374,5 +374,10 @@
374374
"router_js/index.js": "ember-source/router_js/index.js",
375375
"rsvp/index.js": "ember-source/rsvp/index.js"
376376
}
377+
},
378+
"_originalVersion": "7.4.0-alpha.1",
379+
"_versionPreviouslyCalculated": true,
380+
"publishConfig": {
381+
"tag": "alpha"
377382
}
378383
}

‎packages/@glimmer/destroyable/index.ts‎

Lines changed: 57 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@ import { DEBUG } from '@glimmer/env';
22
import type { Destroyable, Destructor } from '@glimmer/interfaces';
33
import debugToString from '@glimmer/debug-util/lib/debug-to-string';
44
import { scheduleDestroy, scheduleDestroyed } from '@glimmer/global-context';
5+
import {
6+
DESTROYABLE_META_KEY,
7+
type HasDestroyableMetaSlot,
8+
} from '@glimmer/util/lib/destroyable-key';
9+
10+
export { DESTROYABLE_META_KEY } from '@glimmer/util/lib/destroyable-key';
511

612
const LIVE_STATE = 0;
713
const DESTROYING_STATE = 1;
@@ -78,22 +84,45 @@ function remove<T extends object>(collection: OneOrMany<T>, item: T, message: st
7884
}
7985
}
8086

87+
function createMeta<T extends Destroyable>(destroyable: T): DestroyableMeta<T> {
88+
let meta: DestroyableMeta<Destroyable> = {
89+
parents: null,
90+
children: null,
91+
eagerDestructors: null,
92+
destructors: null,
93+
state: LIVE_STATE,
94+
};
95+
96+
if (DEBUG) {
97+
meta.source = destroyable;
98+
}
99+
100+
return meta as unknown as DestroyableMeta<T>;
101+
}
102+
81103
function getDestroyableMeta<T extends Destroyable>(destroyable: T): DestroyableMeta<T> {
82-
let meta = DESTROYABLE_META.get(destroyable);
104+
let slotted = destroyable as HasDestroyableMetaSlot;
105+
let own = slotted[DESTROYABLE_META_KEY];
83106

84-
if (meta === undefined) {
85-
meta = {
86-
parents: null,
87-
children: null,
88-
eagerDestructors: null,
89-
destructors: null,
90-
state: LIVE_STATE,
91-
};
92-
93-
if (DEBUG) {
94-
meta.source = destroyable;
107+
if (own !== undefined) return own as DestroyableMeta<T>;
108+
109+
// `in` rather than a write, so this stays a read for everything else.
110+
if (DESTROYABLE_META_KEY in slotted) {
111+
let meta = createMeta(destroyable);
112+
113+
slotted[DESTROYABLE_META_KEY] = meta;
114+
115+
if (DEBUG && DESTROYABLE_META instanceof Map) {
116+
DESTROYABLE_META.set(destroyable, meta as unknown as DestroyableMeta<Destroyable>);
95117
}
96118

119+
return meta;
120+
}
121+
122+
let meta = DESTROYABLE_META.get(destroyable);
123+
124+
if (meta === undefined) {
125+
meta = createMeta(destroyable) as unknown as DestroyableMeta<Destroyable>;
97126
DESTROYABLE_META.set(destroyable, meta);
98127
}
99128

@@ -209,20 +238,33 @@ export function destroyChildren(destroyable: Destroyable) {
209238
iterate(children, destroy);
210239
}
211240

241+
/** Meta if there is any, without creating it. Mirrors `getDestroyableMeta`. */
242+
function peekDestroyableMeta(destroyable: Destroyable): DestroyableMeta<Destroyable> | undefined {
243+
let slotted = destroyable as HasDestroyableMetaSlot;
244+
let own = slotted[DESTROYABLE_META_KEY];
245+
246+
if (own !== undefined) return own as DestroyableMeta<Destroyable>;
247+
248+
// An empty slot is proof there is no meta, so the map can be skipped.
249+
if (DESTROYABLE_META_KEY in slotted) return undefined;
250+
251+
return DESTROYABLE_META.get(destroyable);
252+
}
253+
212254
export function _hasDestroyableChildren(destroyable: Destroyable) {
213-
let meta = DESTROYABLE_META.get(destroyable);
255+
let meta = peekDestroyableMeta(destroyable);
214256

215257
return meta === undefined ? false : meta.children !== null;
216258
}
217259

218260
export function isDestroying(destroyable: Destroyable) {
219-
let meta = DESTROYABLE_META.get(destroyable);
261+
let meta = peekDestroyableMeta(destroyable);
220262

221263
return meta === undefined ? false : meta.state >= DESTROYING_STATE;
222264
}
223265

224266
export function isDestroyed(destroyable: Destroyable) {
225-
let meta = DESTROYABLE_META.get(destroyable);
267+
let meta = peekDestroyableMeta(destroyable);
226268

227269
return meta === undefined ? false : meta.state >= DESTROYED_STATE;
228270
}

‎packages/@glimmer/destroyable/package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@
3939
},
4040
"dependencies": {
4141
"@glimmer/global-context": "workspace:*",
42-
"@glimmer/interfaces": "workspace:*"
42+
"@glimmer/interfaces": "workspace:*",
43+
"@glimmer/util": "workspace:*"
4344
},
4445
"devDependencies": {
4546
"@ember/runloop": "workspace:*",

‎packages/@glimmer/reference/lib/reference.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {
1212
import type { Revision } from '@glimmer/validator/lib/validators';
1313
import type { Tag } from '@glimmer/interfaces';
1414
import { expect } from '@glimmer/debug-util/lib/platform-utils';
15+
import { DESTROYABLE_META_KEY } from '@glimmer/util/lib/destroyable-key';
1516
import { getProp, setProp } from '@glimmer/global-context';
1617
import { isDict } from '@glimmer/util/lib/collections';
1718
import { CONSTANT_TAG, INITIAL, validateTag, valueForTag } from '@glimmer/validator/lib/validators';
@@ -35,6 +36,7 @@ export interface ReferenceEnvironment {
3536
}
3637

3738
class ReferenceImpl<T = unknown> implements Reference<T> {
39+
[DESTROYABLE_META_KEY]: object | undefined;
3840
[REFERENCE]: ReferenceType;
3941
public tag: Nullable<Tag> = null;
4042
public lastRevision: Revision = INITIAL;

‎packages/@glimmer/runtime/lib/vm/append.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import type { MachineRegister, Register, SyscallRegister } from '@glimmer/vm/lib
2626
import { dev, expect } from '@glimmer/debug-util/lib/platform-utils';
2727
import { unwrapHandle } from '@glimmer/debug-util/lib/template';
2828
import { associateDestroyableChild } from '@glimmer/destroyable';
29+
import { DESTROYABLE_META_KEY } from '@glimmer/util/lib/destroyable-key';
2930
import { assertGlobalContextWasSet } from '@glimmer/global-context';
3031
import { LOCAL_DEBUG, LOCAL_TRACE_LOGGING } from '@glimmer/local-debug-flags';
3132
import { createIteratorItemRef } from '@glimmer/reference/lib/iterable';
@@ -54,9 +55,18 @@ import RenderResultImpl from './render-result';
5455
import EvaluationStackImpl from './stack';
5556
import { ListBlockOpcode, ListItemOpcode, TryOpcode } from './update';
5657

58+
/*
59+
* The VM's own root destroyable, on the fast path like the block opcodes.
60+
*
61+
* Introduction and background in https://github.com/emberjs/ember.js/pull/21571
62+
*/
63+
class Drop {
64+
[DESTROYABLE_META_KEY]: object | undefined;
65+
}
66+
5767
class Stacks {
5868
declare debug?: () => DebugStacks;
59-
readonly drop: object = {};
69+
readonly drop: object = new Drop();
6070

6171
readonly scope = new Stack<Scope>();
6272
readonly dynamicScope = new Stack<DynamicScope>();

‎packages/@glimmer/runtime/lib/vm/element-builder.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { expect } from '@glimmer/debug-util/lib/platform-utils';
2222
import assert from '@glimmer/debug-util/lib/assert';
2323
import { setLocalDebugType } from '@glimmer/debug-util/lib/debug-brand';
2424
import { destroy, registerDestructor } from '@glimmer/destroyable';
25+
import { DESTROYABLE_META_KEY } from '@glimmer/util/lib/destroyable-key';
2526
import { LOCAL_DEBUG } from '@glimmer/local-debug-flags';
2627
import { StackImpl as Stack } from '@glimmer/util/lib/collections';
2728

@@ -398,6 +399,8 @@ export class NewTreeBuilder implements TreeBuilder {
398399
export class AppendingBlockImpl implements AppendingBlock {
399400
declare debug?: { first: () => Nullable<SimpleNode>; last: () => Nullable<SimpleNode> };
400401

402+
[DESTROYABLE_META_KEY]: object | undefined;
403+
401404
protected first: Nullable<FirstNode> = null;
402405
protected last: Nullable<LastNode> = null;
403406
protected nesting = 0;

‎packages/@glimmer/runtime/lib/vm/render-result.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,14 @@ import type {
88
} from '@glimmer/interfaces';
99
import { unreachable } from '@glimmer/debug-util/lib/platform-utils';
1010
import { associateDestroyableChild, registerDestructor } from '@glimmer/destroyable';
11+
import { DESTROYABLE_META_KEY } from '@glimmer/util/lib/destroyable-key';
1112

1213
import { clear } from '../bounds';
1314
import { UpdatingVM } from './update';
1415

1516
export default class RenderResultImpl implements RenderResult {
17+
[DESTROYABLE_META_KEY]: object | undefined;
18+
1619
constructor(
1720
public env: Environment,
1821
private updating: UpdatingOpcode[],

‎packages/@glimmer/runtime/lib/vm/update.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type { OpaqueIterationItem, OpaqueIterator } from '@glimmer/reference/lib
1818
import type { Reference } from '@glimmer/reference/lib/reference';
1919
import { expect, unwrap } from '@glimmer/debug-util/lib/platform-utils';
2020
import { associateDestroyableChild, destroy, destroyChildren } from '@glimmer/destroyable';
21+
import { DESTROYABLE_META_KEY } from '@glimmer/util/lib/destroyable-key';
2122
import { LOCAL_DEBUG } from '@glimmer/local-debug-flags';
2223
import { updateRef, valueForRef } from '@glimmer/reference/lib/reference';
2324
import { logStep } from '@glimmer/util/lib/debug-steps';
@@ -111,6 +112,8 @@ export interface VMState {
111112
}
112113

113114
export abstract class BlockOpcode implements UpdatingOpcode, Bounds {
115+
[DESTROYABLE_META_KEY]: object | undefined;
116+
114117
public children: UpdatingOpcode[];
115118

116119
protected readonly bounds: AppendingBlock;

‎packages/@glimmer/util/lib/destroyable-key.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/**
2+
* Classes Glimmer constructs itself declare this slot, so that
3+
* `@glimmer/destroyable` can hold their meta directly rather than in its
4+
* `WeakMap`. It lives here so packages below `@glimmer/destroyable` can declare
5+
* the slot without depending on it.
6+
*
7+
* Never write this onto an object Glimmer did not construct. Destroyables are
8+
* public through `@ember/destroyable` and can be any object a user hands in.
9+
*/
10+
export const DESTROYABLE_META_KEY = Symbol('__destroyable_meta__');
11+
12+
export interface HasDestroyableMetaSlot {
13+
[DESTROYABLE_META_KEY]?: object | undefined;
14+
}

‎pnpm-lock.yaml‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)