[debug] Lazily lookup source positions for StackFrameInfo.

This changes the StackFrameInfo to either hold on to a pair of

  (Script,source position)

or a pair of

  (SharedFunctioInfo,bytecode offset)

similar to what we do for MessageLocation. The idea here is to defer the
costly bytecode offset to source position lookup until really needed,
and in particular, avoid the costly lookup during stack trace capturing.

On the `standalone.js` benchmark in crbug.com/1283162#c1, this reduces
overall average execution time by roughly 25%, and the performance is
almost back to where it was before crrev.com/c/3302794 (being only 12%
slower than before on the `standalone.js` test case).

Note that due to unrelated limitations we cannot encode -1 as bytecode
offset in the flags field of the StackFrameInfo, and so we treat this
case specially (happens when stack trace capturing is triggered in the
function entry sequence) and just eagerly resolve it to the source
position.

Bug: chromium:1278650, chromium:1283162, chromium:1280803
Bug: chromium:1280818, chromium:1280831, chromium:1280832
Doc: https://bit.ly/v8-cheaper-inspector-stack-traces
Change-Id: If7cf62fce48d32c0f188895d1f8c9eee51b9e70d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3359633Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78466}

src/api/api.cc

@@ -3255,8 +3255,9 @@ Location StackFrame::GetLocation() const {
   i::Isolate* isolate = self->GetIsolate();
   i::Handle<i::Script> script(self->script(), isolate);
   i::Script::PositionInfo info;
-  CHECK(i::Script::GetPositionInfo(script, self->source_position(), &info,
-                                   i::Script::WITH_OFFSET));
+  CHECK(i::Script::GetPositionInfo(script,
+                                   i::StackFrameInfo::GetSourcePosition(self),
+                                   &info, i::Script::WITH_OFFSET));
   if (script->HasSourceURLComment()) {
     info.line -= script->line_offset();
     if (info.line == 0) {
@@ -3308,9 +3309,9 @@ Local<String> StackFrame::GetScriptSourceMappingURL() const {
 Local<String> StackFrame::GetFunctionName() const {
   i::Handle<i::StackFrameInfo> self = Utils::OpenHandle(this);
   i::Isolate* isolate = self->GetIsolate();
-  i::Handle<i::PrimitiveHeapObject> name(self->function_name(), isolate);
-  if (!name->IsString()) return {};
-  return Utils::ToLocal(i::Handle<i::String>::cast(name));
+  i::Handle<i::String> name(self->function_name(), isolate);
+  if (name->length() == 0) return {};
+  return Utils::ToLocal(name);
 }
 
 bool StackFrame::IsEval() const {

src/execution/frames.cc

@@ -1593,17 +1593,29 @@ Handle<Context> FrameSummary::JavaScriptFrameSummary::native_context() const {
   return handle(function_->context().native_context(), isolate());
 }
 
-Handle<PrimitiveHeapObject> FrameSummary::JavaScriptFrameSummary::FunctionName()
+Handle<StackFrameInfo> FrameSummary::JavaScriptFrameSummary::StackFrameInfo()
     const {
-  Handle<JSFunction> func = function();
-  Handle<String> name = JSFunction::GetDebugName(func);
-  if (name->length() != 0) return name;
-  if (func->shared().script().IsScript() &&
-      Script::cast(func->shared().script()).compilation_type() ==
-          Script::COMPILATION_TYPE_EVAL) {
-    return isolate()->factory()->eval_string();
-  }
-  return isolate()->factory()->null_value();
+  Handle<SharedFunctionInfo> shared(function_->shared(), isolate());
+  Handle<Script> script(Script::cast(shared->script()), isolate());
+  Handle<String> function_name = JSFunction::GetDebugName(function_);
+  if (function_name->length() == 0 &&
+      script->compilation_type() == Script::COMPILATION_TYPE_EVAL) {
+    function_name = isolate()->factory()->eval_string();
+  }
+  int bytecode_offset = code_offset();
+  if (bytecode_offset == kFunctionEntryBytecodeOffset) {
+    // For the special function entry bytecode offset (-1), which signals
+    // that the stack trace was captured while the function entry was
+    // executing (i.e. during the interrupt check), we cannot store this
+    // sentinel in the bit field, so we just eagerly lookup the source
+    // position within the script.
+    SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate(), shared);
+    int source_position = abstract_code()->SourcePosition(bytecode_offset);
+    return isolate()->factory()->NewStackFrameInfo(
+        script, source_position, function_name, is_constructor());
+  }
+  return isolate()->factory()->NewStackFrameInfo(
+      shared, bytecode_offset, function_name, is_constructor());
 }
 
 #if V8_ENABLE_WEBASSEMBLY
@@ -1643,8 +1655,11 @@ Handle<Context> FrameSummary::WasmFrameSummary::native_context() const {
   return handle(wasm_instance()->native_context(), isolate());
 }
 
-Handle<String> FrameSummary::WasmFrameSummary::FunctionName() const {
-  return GetWasmFunctionDebugName(isolate(), wasm_instance(), function_index());
+Handle<StackFrameInfo> FrameSummary::WasmFrameSummary::StackFrameInfo() const {
+  Handle<String> function_name =
+      GetWasmFunctionDebugName(isolate(), wasm_instance(), function_index());
+  return isolate()->factory()->NewStackFrameInfo(script(), SourcePosition(),
+                                                 function_name, false);
 }
 #endif  // V8_ENABLE_WEBASSEMBLY
 
@@ -1715,7 +1730,7 @@ FRAME_SUMMARY_DISPATCH(Handle<Object>, script)
 FRAME_SUMMARY_DISPATCH(int, SourcePosition)
 FRAME_SUMMARY_DISPATCH(int, SourceStatementPosition)
 FRAME_SUMMARY_DISPATCH(Handle<Context>, native_context)
-FRAME_SUMMARY_DISPATCH(Handle<PrimitiveHeapObject>, FunctionName)
+FRAME_SUMMARY_DISPATCH(Handle<StackFrameInfo>, StackFrameInfo)
 
 #undef FRAME_SUMMARY_DISPATCH
 

src/execution/frames.h

@@ -52,7 +52,6 @@ struct JumpBuffer;
 class StackMemory;
 }  // namespace wasm
 
-// Forward declarations.
 class AbstractCode;
 class Debug;
 class ExternalCallbackScope;
@@ -61,6 +60,7 @@ class Isolate;
 class ObjectVisitor;
 class Register;
 class RootVisitor;
+class StackFrameInfo;
 class StackFrameIteratorBase;
 class StringStream;
 class ThreadLocalTop;
@@ -394,7 +394,7 @@ class V8_EXPORT_PRIVATE FrameSummary {
     int SourceStatementPosition() const;
     Handle<Object> script() const;
     Handle<Context> native_context() const;
-    Handle<PrimitiveHeapObject> FunctionName() const;
+    Handle<StackFrameInfo> StackFrameInfo() const;
 
    private:
     Handle<Object> receiver_;
@@ -424,7 +424,7 @@ class V8_EXPORT_PRIVATE FrameSummary {
     Handle<WasmInstanceObject> wasm_instance() const { return wasm_instance_; }
     Handle<Context> native_context() const;
     bool at_to_number_conversion() const { return at_to_number_conversion_; }
-    Handle<String> FunctionName() const;
+    Handle<StackFrameInfo> StackFrameInfo() const;
 
    private:
     Handle<WasmInstanceObject> wasm_instance_;
@@ -458,7 +458,7 @@ class V8_EXPORT_PRIVATE FrameSummary {
   int SourcePosition() const;
   int SourceStatementPosition() const;
   Handle<Context> native_context() const;
-  Handle<PrimitiveHeapObject> FunctionName() const;
+  Handle<StackFrameInfo> StackFrameInfo() const;
 
 #define FRAME_SUMMARY_CAST(kind_, type, field, desc)      \
   bool Is##desc() const { return base_.kind() == kind_; } \

src/execution/isolate.cc

@@ -1262,10 +1262,7 @@ class StackFrameBuilder {
     if (index_ >= limit_) return false;
     // Skip frames that aren't subject to debugging.
     if (!summary.is_subject_to_debugging()) return true;
-    summary.EnsureSourcePositionsAvailable();
-    Handle<StackFrameInfo> frame = isolate_->factory()->NewStackFrameInfo(
-        Handle<Script>::cast(summary.script()), summary.SourcePosition(),
-        summary.FunctionName(), summary.is_constructor());
+    Handle<StackFrameInfo> frame = summary.StackFrameInfo();
     frames_ = FixedArray::SetAndGrow(isolate_, frames_, index_++, frame);
     return true;
   }

src/heap/factory.cc

@@ -3358,14 +3358,16 @@ Handle<CallSiteInfo> Factory::NewCallSiteInfo(
 }
 
 Handle<StackFrameInfo> Factory::NewStackFrameInfo(
-    Handle<Script> script, int source_position,
-    Handle<PrimitiveHeapObject> function_name, bool is_constructor) {
+    Handle<HeapObject> shared_or_script, int bytecode_offset_or_source_position,
+    Handle<String> function_name, bool is_constructor) {
+  DCHECK_GE(bytecode_offset_or_source_position, 0);
   StackFrameInfo info = NewStructInternal<StackFrameInfo>(
       STACK_FRAME_INFO_TYPE, AllocationType::kYoung);
   DisallowGarbageCollection no_gc;
   info.set_flags(0);
-  info.set_script(*script, SKIP_WRITE_BARRIER);
-  info.set_source_position(source_position);
+  info.set_shared_or_script(*shared_or_script, SKIP_WRITE_BARRIER);
+  info.set_bytecode_offset_or_source_position(
+      bytecode_offset_or_source_position);
   info.set_function_name(*function_name, SKIP_WRITE_BARRIER);
   info.set_is_constructor(is_constructor);
   return handle(info, isolate());

src/heap/factory.h

@@ -403,8 +403,9 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
                                        int flags,
                                        Handle<FixedArray> parameters);
   Handle<StackFrameInfo> NewStackFrameInfo(
-      Handle<Script> script, int source_position,
-      Handle<PrimitiveHeapObject> function_name, bool is_constructor);
+      Handle<HeapObject> shared_or_script,
+      int bytecode_offset_or_source_position, Handle<String> function_name,
+      bool is_constructor);
 
   // Allocate various microtasks.
   Handle<CallableTask> NewCallableTask(Handle<JSReceiver> callable,

src/objects/debug-objects-inl.h

@@ -55,8 +55,16 @@ BytecodeArray DebugInfo::DebugBytecodeArray() {
 TQ_OBJECT_CONSTRUCTORS_IMPL(StackFrameInfo)
 NEVER_READ_ONLY_SPACE_IMPL(StackFrameInfo)
 
-BIT_FIELD_ACCESSORS(StackFrameInfo, flags, source_position,
-                    StackFrameInfo::SourcePositionBits)
+Script StackFrameInfo::script() const {
+  HeapObject object = shared_or_script();
+  if (object.IsSharedFunctionInfo()) {
+    object = SharedFunctionInfo::cast(object).script();
+  }
+  return Script::cast(object);
+}
+
+BIT_FIELD_ACCESSORS(StackFrameInfo, flags, bytecode_offset_or_source_position,
+                    StackFrameInfo::BytecodeOffsetOrSourcePositionBits)
 BIT_FIELD_ACCESSORS(StackFrameInfo, flags, is_constructor,
                     StackFrameInfo::IsConstructorBit)
 

src/objects/debug-objects.cc

@@ -395,5 +395,21 @@ void CoverageInfo::CoverageInfoPrint(std::ostream& os,
   }
 }
 
+// static
+int StackFrameInfo::GetSourcePosition(Handle<StackFrameInfo> info) {
+  if (info->shared_or_script().IsScript()) {
+    return info->bytecode_offset_or_source_position();
+  }
+  Isolate* isolate = info->GetIsolate();
+  Handle<SharedFunctionInfo> shared(
+      SharedFunctionInfo::cast(info->shared_or_script()), isolate);
+  SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate, shared);
+  int source_position = shared->abstract_code(isolate).SourcePosition(
+      info->bytecode_offset_or_source_position());
+  info->set_shared_or_script(shared->script());
+  info->set_bytecode_offset_or_source_position(source_position);
+  return source_position;
+}
+
 }  // namespace internal
 }  // namespace v8

src/objects/debug-objects.h

@@ -208,8 +208,13 @@ class StackFrameInfo
  public:
   NEVER_READ_ONLY_SPACE
 
-  // The source position for the stack frame.
-  DECL_INT_ACCESSORS(source_position)
+  static int GetSourcePosition(Handle<StackFrameInfo> info);
+
+  // The script for the stack frame.
+  inline Script script() const;
+
+  // The bytecode offset or source position for the stack frame.
+  DECL_INT_ACCESSORS(bytecode_offset_or_source_position)
 
   // Indicates that the frame corresponds to a 'new' invocation.
   DECL_BOOLEAN_ACCESSORS(is_constructor)

src/objects/debug-objects.tq

@@ -70,11 +70,17 @@ extern class CoverageInfo extends HeapObject {
 
 bitfield struct StackFrameInfoFlags extends uint31 {
   is_constructor: bool: 1 bit;
-  source_position: int32: 30 bit;
+  bytecode_offset_or_source_position: int32: 30 bit;
 }
 
 extern class StackFrameInfo extends Struct {
-  script: Script;
-  function_name: String|Null;
+  // In case this field holds a SharedFunctionInfo, the
+  // |bytecode_offset_or_source_position| part of the
+  // |flags| bit field below contains the bytecode offset
+  // within that SharedFunctionInfo. Otherwise if this
+  // is a Script, the |bytecode_offset_or_source_position|
+  // holds the source position within the Script.
+  shared_or_script: SharedFunctionInfo|Script;
+  function_name: String;
   flags: SmiTagged<StackFrameInfoFlags>;
 }