Skip to content

perf(compiler-cli): use null instead of {} in ngDevMode spread fallback #68828

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
arturovt wants to merge 1 commit into from
Closed

perf(compiler-cli): use null instead of {} in ngDevMode spread fallback #68828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions packages/compiler-cli/src/ngtsc/transform/src/implicit_signal_debug_name_transform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,22 @@ function insertDebugNameIntoCallExpression(

if (existingArg !== null) {
// If there's an existing object literal already, we transform it as follows:
// `signal(0, {equal})` becomes `signal(0, { ...(ngDevMode ? {debugName: "n"} : {}), equal })`.
// During minification the spread will be removed since it's pointing to an empty object.
// `signal(0, {equal})` becomes `signal(0, { ...(ngDevMode ? {debugName: "n"} : null), equal })`.
//
// The falsy branch is `null` rather than `{}` (ECMA-262 §13.2.5 makes `{...null}` a no-op):
// - `{}` allocates in V8's young generation via a bump-pointer allocator. Individual
// allocations are nanoseconds, but they raise the allocation rate and increase the
// frequency of scavenger (minor GC) pauses — directly on the startup critical path.
// - TurboFan's escape analysis could theoretically eliminate the allocation, but escape
// analysis is per-function. When `{}` is passed into `computed()`, TurboFan must inline
// `computed()` to prove it doesn't escape — not guaranteed at thousands of call sites.
// - `null` hits a fast path in V8's object-spread runtime: it checks the value, finds it
// nullish, and returns immediately — no heap allocation, no property iteration.
const transformedArg = ts.factory.createObjectLiteralExpression([
ts.factory.createSpreadAssignment(
createNgDevModeConditional(
ts.factory.createObjectLiteralExpression([debugNameProperty]),
ts.factory.createObjectLiteralExpression(),
ts.factory.createNull(),
),
),
...existingArg.properties,
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/test/compliance/test_cases/model_inputs/GOLDEN_PARTIAL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ import * as i0 from "@angular/core";
export class TestDir {
counter = model(0, /* @ts-ignore */
...(ngDevMode ? [{ debugName: "counter" }] : /* istanbul ignore next */ []));
modelWithAlias = model(false, { ...(ngDevMode ? { debugName: "modelWithAlias" } : /* istanbul ignore next */ {}), alias: 'alias' });
modelWithAlias = model(false, { ...(ngDevMode ? { debugName: "modelWithAlias" } : /* istanbul ignore next */ null), alias: 'alias' });
decoratorInput = true;
decoratorInputWithAlias = true;
decoratorOutput = new EventEmitter();
Expand Down
14 changes: 7 additions & 7 deletions packages/compiler-cli/test/compliance/test_cases/signal_inputs/GOLDEN_PARTIAL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ function convertToBoolean(value) {
export class TestDir {
counter = input(0, /* @ts-ignore */
...(ngDevMode ? [{ debugName: "counter" }] : /* istanbul ignore next */ []));
signalWithTransform = input(false, { ...(ngDevMode ? { debugName: "signalWithTransform" } : /* istanbul ignore next */ {}), transform: convertToBoolean });
signalWithTransformAndAlias = input(false, { ...(ngDevMode ? { debugName: "signalWithTransformAndAlias" } : /* istanbul ignore next */ {}), alias: 'publicNameSignal', transform: convertToBoolean });
signalWithTransform = input(false, { ...(ngDevMode ? { debugName: "signalWithTransform" } : /* istanbul ignore next */ null), transform: convertToBoolean });
signalWithTransformAndAlias = input(false, { ...(ngDevMode ? { debugName: "signalWithTransformAndAlias" } : /* istanbul ignore next */ null), alias: 'publicNameSignal', transform: convertToBoolean });
decoratorInput = true;
decoratorInputWithAlias = true;
decoratorInputWithTransformAndAlias = true;
Expand Down Expand Up @@ -115,7 +115,7 @@ function convertToBoolean(value) {
return value === true || value !== '';
}
export class TestDir {
name = input.required({ ...(ngDevMode ? { debugName: "name" } : /* istanbul ignore next */ {}), transform: convertToBoolean });
name = input.required({ ...(ngDevMode ? { debugName: "name" } : /* istanbul ignore next */ null), transform: convertToBoolean });
static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "17.1.0", version: "0.0.0-PLACEHOLDER", type: TestDir, isStandalone: true, inputs: { name: { classPropertyName: "name", publicName: "name", isSignal: true, isRequired: true, transformFunction: null } }, ngImport: i0 });
}
Expand Down Expand Up @@ -144,10 +144,10 @@ const toBoolean = (v) => v === true || v !== '';
// Note: `@Input` non-signal inputs did not support transform function "builders" and generics.
const complexTransform = (defaultVal) => (v) => v || defaultVal;
export class TestDir {
name = input.required({ ...(ngDevMode ? { debugName: "name" } : /* istanbul ignore next */ {}), transform: (v) => v === true || v !== '' });
name2 = input.required({ ...(ngDevMode ? { debugName: "name2" } : /* istanbul ignore next */ {}), transform: toBoolean });
genericTransform = input.required({ ...(ngDevMode ? { debugName: "genericTransform" } : /* istanbul ignore next */ {}), transform: complexTransform(1) });
genericTransform2 = input.required({ ...(ngDevMode ? { debugName: "genericTransform2" } : /* istanbul ignore next */ {}), transform: complexTransform(null) });
name = input.required({ ...(ngDevMode ? { debugName: "name" } : /* istanbul ignore next */ null), transform: (v) => v === true || v !== '' });
name2 = input.required({ ...(ngDevMode ? { debugName: "name2" } : /* istanbul ignore next */ null), transform: toBoolean });
genericTransform = input.required({ ...(ngDevMode ? { debugName: "genericTransform" } : /* istanbul ignore next */ null), transform: complexTransform(1) });
genericTransform2 = input.required({ ...(ngDevMode ? { debugName: "genericTransform2" } : /* istanbul ignore next */ null), transform: complexTransform(null) });
static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "17.1.0", version: "0.0.0-PLACEHOLDER", type: TestDir, isStandalone: true, inputs: { name: { classPropertyName: "name", publicName: "name", isSignal: true, isRequired: true, transformFunction: null }, name2: { classPropertyName: "name2", publicName: "name2", isSignal: true, isRequired: true, transformFunction: null }, genericTransform: { classPropertyName: "genericTransform", publicName: "genericTransform", isSignal: true, isRequired: true, transformFunction: null }, genericTransform2: { classPropertyName: "genericTransform2", publicName: "genericTransform2", isSignal: true, isRequired: true, transformFunction: null } }, ngImport: i0 });
}
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler-cli/test/compliance/test_cases/signal_queries/GOLDEN_PARTIAL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ export class TestDir {
...(ngDevMode ? [{ debugName: "query5" }] : /* istanbul ignore next */ []));
query6 = viewChildren(SomeToken, /* @ts-ignore */
...(ngDevMode ? [{ debugName: "query6" }] : /* istanbul ignore next */ []));
query7 = viewChild('locatorE', { ...(ngDevMode ? { debugName: "query7" } : /* istanbul ignore next */ {}), read: SomeToken });
query8 = contentChildren('locatorF, locatorG', { ...(ngDevMode ? { debugName: "query8" } : /* istanbul ignore next */ {}), descendants: true });
query9 = contentChildren(nonAnalyzableRefersToString, { ...(ngDevMode ? { debugName: "query9" } : /* istanbul ignore next */ {}), descendants: true });
query7 = viewChild('locatorE', { ...(ngDevMode ? { debugName: "query7" } : /* istanbul ignore next */ null), read: SomeToken });
query8 = contentChildren('locatorF, locatorG', { ...(ngDevMode ? { debugName: "query8" } : /* istanbul ignore next */ null), descendants: true });
query9 = contentChildren(nonAnalyzableRefersToString, { ...(ngDevMode ? { debugName: "query9" } : /* istanbul ignore next */ null), descendants: true });
static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "17.2.0", version: "0.0.0-PLACEHOLDER", type: TestDir, isStandalone: true, queries: [{ propertyName: "query3", first: true, predicate: ["locatorC"], descendants: true, isSignal: true }, { propertyName: "query4", predicate: ["locatorD"], isSignal: true }, { propertyName: "query8", predicate: ["locatorF, locatorG"], descendants: true, isSignal: true }, { propertyName: "query9", predicate: nonAnalyzableRefersToString, descendants: true, isSignal: true }], viewQueries: [{ propertyName: "query1", first: true, predicate: ["locatorA"], descendants: true, isSignal: true }, { propertyName: "query2", predicate: ["locatorB"], descendants: true, isSignal: true }, { propertyName: "query5", first: true, predicate: i0.forwardRef(() => SomeToken), descendants: true, isSignal: true }, { propertyName: "query6", predicate: SomeToken, descendants: true, isSignal: true }, { propertyName: "query7", first: true, predicate: ["locatorE"], descendants: true, read: SomeToken, isSignal: true }], ngImport: i0 });
}
Expand Down
34 changes: 17 additions & 17 deletions packages/compiler-cli/test/ngtsc/debug_transform_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ runInEachFileSystem(() => {

const jsContents = env.getContents('test.js');
expect(cleanNewLines(jsContents)).toContain(
`signal('Hello World', { ...(ngDevMode ? { debugName: "testSignal" } : /* istanbul ignore next */ {}), equal: () => true })`,
`signal('Hello World', { ...(ngDevMode ? { debugName: "testSignal" } : /* istanbul ignore next */ null), equal: () => true })`,
);
});

