[stack-trace] Mitigate stack trace symbolization performance regression

This CL mitigates the worst performance regressions for stack trace
symbolization.

The first fix is to hold the StackFrameBase instance in a local
variable. The instance is produced by an iterator but newly created
everytime FrameArrayIterator::Frame is called.

The second fix is to skip symbolization of "MethodName" and
"TypeName" for non-method calls.

R=ulan@chromium.org

Bug: chromium:981541,v8:8742
Change-Id: I58b2e0c87693c3914cf1946ce56341fbd4a797ac
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1688927
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62549}

src/heap/factory.cc

@@ -3688,22 +3688,23 @@ Handle<StackFrameInfo> Factory::NewStackFrameInfo(
   DCHECK(it.HasFrame());
 
   const bool is_wasm = frame_array->IsAnyWasmFrame(index);
+  StackFrameBase* frame = it.Frame();
 
-  int line = it.Frame()->GetLineNumber();
-  int column = it.Frame()->GetColumnNumber();
+  int line = frame->GetLineNumber();
+  int column = frame->GetColumnNumber();
 
-  const int script_id = it.Frame()->GetScriptId();
+  const int script_id = frame->GetScriptId();
 
-  Handle<Object> script_name = it.Frame()->GetFileName();
-  Handle<Object> script_or_url = it.Frame()->GetScriptNameOrSourceUrl();
+  Handle<Object> script_name = frame->GetFileName();
+  Handle<Object> script_or_url = frame->GetScriptNameOrSourceUrl();
 
   // TODO(szuend): Adjust this, once it is decided what name to use in both
   //               "simple" and "detailed" stack traces. This code is for
   //               backwards compatibility to fullfill test expectations.
-  auto function_name = it.Frame()->GetFunctionName();
+  auto function_name = frame->GetFunctionName();
   bool is_user_java_script = false;
   if (!is_wasm) {
-    Handle<Object> function = it.Frame()->GetFunction();
+    Handle<Object> function = frame->GetFunction();
     if (function->IsJSFunction()) {
       Handle<JSFunction> fun = Handle<JSFunction>::cast(function);
 
@@ -3711,10 +3712,24 @@ Handle<StackFrameInfo> Factory::NewStackFrameInfo(
     }
   }
 
-  Handle<Object> method_name = it.Frame()->GetMethodName();
-  Handle<Object> type_name = it.Frame()->GetTypeName();
-  Handle<Object> eval_origin = it.Frame()->GetEvalOrigin();
-  Handle<Object> wasm_module_name = it.Frame()->GetWasmModuleName();
+  Handle<Object> method_name = undefined_value();
+  Handle<Object> type_name = undefined_value();
+  Handle<Object> eval_origin = frame->GetEvalOrigin();
+  Handle<Object> wasm_module_name = frame->GetWasmModuleName();
+
+  // MethodName and TypeName are expensive to look up, so they are only
+  // included when they are strictly needed by the stack trace
+  // serialization code.
+  // Note: The {is_method_call} predicate needs to be kept in sync with
+  //       the corresponding predicate in the stack trace serialization code
+  //       in stack-frame-info.cc.
+  const bool is_toplevel = frame->IsToplevel();
+  const bool is_constructor = frame->IsConstructor();
+  const bool is_method_call = !(is_toplevel || is_constructor);
+  if (is_method_call) {
+    method_name = frame->GetMethodName();
+    type_name = frame->GetTypeName();
+  }
 
   Handle<StackFrameInfo> info = Handle<StackFrameInfo>::cast(
       NewStruct(STACK_FRAME_INFO_TYPE, AllocationType::kYoung));
@@ -3737,12 +3752,12 @@ Handle<StackFrameInfo> Factory::NewStackFrameInfo(
   info->set_eval_origin(*eval_origin);
   info->set_wasm_module_name(*wasm_module_name);
 
-  info->set_is_eval(it.Frame()->IsEval());
-  info->set_is_constructor(it.Frame()->IsConstructor());
-  info->set_is_toplevel(it.Frame()->IsToplevel());
-  info->set_is_async(it.Frame()->IsAsync());
-  info->set_is_promise_all(it.Frame()->IsPromiseAll());
-  info->set_promise_all_index(it.Frame()->GetPromiseIndex());
+  info->set_is_eval(frame->IsEval());
+  info->set_is_constructor(is_constructor);
+  info->set_is_toplevel(is_toplevel);
+  info->set_is_async(frame->IsAsync());
+  info->set_is_promise_all(frame->IsPromiseAll());
+  info->set_promise_all_index(frame->GetPromiseIndex());
 
   return info;
 }

src/objects/stack-frame-info.cc

@@ -294,6 +294,10 @@ void SerializeJSStackFrame(Isolate* isolate, Handle<StackTraceFrame> frame,
   const bool is_async = StackTraceFrame::IsAsync(frame);
   const bool is_promise_all = StackTraceFrame::IsPromiseAll(frame);
   const bool is_constructor = StackTraceFrame::IsConstructor(frame);
+  // Note: Keep the {is_method_call} predicate in sync with the corresponding
+  //       predicate in factory.cc where the StackFrameInfo is created.
+  //       Otherwise necessary fields for serialzing this frame might be
+  //       missing.
   const bool is_method_call = !(is_toplevel || is_constructor);
 
   if (is_async) {