From ed7b66400e1c2d4acb9b8f7e1487e83fca16209b Mon Sep 17 00:00:00 2001 From: Benedikt Meurer Date: Wed, 29 Dec 2021 11:50:01 +0100 Subject: [PATCH] [inspector] Introduce v8::StackFrame::GetLocation() API. This introduces a new `GetLocation()` method for `v8::StackFrame`s, which returns both line and column number at the same time (using the existing `v8::Location` class). Since `v8::StackFrame` instances store only the source position (per https://bit.ly/v8-stack-frame), we currently need to look up the source position in the Script's line table twice, once when we request the line number, and another time when we request the column number. With `GetLocation()` we perform only a single lookup in the Script's line table and return both line and column number at the same time. This cuts roughly 8% of the average execution time from the `standalone.js` benchmark mentioned in crbug.com/1280519. Bug: chromium:1280519, chromium:1278650, chromium:1069425 Bug: chromium:1077657, chromium:1283162 Doc: https://bit.ly/v8-cheaper-inspector-stack-traces Change-Id: Ia3a0502990b6230363112a358b59875283399404 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3359628 Reviewed-by: Yang Guo Commit-Queue: Benedikt Meurer Cr-Commit-Position: refs/heads/main@{#78452} --- include/v8-debug.h | 13 +++++++++---- src/api/api.cc | 28 ++++++++++------------------ src/inspector/v8-debugger.cc | 5 +++-- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/include/v8-debug.h b/include/v8-debug.h index a13ae3f6d6..d7cf129999 100644 --- a/include/v8-debug.h +++ b/include/v8-debug.h @@ -7,8 +7,8 @@ #include -#include "v8-local-handle.h" // NOLINT(build/include_directory) -#include "v8config.h" // NOLINT(build/include_directory) +#include "v8-script.h" // NOLINT(build/include_directory) +#include "v8config.h" // NOLINT(build/include_directory) namespace v8 { @@ -20,13 +20,18 @@ class String; */ class V8_EXPORT StackFrame { public: + /** + * Returns the source location, 0-based, for the associated function call. + */ + Location GetLocation() const; + /** * Returns the number, 1-based, of the line for the associate function call. * This method will return Message::kNoLineNumberInfo if it is unable to * retrieve the line number, or if kLineNumber was not passed as an option * when capturing the StackTrace. */ - int GetLineNumber() const; + int GetLineNumber() const { return GetLocation().GetLineNumber() + 1; } /** * Returns the 1-based column offset on the line for the associated function @@ -35,7 +40,7 @@ class V8_EXPORT StackFrame { * the column number, or if kColumnOffset was not passed as an option when * capturing the StackTrace. */ - int GetColumn() const; + int GetColumn() const { return GetLocation().GetColumnNumber() + 1; } /** * Returns the id of the script for the function for this StackFrame. diff --git a/src/api/api.cc b/src/api/api.cc index d4bd417a7e..8c36b2af90 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -3250,28 +3250,20 @@ Local StackTrace::CurrentStackTrace(Isolate* isolate, // --- S t a c k F r a m e --- -int StackFrame::GetLineNumber() const { +Location StackFrame::GetLocation() const { i::Handle self = Utils::OpenHandle(this); - i::Handle script(self->script(), self->GetIsolate()); - int position = self->source_position(); - int line_number = i::Script::GetLineNumber(script, position) + 1; + i::Isolate* isolate = self->GetIsolate(); + i::Handle script(self->script(), isolate); + i::Script::PositionInfo info; + CHECK(i::Script::GetPositionInfo(script, self->source_position(), &info, + i::Script::WITH_OFFSET)); if (script->HasSourceURLComment()) { - line_number -= script->line_offset(); - } - return line_number; -} - -int StackFrame::GetColumn() const { - i::Handle self = Utils::OpenHandle(this); - i::Handle script(self->script(), self->GetIsolate()); - int position = self->source_position(); - int column_number = i::Script::GetColumnNumber(script, position) + 1; - if (script->HasSourceURLComment()) { - if (i::Script::GetLineNumber(script, position) == script->line_offset()) { - column_number -= script->column_offset(); + info.line -= script->line_offset(); + if (info.line == 0) { + info.column -= script->column_offset(); } } - return column_number; + return {info.line, info.column}; } int StackFrame::GetScriptId() const { diff --git a/src/inspector/v8-debugger.cc b/src/inspector/v8-debugger.cc index e3f724b5a2..e37c1fbb14 100644 --- a/src/inspector/v8-debugger.cc +++ b/src/inspector/v8-debugger.cc @@ -1091,8 +1091,9 @@ void V8Debugger::collectOldAsyncStacksIfNeeded() { std::shared_ptr V8Debugger::symbolize( v8::Local v8Frame) { int scriptId = v8Frame->GetScriptId(); - int lineNumber = v8Frame->GetLineNumber() - 1; - int columnNumber = v8Frame->GetColumn() - 1; + auto location = v8Frame->GetLocation(); + int lineNumber = location.GetLineNumber(); + int columnNumber = location.GetColumnNumber(); CachedStackFrameKey key{scriptId, lineNumber, columnNumber}; auto functionName = toProtocolString(isolate(), v8Frame->GetFunctionName()); auto it = m_cachedStackFrames.find(key);