Skip to content

Commit 7607c60

Browse files
and-oliDevtools-frontend LUCI CQ
authored andcommitted
[Perf P] Use call site location in sync portion of stack traces
Instead of only using the top frame of the event's payload stack trace, use all frames available. Bug: 415987706 Change-Id: I882c99a7f30147710415c65d0239edc69f97b8eb Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6512984 Commit-Queue: Andres Olivares <andoli@chromium.org> Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
1 parent f926b41 commit 7607c60

File tree

2 files changed

+31
-28
lines changed

2 files changed

+31
-28
lines changed

front_end/models/trace/extras/StackTraceForEvent.test.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ function shapeStackTraceAsArray(stackTrace: Protocol.Runtime.StackTrace):
2525
stackTraceAsArray.push({callFrames: currentStackTrace.callFrames, description: currentStackTrace.description});
2626
currentStackTrace = currentStackTrace.parent;
2727
}
28-
2928
return stackTraceAsArray;
3029
}
3130

@@ -269,23 +268,20 @@ describeWithEnvironment('StackTraceForTraceEvent', function() {
269268
profileCall2.callFrame.lineNumber = 0;
270269
const traceEvent = makeCompleteEvent(Trace.Types.Events.Name.UPDATE_LAYOUT_TREE, 100, 10, '', pid, tid) as
271270
Trace.Types.Events.UpdateLayoutTree;
272-
const payloadCallFrame = {
273-
columnNumber: payloadColumnNumber,
274-
functionName: 'bar',
275-
lineNumber: payloadLineNumber,
276-
scriptId: '115',
277-
url: ''
278-
};
279-
traceEvent.args = {elementCount: 1, beginData: {frame: '', stackTrace: [payloadCallFrame]}};
271+
const payloadCallStack = [
272+
{columnNumber: payloadColumnNumber, functionName: 'bar', lineNumber: payloadLineNumber, scriptId: '115', url: ''},
273+
{columnNumber: payloadColumnNumber, functionName: 'foo', lineNumber: payloadLineNumber, scriptId: '115', url: ''},
274+
];
275+
traceEvent.args = {elementCount: 1, beginData: {frame: '', stackTrace: payloadCallStack}};
280276
const trace = parsedTraceFromEvents([profileCall1, profileCall2, traceEvent]);
281277

282278
const stackTraceForExtensionEntry = Trace.Extras.StackTraceForEvent.get(traceEvent, trace);
283279
assert.exists(stackTraceForExtensionEntry);
284280
assert.deepEqual(shapeStackTraceAsArray(stackTraceForExtensionEntry) as unknown[], [
285281
{
286282
callFrames: [
287-
{...payloadCallFrame, lineNumber: payloadLineNumber - 1, columnNumber: payloadColumnNumber - 1},
288-
profileCall1.callFrame
283+
{...payloadCallStack[0], lineNumber: payloadLineNumber - 1, columnNumber: payloadColumnNumber - 1},
284+
{...payloadCallStack[1], lineNumber: payloadLineNumber - 1, columnNumber: payloadColumnNumber - 1},
289285
],
290286
description: undefined
291287
},

front_end/models/trace/extras/StackTraceForEvent.ts

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,21 @@ export function get(event: Types.Events.Event, parsedTrace: Handlers.Types.Parse
3737
result = getForPerformanceMeasure(event, parsedTrace);
3838
} else {
3939
result = getForEvent(event, parsedTrace);
40-
const maybeProtocolCallFrame = getPayloadStackAsProtocolCallFrame(event);
41-
if (result && maybeProtocolCallFrame && !isNativeJSFunction(maybeProtocolCallFrame)) {
42-
// If the event has a payload stack trace, replace the top frame
43-
// of the calculated stack with the top frame of the payload stack
44-
// because trace payload call frames contain call locations, unlike
45-
// profile call frames (which contain function declaration locations).
46-
// This way the user knows which exact JS location triggered an
47-
// event.
48-
result.callFrames[0] = maybeProtocolCallFrame;
40+
const payloadCallFrames =
41+
getTraceEventPayloadStackAsProtocolCallFrame(event).filter(callFrame => !isNativeJSFunction(callFrame));
42+
// If the event has a payload stack trace, replace the synchronous
43+
// portion of the calculated stack with the payload's call frames.
44+
// We do this because trace payload call frames contain call
45+
// locations, unlike profile call frames obtained with getForEvent
46+
// (which contain function declaration locations).
47+
// This way the user knows which exact JS location triggered an
48+
// event.
49+
if (!result.callFrames.length) {
50+
result.callFrames = payloadCallFrames;
51+
} else {
52+
for (let i = 0; i < payloadCallFrames.length && i < result.callFrames.length; i++) {
53+
result.callFrames[i] = payloadCallFrames[i];
54+
}
4955
}
5056
}
5157
if (result) {
@@ -187,13 +193,14 @@ function isNativeJSFunction({columnNumber, lineNumber, url, scriptId}: Protocol.
187193
}
188194

189195
/**
190-
* Extracts the top frame in the stack contained in a trace event's payload
191-
* (if any) and casts it as a Protocol.Runtime.CallFrame.
196+
* Converts a stack trace from a trace event's payload into an array of
197+
* Protocol.Runtime.CallFrame.
192198
*/
193-
function getPayloadStackAsProtocolCallFrame(event: Types.Events.Event): Protocol.Runtime.CallFrame|null {
194-
const maybeCallStack = Helpers.Trace.getZeroIndexedStackTraceInEventPayload(event);
195-
const maybeCallFrame = maybeCallStack?.at(0);
196-
const maybeProtocolCallFrame =
197-
maybeCallFrame && {...maybeCallFrame, scriptId: String(maybeCallFrame.scriptId) as Protocol.Runtime.ScriptId};
198-
return maybeProtocolCallFrame || null;
199+
function getTraceEventPayloadStackAsProtocolCallFrame(event: Types.Events.Event): Protocol.Runtime.CallFrame[] {
200+
const payloadCallStack = Helpers.Trace.getZeroIndexedStackTraceInEventPayload(event) || [];
201+
const callFrames: Protocol.Runtime.CallFrame[] = [];
202+
for (const frame of payloadCallStack) {
203+
callFrames.push({...frame, scriptId: String(frame.scriptId) as Protocol.Runtime.ScriptId});
204+
}
205+
return callFrames;
199206
}

0 commit comments

Comments
 (0)