Expand Down Expand Up @@ -229,7 +229,7 @@ runInEachFileSystem(() => {

const jsContents = env.getContents('test.js');
expect(cleanNewLines(jsContents)).toContain(
`signal('Hello World', { ...(ngDevMode ? { debugName: "testSignal" } : /* istanbul ignore next */ {}), equal: () => true })`,
`signal('Hello World', { ...(ngDevMode ? { debugName: "testSignal" } : /* istanbul ignore next */ null), equal: () => true })`,
);
});

Expand Down Expand Up @@ -357,7 +357,7 @@ runInEachFileSystem(() => {

const jsContents = env.getContents('test.js');
expect(cleanNewLines(jsContents)).toContain(
`signal('Hello World', { ...(ngDevMode ? { debugName: "testSignal" } : /* istanbul ignore next */ {}), equal: () => true })`,
`signal('Hello World', { ...(ngDevMode ? { debugName: "testSignal" } : /* istanbul ignore next */ null), equal: () => true })`,
);
});

Expand Down Expand Up @@ -468,7 +468,7 @@ runInEachFileSystem(() => {

const jsContents = env.getContents('test.js');
expect(cleanNewLines(jsContents)).toContain(
`computed(() => testSignal(), { ...(ngDevMode ? { debugName: "testComputed" } : /* istanbul ignore next */ {}), equal: () => true })`,
`computed(() => testSignal(), { ...(ngDevMode ? { debugName: "testComputed" } : /* istanbul ignore next */ null), equal: () => true })`,
);
});

