[inspector] Omit call frames in instrumentation pause messages

Creating the full call frames is expensive. The client should only
need the script id. As the script id is passed in the 'data.scriptId'
field of the message, we can omit call frames from the instrumentation
pause event.

Bug: chromium:1408105
Change-Id: I11827865168946e1f412f7d351a0d359e2ac80ed
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4174085
Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85382}
This commit is contained in:
Jaroslav Sevcik 2023-01-18 08:24:14 +01:00 committed by V8 LUCI CQ
parent bbbf5d8c7b
commit 1009874faa
10 changed files with 39 additions and 43 deletions

View File

@ -1997,19 +1997,15 @@ void V8DebuggerAgentImpl::didPauseOnInstrumentation(
String16 breakReason = protocol::Debugger::Paused::ReasonEnum::Other;
std::unique_ptr<protocol::DictionaryValue> breakAuxData;
std::unique_ptr<Array<CallFrame>> protocolCallFrames;
Response response = currentCallFrames(&protocolCallFrames);
if (!response.IsSuccess())
protocolCallFrames = std::make_unique<Array<CallFrame>>();
if (m_debuggerBreakpointIdToBreakpointId.find(instrumentationId) !=
m_debuggerBreakpointIdToBreakpointId.end()) {
DCHECK_GT(protocolCallFrames->size(), 0);
if (protocolCallFrames->size() > 0) {
std::unique_ptr<V8StackTraceImpl> stack =
V8StackTraceImpl::capture(m_inspector->debugger(), 1);
DCHECK(stack && !stack->isEmpty());
if (stack && !stack->isEmpty()) {
m_instrumentationFinished = false;
breakReason = protocol::Debugger::Paused::ReasonEnum::Instrumentation;
const String16 scriptId =
protocolCallFrames->at(0)->getLocation()->getScriptId();
const String16 scriptId = String16::fromInteger(stack->topScriptId());
DCHECK_NE(m_scripts.find(scriptId), m_scripts.end());
const auto& script = m_scripts[scriptId];
@ -2022,6 +2018,8 @@ void V8DebuggerAgentImpl::didPauseOnInstrumentation(
}
}
std::unique_ptr<Array<CallFrame>> protocolCallFrames =
std::make_unique<Array<CallFrame>>();
m_frontend.paused(std::move(protocolCallFrames), breakReason,
std::move(breakAuxData),
std::make_unique<Array<String16>>(),

View File

@ -8,7 +8,9 @@ const { session, contextGroup, Protocol } = InspectorTest.start(
function handlePause(
noInstrumentationStepAction, options,
{params: {reason, data, callFrames}}) {
const scriptId = callFrames[0].functionLocation.scriptId;
const scriptId = reason === 'instrumentation' ?
data.scriptId :
callFrames[0].functionLocation.scriptId;
InspectorTest.log(`Paused with reason ${reason}, data ${
data ? JSON.stringify(data) : '{}'} and scriptId: ${scriptId}.`);

View File

@ -8,9 +8,8 @@ const {session, contextGroup, Protocol} = InspectorTest.start(
session.setupScriptMap();
function logPause(msg) {
const top_frame = msg.params.callFrames[0];
const reason = msg.params.reason;
const url = session.getCallFrameUrl(top_frame);
const url = session.getPausedUrl(msg);
InspectorTest.log(`Paused at ${url} with reason "${reason}".`);
};

View File

@ -6,7 +6,7 @@ Setting breakpoint at instrumentation break location
Paused at foo.js with reason "instrumentation".
Hit breakpoints: []
Paused at foo.js with reason "other".
Hit breakpoints: ["4:0:20:3"]
Hit breakpoints: ["4:0:0:3"]
Done.
Running test: testSetConditionalBreakpointTrueConditionOnInstrumentationPause
@ -15,7 +15,7 @@ Setting breakpoint at instrumentation break location
Paused at foo.js with reason "instrumentation".
Hit breakpoints: []
Paused at foo.js with reason "other".
Hit breakpoints: ["4:0:20:4"]
Hit breakpoints: ["4:0:0:4"]
Done.
Running test: testSetConditionalBreakpointFalseConditionOnInstrumentationPause

View File

@ -9,13 +9,11 @@ session.setupScriptMap();
function setBreakpoint(msg, condition) {
const reason = msg.params.reason;
if (reason === 'instrumentation') {
const top_frame = msg.params.callFrames[0];
const scriptId = top_frame.location.scriptId;
const columnNumber = top_frame.location.columnNumber;
const scriptId = msg.params.data.scriptId;
InspectorTest.log('Setting breakpoint at instrumentation break location');
const breakpoint_info = {
'location': {scriptId, 'lineNumber': 0, columnNumber}
'location': {scriptId, 'lineNumber': 0, 'columnNumber': 0}
};
if (condition) {
breakpoint_info.condition = condition;
@ -26,9 +24,8 @@ function setBreakpoint(msg, condition) {
}
function handlePause(msg) {
const top_frame = msg.params.callFrames[0];
const reason = msg.params.reason;
const url = session.getCallFrameUrl(top_frame);
const url = session.getPausedUrl(msg);
InspectorTest.log(`Paused at ${url} with reason "${reason}".`);
InspectorTest.log(
`Hit breakpoints: ${JSON.stringify(msg.params.hitBreakpoints)}`);
@ -45,7 +42,7 @@ async function runSetBreakpointOnInstrumentationTest(condition) {
await Protocol.Debugger.setInstrumentationBreakpoint(
{instrumentation: 'beforeScriptExecution'});
const runPromise =
Protocol.Runtime.evaluate({expression: '//# sourceURL=foo.js'});
Protocol.Runtime.evaluate({expression: '42\n//# sourceURL=foo.js'});
// First pause: instrumentation
const msg = await Protocol.Debugger.oncePaused();

View File

@ -15,7 +15,6 @@ Instantiating module.
Paused at v8://test/instantiate with reason "instrumentation".
Hit breakpoints: []
Paused at wasm://wasm/20da547a with reason "instrumentation".
Script wasm://wasm/20da547a byte offset 26: Wasm opcode 0x01 (kExprNop)
Hit breakpoints: []
Instantiating a second time (should trigger no breakpoint).
Paused at v8://test/instantiate2 with reason "instrumentation".
@ -34,13 +33,11 @@ Instantiating module.
Paused at v8://test/instantiate with reason "instrumentation".
Hit breakpoints: []
Paused at wasm://wasm/20da547a with reason "instrumentation".
Script wasm://wasm/20da547a byte offset 26: Wasm opcode 0x01 (kExprNop)
Hit breakpoints: []
Instantiating a second time (should trigger another breakpoint).
Paused at v8://test/instantiate with reason "instrumentation".
Hit breakpoints: []
Paused at wasm://wasm/20da547a with reason "instrumentation".
Script wasm://wasm/20da547a byte offset 26: Wasm opcode 0x01 (kExprNop)
Hit breakpoints: []
Done.
@ -59,7 +56,6 @@ Calling exported function 'func' (should trigger a breakpoint).
Paused at v8://test/call_func with reason "instrumentation".
Hit breakpoints: []
Paused at wasm://wasm/8c388106 with reason "instrumentation".
Script wasm://wasm/8c388106 byte offset 33: Wasm opcode 0x01 (kExprNop)
Hit breakpoints: []
Calling exported function 'func' a second time (should trigger no breakpoint).
Paused at v8://test/call_func with reason "instrumentation".
@ -79,7 +75,6 @@ Calling exported function 'func' (should trigger no breakpoint).
Instantiating wasm module with source map.
Calling exported function 'func' (should trigger a breakpoint).
Paused at wasm://wasm/c8e3a856 with reason "instrumentation".
Script wasm://wasm/c8e3a856 byte offset 33: Wasm opcode 0x01 (kExprNop)
Hit breakpoints: []
Done.
@ -126,7 +121,6 @@ Instantiating module should trigger a break.
Paused at v8://test/instantiate with reason "instrumentation".
Hit breakpoints: []
Paused at wasm://wasm/20da547a with reason "instrumentation".
Script wasm://wasm/20da547a byte offset 26: Wasm opcode 0x01 (kExprNop)
Hit breakpoints: []
Remove instrumentation breakpoint..
Compiling another wasm module.

View File

@ -9,12 +9,12 @@ const {session, contextGroup, Protocol} =
session.setupScriptMap();
Protocol.Debugger.onPaused(async msg => {
let top_frame = msg.params.callFrames[0];
let reason = msg.params.reason;
let hitBreakpoints = msg.params.hitBreakpoints;
const url = session.getCallFrameUrl(top_frame);
const url = session.getPausedUrl(msg);
InspectorTest.log(`Paused at ${url} with reason "${reason}".`);
if (!url.startsWith('v8://test/')) {
if (!url.startsWith('v8://test/') && msg.params.callFrames.length > 0) {
let top_frame = msg.params.callFrames[0];
await session.logSourceLocation(top_frame.location);
}
// Report the hit breakpoints to make sure that it is empty, as

View File

@ -10,7 +10,6 @@ Paused at v8://test/instantiate with reason "instrumentation".
Hit breakpoints: []
Setting breakpoint at instrumentation break location
Paused at wasm://wasm/20da547a with reason "instrumentation".
Script wasm://wasm/20da547a byte offset 26: Wasm opcode 0x01 (kExprNop)
Hit breakpoints: []
Paused at wasm://wasm/20da547a with reason "other".
Script wasm://wasm/20da547a byte offset 26: Wasm opcode 0x01 (kExprNop)
@ -27,7 +26,6 @@ Paused at v8://test/instantiate with reason "instrumentation".
Hit breakpoints: []
Setting breakpoint at instrumentation break location
Paused at wasm://wasm/20da547a with reason "instrumentation".
Script wasm://wasm/20da547a byte offset 26: Wasm opcode 0x01 (kExprNop)
Hit breakpoints: []
Paused at wasm://wasm/20da547a with reason "other".
Script wasm://wasm/20da547a byte offset 26: Wasm opcode 0x01 (kExprNop)
@ -44,6 +42,5 @@ Paused at v8://test/instantiate with reason "instrumentation".
Hit breakpoints: []
Setting breakpoint at instrumentation break location
Paused at wasm://wasm/20da547a with reason "instrumentation".
Script wasm://wasm/20da547a byte offset 26: Wasm opcode 0x01 (kExprNop)
Hit breakpoints: []
Done.

View File

@ -8,17 +8,16 @@ const {session, contextGroup, Protocol} = InspectorTest.start(
'Test if breakpoints are hit that are set on instrumentation pause in wasm.');
session.setupScriptMap();
function setBreakpoint(msg, condition) {
function setBreakpoint(msg, func, condition) {
const top_frame = msg.params.callFrames[0];
const reason = msg.params.reason;
const url = session.getCallFrameUrl(top_frame);
if (reason === 'instrumentation' && url.startsWith('wasm://')) {
const scriptId = top_frame.location.scriptId;
const columnNumber = top_frame.location.columnNumber;
if (reason === 'instrumentation' &&
msg.params.data.url.startsWith('wasm://')) {
const scriptId = msg.params.data.scriptId;
InspectorTest.log('Setting breakpoint at instrumentation break location');
const breakpoint_info = {
'location': {scriptId, 'lineNumber': 0, columnNumber}
'location': {scriptId, 'lineNumber': 0, 'columnNumber': func.body_offset}
};
if (condition) {
breakpoint_info.condition = condition;
@ -29,11 +28,11 @@ function setBreakpoint(msg, condition) {
}
async function handlePause(msg) {
const top_frame = msg.params.callFrames[0];
const reason = msg.params.reason;
const url = session.getCallFrameUrl(top_frame);
const url = session.getPausedUrl(msg);
InspectorTest.log(`Paused at ${url} with reason "${reason}".`);
if (!url.startsWith('v8://test/')) {
if (!url.startsWith('v8://test/') && msg.params.callFrames.length > 0) {
const top_frame = msg.params.callFrames[0];
await session.logSourceLocation(top_frame.location);
}
InspectorTest.log(
@ -68,7 +67,7 @@ async function runSetBreakpointOnInstrumentationTest(condition) {
// Third pause: wasm script. This will set a breakpoint. Pass on a condition.
const msg = await Protocol.Debugger.oncePaused();
await setBreakpoint(msg, condition);
await setBreakpoint(msg, start_fn, condition);
await handlePause(msg);
// Fourth pause: wasm script, if condition evaluates to true.

View File

@ -280,6 +280,16 @@ InspectorTest.Session = class {
return (this._scriptMap.get(scriptId) ?? frame).url;
}
getPausedUrl(msg) {
const reason = msg.params.reason;
if (reason === 'instrumentation') {
return msg.params.data.url;
} else {
const top_frame = msg.params.callFrames[0];
return this.getCallFrameUrl(top_frame);
}
}
logCallFrames(callFrames) {
for (var frame of callFrames) {
var functionName = frame.functionName || '(anonymous)';