Skip to content

Commit a966ff1

Browse files
JoostKthePunderWoman
authored andcommitted
refactor(core): let the profiler handle asymmetric events leniently
Although the prior commit has made more profiler events guaranteed symmetric through the use of finally-blocks, there continue to be some situations that could potentially result in asymmetric events, e.g. application bootstrap doesn't guarantee symmetric events. This commit makes the profiler lenient to these situations by unrolling the stack past the asymmetric event data, eventually reaching the expected start event. (cherry picked from commit da9911f)
1 parent 52cf658 commit a966ff1

File tree

2 files changed

+32
-9
lines changed

2 files changed

+32
-9
lines changed

packages/core/src/render3/debug/chrome_dev_tools_performance.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,14 @@ function measureEnd(
7171
entryName: string,
7272
color: DevToolsColor,
7373
) {
74-
const top = eventsStack.pop();
75-
76-
assertDefined(top, 'Profiling error: could not find start event entry ' + startEvent);
77-
assertEqual(
78-
top[0],
79-
startEvent,
80-
`Profiling error: expected to see ${startEvent} event but got ${top[0]}`,
81-
);
74+
let top: stackEntry | undefined;
75+
76+
// The stack may be asymmetric when an end event for a prior start event is missing (e.g. when an exception
77+
// has occurred), unroll the stack until a matching item has been found in that case.
78+
do {
79+
top = eventsStack.pop();
80+
assertDefined(top, 'Profiling error: could not find start event entry ' + startEvent);
81+
} while (top[0] !== startEvent);
8282

8383
console.timeStamp(
8484
entryName,

packages/core/test/acceptance/chrome_dev_tools_performance_spec.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {enableProfiling} from '../../src/render3/debug/chrome_dev_tools_performance';
109
import {Component, HostAttributeToken, inject, Inject} from '../../src/core';
10+
import {enableProfiling} from '../../src/render3/debug/chrome_dev_tools_performance';
11+
import {profiler} from '../../src/render3/profiler';
12+
import {ProfilerEvent} from '../../src/render3/profiler_types';
1113
import {TestBed} from '../../testing';
1214

1315
describe('Chrome DevTools Performance integration', () => {
@@ -59,5 +61,26 @@ describe('Chrome DevTools Performance integration', () => {
5961
stopProfiling();
6062
}
6163
});
64+
65+
it('should not crash when asymmetric events are processed', () => {
66+
const timeStampSpy = spyOn(console, 'timeStamp');
67+
const stopProfiling = enableProfiling();
68+
69+
try {
70+
profiler(ProfilerEvent.ChangeDetectionSyncStart);
71+
profiler(ProfilerEvent.ChangeDetectionStart);
72+
profiler(ProfilerEvent.ChangeDetectionSyncEnd);
73+
74+
const calls = timeStampSpy.calls.all();
75+
expect(calls.length).toBe(3);
76+
77+
const [syncStart, cdStart, syncEnd] = calls;
78+
expect(syncStart.args[0]).toMatch(/^Event_/);
79+
expect(cdStart.args[0]).toMatch(/^Event_/);
80+
expect(syncEnd.args[0]).toMatch(/^Synchronization /);
81+
} finally {
82+
stopProfiling();
83+
}
84+
});
6285
});
6386
});

0 commit comments

Comments
 (0)