Expand Down Expand Up @@ -577,7 +577,7 @@ runInEachFileSystem(() => {

const jsContents = env.getContents('test.js');
expect(cleanNewLines(jsContents)).toContain(
`computed(() => this.testSignal(), { ...(ngDevMode ? { debugName: "testComputed" } : /* istanbul ignore next */ {}), equal: () => true })`,
`computed(() => this.testSignal(), { ...(ngDevMode ? { debugName: "testComputed" } : /* istanbul ignore next */ null), equal: () => true })`,
);
});

Expand Down Expand Up @@ -711,7 +711,7 @@ runInEachFileSystem(() => {

const jsContents = env.getContents('test.js');
expect(cleanNewLines(jsContents)).toContain(
`computed(() => this.testSignal(), { ...(ngDevMode ? { debugName: "testComputed" } : /* istanbul ignore next */ {}), equal: () => true })`,
`computed(() => this.testSignal(), { ...(ngDevMode ? { debugName: "testComputed" } : /* istanbul ignore next */ null), equal: () => true })`,
);
});

Expand Down Expand Up @@ -878,7 +878,7 @@ runInEachFileSystem(() => {

const jsContents = env.getContents('test.js');
expect(cleanNewLines(jsContents)).toContain(
`model.required({ ...(ngDevMode ? { debugName: "testModel" } : /* istanbul ignore next */ {}), alias: 'testModelAlias' })`,
`model.required({ ...(ngDevMode ? { debugName: "testModel" } : /* istanbul ignore next */ null), alias: 'testModelAlias' })`,
);
});

Expand Down Expand Up @@ -1068,7 +1068,7 @@ runInEachFileSystem(() => {

const jsContents = env.getContents('test.js');
expect(cleanNewLines(jsContents)).toContain(
`input.required({ ...(ngDevMode ? { debugName: "testInput" } : /* istanbul ignore next */ {}), alias: 'testInputAlias' })`,
`input.required({ ...(ngDevMode ? { debugName: "testInput" } : /* istanbul ignore next */ null), alias: 'testInputAlias' })`,
);
});

Expand Down Expand Up @@ -1920,7 +1920,7 @@ runInEachFileSystem(() => {

const jsContents = env.getContents('test.js');
expect(cleanNewLines(jsContents)).toContain(
`linkedSignal(() => testSignal(), { ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ {}), equal: () => true })`,
`linkedSignal(() => testSignal(), { ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ null), equal: () => true })`,
);
});

Expand Down Expand Up @@ -1982,7 +1982,7 @@ runInEachFileSystem(() => {

const jsContents = cleanNewLines(env.getContents('test.js'));
expect(jsContents).toContain(
'testLinkedSignal = linkedSignal({ ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ {}), ' +
'testLinkedSignal = linkedSignal({ ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ null), ' +
'source: testSignal, ' +
'computation: (src, prev) => src ' +
'})',
Expand Down Expand Up @@ -2106,7 +2106,7 @@ runInEachFileSystem(() => {

const jsContents = env.getContents('test.js');
expect(cleanNewLines(jsContents)).toContain(
`linkedSignal(() => this.testSignal(), { ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ {}), equal: () => true })`,
`linkedSignal(() => this.testSignal(), { ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ null), equal: () => true })`,
);
});

Expand Down Expand Up @@ -2181,7 +2181,7 @@ runInEachFileSystem(() => {

const jsContents = cleanNewLines(env.getContents('test.js'));
expect(jsContents).toContain(
'linkedSignal({ ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ {}), ' +
'linkedSignal({ ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ null), ' +
'source: this.testSignal, ' +
'computation: (src, prev) => src ' +
'})',
Expand Down Expand Up @@ -2328,7 +2328,7 @@ runInEachFileSystem(() => {

const jsContents = env.getContents('test.js');
expect(cleanNewLines(jsContents)).toContain(
`linkedSignal(() => this.testSignal(), { ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ {}), equal: () => true })`,
`linkedSignal(() => this.testSignal(), { ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ null), equal: () => true })`,
);
});

Expand Down Expand Up @@ -2418,7 +2418,7 @@ runInEachFileSystem(() => {

const jsContents = cleanNewLines(env.getContents('test.js'));
expect(jsContents).toContain(
'linkedSignal({ ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ {}), ' +
'linkedSignal({ ...(ngDevMode ? { debugName: "testLinkedSignal" } : /* istanbul ignore next */ null), ' +
'source: this.testSignal, ' +
'computation: (src, prev) => src ' +
'})',
Expand Down Expand Up @@ -2533,7 +2533,7 @@ runInEachFileSystem(() => {
const jsContents = cleanNewLines(env.getContents('test.js'));
expect(jsContents).toContain(
'resource({ ' +
'...(ngDevMode ? { debugName: "testResource" } : /* istanbul ignore next */ {}), ' +
'...(ngDevMode ? { debugName: "testResource" } : /* istanbul ignore next */ null), ' +
`defaultValue: 'foo', ` +
`loader: async () => 'bar' ` +
'})',
Expand Down Expand Up @@ -2611,7 +2611,7 @@ runInEachFileSystem(() => {
const jsContents = cleanNewLines(env.getContents('test.js'));
expect(jsContents).toContain(
'resource({ ' +
'...(ngDevMode ? { debugName: "testResource" } : /* istanbul ignore next */ {}), ' +
'...(ngDevMode ? { debugName: "testResource" } : /* istanbul ignore next */ null), ' +
`defaultValue: 'foo', ` +
`loader: async () => 'bar' ` +
'})',
Expand Down Expand Up @@ -2702,7 +2702,7 @@ runInEachFileSystem(() => {
const jsContents = cleanNewLines(env.getContents('test.js'));
expect(jsContents).toContain(
'resource({ ' +
'...(ngDevMode ? { debugName: "testResource" } : /* istanbul ignore next */ {}), ' +
'...(ngDevMode ? { debugName: "testResource" } : /* istanbul ignore next */ null), ' +
`defaultValue: 'foo', ` +
`loader: async () => 'bar' ` +
'})',
Expand Down
Loading