[inspector] deduplicate stack frames

Since we already have cache on V8 side we can introduce caching on inspector side. It will decrease memory consumption and reduce time which we spend for collecting stacks. See [1] for details.

[1] https://docs.google.com/a/google.com/document/d/13H1Pn6dekcwqlaYP26CfyyYGuL-U9LtUPWmt3TIpOag/edit?usp=sharing

BUG=v8:6189
R=dgozman@chromium.org,yangguo@chromium.org

Review-Url: https://codereview.chromium.org/2825903002
Cr-Commit-Position: refs/heads/master@{#44753}

src/api.cc

@@ -9699,6 +9699,10 @@ debug::ConsoleCallArguments::ConsoleCallArguments(
     : v8::FunctionCallbackInfo<v8::Value>(nullptr, &args[0] - 1,
                                           args.length() - 1) {}
 
+int debug::GetStackFrameId(v8::Local<v8::StackFrame> frame) {
+  return Utils::OpenHandle(*frame)->id();
+}
+
 MaybeLocal<debug::Script> debug::GeneratorObject::Script() {
   i::Handle<i::JSGeneratorObject> obj = Utils::OpenHandle(this);
   i::Object* maybe_script = obj->function()->shared()->script();

src/debug/debug-interface.h

@@ -211,6 +211,8 @@ Local<Function> GetBuiltin(Isolate* isolate, Builtin builtin);
 
 void SetConsoleDelegate(Isolate* isolate, ConsoleDelegate* delegate);
 
+int GetStackFrameId(v8::Local<v8::StackFrame> frame);
+
 /**
  * Native wrapper around v8::internal::JSGeneratorObject object.
  */

src/inspector/v8-debugger.cc

@@ -975,6 +975,7 @@ void V8Debugger::allAsyncTasksCanceled() {
   m_parentTask.clear();
   m_asyncTaskCreationStacks.clear();
 
+  m_framesCache.clear();
   m_allAsyncStacks.clear();
   m_asyncStacksCount = 0;
 }
@@ -1033,6 +1034,11 @@ void V8Debugger::collectOldAsyncStacksIfNeeded() {
     parentLeft.insert(it);
   }
   m_parentTask.swap(parentLeft);
+  std::map<int, std::weak_ptr<StackFrame>> framesCache;
+  for (auto it : m_framesCache) {
+    if (!it.second.expired()) framesCache.insert(it);
+  }
+  m_framesCache.swap(framesCache);
 }
 
 void V8Debugger::removeOldAsyncTasks(AsyncTaskToStackTrace& map) {
@@ -1043,6 +1049,25 @@ void V8Debugger::removeOldAsyncTasks(AsyncTaskToStackTrace& map) {
   map.swap(cleanCopy);
 }
 
+std::shared_ptr<StackFrame> V8Debugger::symbolize(
+    v8::Local<v8::StackFrame> v8Frame) {
+  auto it = m_framesCache.end();
+  int frameId = 0;
+  if (m_maxAsyncCallStackDepth) {
+    frameId = v8::debug::GetStackFrameId(v8Frame);
+    it = m_framesCache.find(frameId);
+  }
+  if (it != m_framesCache.end() && it->second.lock()) return it->second.lock();
+  std::shared_ptr<StackFrame> frame(new StackFrame(v8Frame));
+  // TODO(clemensh): Figure out a way to do this translation only right before
+  // sending the stack trace over wire.
+  if (v8Frame->IsWasm()) frame->translate(&m_wasmTranslation);
+  if (m_maxAsyncCallStackDepth) {
+    m_framesCache[frameId] = frame;
+  }
+  return frame;
+}
+
 void V8Debugger::setMaxAsyncTaskStacksForTest(int limit) {
   m_maxAsyncCallStacks = 0;
   collectOldAsyncStacksIfNeeded();

src/inspector/v8-debugger.h

@@ -23,6 +23,7 @@ namespace v8_inspector {
 
 class AsyncStackTrace;
 struct ScriptBreakpoint;
+class StackFrame;
 class V8Debugger;
 class V8DebuggerAgentImpl;
 class V8InspectorImpl;
@@ -85,6 +86,8 @@ class V8Debugger : public v8::debug::DebugDelegate {
   std::shared_ptr<AsyncStackTrace> currentAsyncParent();
   std::shared_ptr<AsyncStackTrace> currentAsyncCreation();
 
+  std::shared_ptr<StackFrame> symbolize(v8::Local<v8::StackFrame> v8Frame);
+
   std::unique_ptr<V8StackTraceImpl> createStackTrace(v8::Local<v8::StackTrace>);
   std::unique_ptr<V8StackTraceImpl> captureStackTrace(bool fullStack);
 
@@ -202,6 +205,7 @@ class V8Debugger : public v8::debug::DebugDelegate {
   // V8Debugger owns all the async stacks, while most of the other references
   // are weak, which allows to collect some stacks when there are too many.
   std::list<std::shared_ptr<AsyncStackTrace>> m_allAsyncStacks;
+  std::map<int, std::weak_ptr<StackFrame>> m_framesCache;
 
   protocol::HashMap<V8DebuggerAgentImpl*, int> m_maxAsyncCallStackDepthMap;
   void* m_taskWithScheduledBreak = nullptr;

src/inspector/v8-stack-trace-impl.cc

@@ -18,18 +18,14 @@ static const v8::StackTrace::StackTraceOptions stackTraceOptions =
         v8::StackTrace::kDetailed |
         v8::StackTrace::kExposeFramesAcrossSecurityOrigins);
 
-std::vector<V8StackTraceImpl::Frame> toFramesVector(
+std::vector<std::shared_ptr<StackFrame>> toFramesVector(
     V8Debugger* debugger, v8::Local<v8::StackTrace> v8StackTrace,
     int maxStackSize) {
   DCHECK(debugger->isolate()->InContext());
   int frameCount = std::min(v8StackTrace->GetFrameCount(), maxStackSize);
-  std::vector<V8StackTraceImpl::Frame> frames;
+  std::vector<std::shared_ptr<StackFrame>> frames;
   for (int i = 0; i < frameCount; ++i) {
-    v8::Local<v8::StackFrame> v8Frame = v8StackTrace->GetFrame(i);
-    frames.emplace_back(v8Frame);
-    // TODO(clemensh): Figure out a way to do this translation only right before
-    // sending the stack trace over wire.
-    if (v8Frame->IsWasm()) frames.back().translate(debugger->wasmTranslation());
+    frames.push_back(debugger->symbolize(v8StackTrace->GetFrame(i)));
   }
   return frames;
 }
@@ -66,13 +62,13 @@ void calculateAsyncChain(V8Debugger* debugger, int contextGroupId,
 }
 
 std::unique_ptr<protocol::Runtime::StackTrace> buildInspectorObjectCommon(
-    const std::vector<V8StackTraceImpl::Frame>& frames,
+    const std::vector<std::shared_ptr<StackFrame>>& frames,
     const std::shared_ptr<AsyncStackTrace>& asyncParent,
     const std::shared_ptr<AsyncStackTrace>& asyncCreation, int maxAsyncDepth) {
   std::unique_ptr<protocol::Array<protocol::Runtime::CallFrame>>
       inspectorFrames = protocol::Array<protocol::Runtime::CallFrame>::create();
   for (size_t i = 0; i < frames.size(); i++) {
-    inspectorFrames->addItem(frames[i].buildInspectorObject());
+    inspectorFrames->addItem(frames[i]->buildInspectorObject());
   }
   std::unique_ptr<protocol::Runtime::StackTrace> stackTrace =
       protocol::Runtime::StackTrace::create()
@@ -87,7 +83,7 @@ std::unique_ptr<protocol::Runtime::StackTrace> buildInspectorObjectCommon(
 
 }  //  namespace
 
-V8StackTraceImpl::Frame::Frame(v8::Local<v8::StackFrame> v8Frame)
+StackFrame::StackFrame(v8::Local<v8::StackFrame> v8Frame)
     : m_functionName(toProtocolString(v8Frame->GetFunctionName())),
       m_scriptId(String16::fromInteger(v8Frame->GetScriptId())),
       m_sourceURL(toProtocolString(v8Frame->GetScriptNameOrSourceURL())),
@@ -97,27 +93,23 @@ V8StackTraceImpl::Frame::Frame(v8::Local<v8::StackFrame> v8Frame)
   DCHECK(m_columnNumber + 1 != v8::Message::kNoColumnInfo);
 }
 
-void V8StackTraceImpl::Frame::translate(WasmTranslation* wasmTranslation) {
+void StackFrame::translate(WasmTranslation* wasmTranslation) {
   wasmTranslation->TranslateWasmScriptLocationToProtocolLocation(
       &m_scriptId, &m_lineNumber, &m_columnNumber);
 }
 
-const String16& V8StackTraceImpl::Frame::functionName() const {
-  return m_functionName;
-}
+const String16& StackFrame::functionName() const { return m_functionName; }
 
-const String16& V8StackTraceImpl::Frame::scriptId() const { return m_scriptId; }
+const String16& StackFrame::scriptId() const { return m_scriptId; }
 
-const String16& V8StackTraceImpl::Frame::sourceURL() const {
-  return m_sourceURL;
-}
+const String16& StackFrame::sourceURL() const { return m_sourceURL; }
 
-int V8StackTraceImpl::Frame::lineNumber() const { return m_lineNumber; }
+int StackFrame::lineNumber() const { return m_lineNumber; }
 
-int V8StackTraceImpl::Frame::columnNumber() const { return m_columnNumber; }
+int StackFrame::columnNumber() const { return m_columnNumber; }
 
-std::unique_ptr<protocol::Runtime::CallFrame>
-V8StackTraceImpl::Frame::buildInspectorObject() const {
+std::unique_ptr<protocol::Runtime::CallFrame> StackFrame::buildInspectorObject()
+    const {
   return protocol::Runtime::CallFrame::create()
       .setFunctionName(m_functionName)
       .setScriptId(m_scriptId)
@@ -143,7 +135,7 @@ std::unique_ptr<V8StackTraceImpl> V8StackTraceImpl::create(
   v8::Isolate* isolate = debugger->isolate();
   v8::HandleScope scope(isolate);
 
-  std::vector<V8StackTraceImpl::Frame> frames;
+  std::vector<std::shared_ptr<StackFrame>> frames;
   if (!v8StackTrace.IsEmpty() && v8StackTrace->GetFrameCount()) {
     frames = toFramesVector(debugger, v8StackTrace, maxStackSize);
   }
@@ -154,8 +146,8 @@ std::unique_ptr<V8StackTraceImpl> V8StackTraceImpl::create(
   calculateAsyncChain(debugger, contextGroupId, &asyncParent, &asyncCreation,
                       &maxAsyncDepth);
   if (frames.empty() && !asyncCreation && !asyncParent) return nullptr;
-  return std::unique_ptr<V8StackTraceImpl>(
-      new V8StackTraceImpl(frames, maxAsyncDepth, asyncParent, asyncCreation));
+  return std::unique_ptr<V8StackTraceImpl>(new V8StackTraceImpl(
+      std::move(frames), maxAsyncDepth, asyncParent, asyncCreation));
 }
 
 // static
@@ -174,10 +166,10 @@ std::unique_ptr<V8StackTraceImpl> V8StackTraceImpl::capture(
 }
 
 V8StackTraceImpl::V8StackTraceImpl(
-    const std::vector<Frame> frames, int maxAsyncDepth,
+    std::vector<std::shared_ptr<StackFrame>> frames, int maxAsyncDepth,
     std::shared_ptr<AsyncStackTrace> asyncParent,
     std::shared_ptr<AsyncStackTrace> asyncCreation)
-    : m_frames(frames),
+    : m_frames(std::move(frames)),
       m_maxAsyncDepth(maxAsyncDepth),
       m_asyncParent(asyncParent),
       m_asyncCreation(asyncCreation) {}
@@ -193,23 +185,23 @@ std::unique_ptr<V8StackTrace> V8StackTraceImpl::clone() {
 bool V8StackTraceImpl::isEmpty() const { return m_frames.empty(); }
 
 StringView V8StackTraceImpl::topSourceURL() const {
-  return toStringView(m_frames[0].sourceURL());
+  return toStringView(m_frames[0]->sourceURL());
 }
 
 int V8StackTraceImpl::topLineNumber() const {
-  return m_frames[0].lineNumber() + 1;
+  return m_frames[0]->lineNumber() + 1;
 }
 
 int V8StackTraceImpl::topColumnNumber() const {
-  return m_frames[0].columnNumber() + 1;
+  return m_frames[0]->columnNumber() + 1;
 }
 
 StringView V8StackTraceImpl::topScriptId() const {
-  return toStringView(m_frames[0].scriptId());
+  return toStringView(m_frames[0]->scriptId());
 }
 
 StringView V8StackTraceImpl::topFunctionName() const {
-  return toStringView(m_frames[0].functionName());
+  return toStringView(m_frames[0]->functionName());
 }
 
 std::unique_ptr<protocol::Runtime::StackTrace>
@@ -226,7 +218,7 @@ V8StackTraceImpl::buildInspectorObject() const {
 std::unique_ptr<StringBuffer> V8StackTraceImpl::toString() const {
   String16Builder stackTrace;
   for (size_t i = 0; i < m_frames.size(); ++i) {
-    const Frame& frame = m_frames[i];
+    const StackFrame& frame = *m_frames[i];
     stackTrace.append("\n    at " + (frame.functionName().length()
                                          ? frame.functionName()
                                          : "(anonymous function)"));
@@ -251,7 +243,7 @@ std::shared_ptr<AsyncStackTrace> AsyncStackTrace::capture(
   v8::Isolate* isolate = debugger->isolate();
   v8::HandleScope handleScope(isolate);
 
-  std::vector<V8StackTraceImpl::Frame> frames;
+  std::vector<std::shared_ptr<StackFrame>> frames;
   if (isolate->InContext()) {
     v8::Local<v8::StackTrace> v8StackTrace = v8::StackTrace::CurrentStackTrace(
         isolate, maxStackSize, stackTraceOptions);
@@ -278,18 +270,19 @@ std::shared_ptr<AsyncStackTrace> AsyncStackTrace::capture(
   if (!contextGroupId && asyncParent) {
     contextGroupId = asyncParent->m_contextGroupId;
   }
-  return std::shared_ptr<AsyncStackTrace>(new AsyncStackTrace(
-      contextGroupId, description, frames, asyncParent, asyncCreation));
+  return std::shared_ptr<AsyncStackTrace>(
+      new AsyncStackTrace(contextGroupId, description, std::move(frames),
+                          asyncParent, asyncCreation));
 }
 
 AsyncStackTrace::AsyncStackTrace(
     int contextGroupId, const String16& description,
-    const std::vector<V8StackTraceImpl::Frame>& frames,
+    std::vector<std::shared_ptr<StackFrame>> frames,
     std::shared_ptr<AsyncStackTrace> asyncParent,
     std::shared_ptr<AsyncStackTrace> asyncCreation)
     : m_contextGroupId(contextGroupId),
       m_description(description),
-      m_frames(frames),
+      m_frames(std::move(frames)),
       m_asyncParent(asyncParent),
       m_asyncCreation(asyncCreation) {
   DCHECK(m_contextGroupId);
@@ -304,7 +297,7 @@ AsyncStackTrace::buildInspectorObject(AsyncStackTrace* asyncCreation,
   if (!m_description.isEmpty()) stackTrace->setDescription(m_description);
   if (asyncCreation && !asyncCreation->isEmpty()) {
     stackTrace->setPromiseCreationFrame(
-        asyncCreation->m_frames[0].buildInspectorObject());
+        asyncCreation->m_frames[0]->buildInspectorObject());
   }
   return stackTrace;
 }

src/inspector/v8-stack-trace-impl.h

@@ -20,6 +20,28 @@ class AsyncStackTrace;
 class V8Debugger;
 class WasmTranslation;
 
+class StackFrame {
+ public:
+  explicit StackFrame(v8::Local<v8::StackFrame> frame);
+  ~StackFrame() = default;
+
+  void translate(WasmTranslation* wasmTranslation);
+
+  const String16& functionName() const;
+  const String16& scriptId() const;
+  const String16& sourceURL() const;
+  int lineNumber() const;    // 0-based.
+  int columnNumber() const;  // 0-based.
+  std::unique_ptr<protocol::Runtime::CallFrame> buildInspectorObject() const;
+
+ private:
+  String16 m_functionName;
+  String16 m_scriptId;
+  String16 m_sourceURL;
+  int m_lineNumber;    // 0-based.
+  int m_columnNumber;  // 0-based.
+};
+
 class V8StackTraceImpl : public V8StackTrace {
  public:
   static void setCaptureStackTraceForUncaughtExceptions(v8::Isolate*,
@@ -50,34 +72,13 @@ class V8StackTraceImpl : public V8StackTrace {
       const override;
   std::unique_ptr<StringBuffer> toString() const override;
 
-  class Frame {
-   public:
-    explicit Frame(v8::Local<v8::StackFrame> frame);
-    ~Frame() = default;
-
-    void translate(WasmTranslation* wasmTranslation);
-
-    const String16& functionName() const;
-    const String16& scriptId() const;
-    const String16& sourceURL() const;
-    int lineNumber() const;    // 0-based.
-    int columnNumber() const;  // 0-based.
-    std::unique_ptr<protocol::Runtime::CallFrame> buildInspectorObject() const;
-
-   private:
-    String16 m_functionName;
-    String16 m_scriptId;
-    String16 m_sourceURL;
-    int m_lineNumber;    // 0-based.
-    int m_columnNumber;  // 0-based.
-  };
-
  private:
-  V8StackTraceImpl(const std::vector<Frame> frames, int maxAsyncDepth,
+  V8StackTraceImpl(std::vector<std::shared_ptr<StackFrame>> frames,
+                   int maxAsyncDepth,
                    std::shared_ptr<AsyncStackTrace> asyncParent,
                    std::shared_ptr<AsyncStackTrace> asyncCreation);
 
-  std::vector<Frame> m_frames;
+  std::vector<std::shared_ptr<StackFrame>> m_frames;
   int m_maxAsyncDepth;
   std::weak_ptr<AsyncStackTrace> m_asyncParent;
   std::weak_ptr<AsyncStackTrace> m_asyncCreation;
@@ -102,14 +103,14 @@ class AsyncStackTrace {
 
  private:
   AsyncStackTrace(int contextGroupId, const String16& description,
-                  const std::vector<V8StackTraceImpl::Frame>& frames,
+                  std::vector<std::shared_ptr<StackFrame>> frames,
                   std::shared_ptr<AsyncStackTrace> asyncParent,
                   std::shared_ptr<AsyncStackTrace> asyncCreation);
 
   int m_contextGroupId;
   String16 m_description;
 
-  std::vector<V8StackTraceImpl::Frame> m_frames;
+  std::vector<std::shared_ptr<StackFrame>> m_frames;
   std::weak_ptr<AsyncStackTrace> m_asyncParent;
   std::weak_ptr<AsyncStackTrace> m_asyncCreation;
 

src/isolate.cc

@@ -717,6 +717,7 @@ class CaptureStackTraceHelper {
         AbstractCode::SetStackFrameCache(summ.abstract_code(), new_cache);
       }
     }
+    frame->set_id(next_id());
     return frame;
   }
 
@@ -738,12 +739,19 @@ class CaptureStackTraceHelper {
     info->set_column_number(position);
     info->set_script_id(summ.script()->id());
     info->set_is_wasm(true);
+    info->set_id(next_id());
     return info;
   }
 
  private:
   inline Factory* factory() { return isolate_->factory(); }
 
+  int next_id() const {
+    int id = isolate_->last_stack_frame_info_id() + 1;
+    isolate_->set_last_stack_frame_info_id(id);
+    return id;
+  }
+
   Isolate* isolate_;
 };
 

src/isolate.h

@@ -434,6 +434,7 @@ typedef List<HeapObject*> DebugObjectCache;
   V(bool, needs_side_effect_check, false)                                     \
   /* Current code coverage mode */                                            \
   V(debug::Coverage::Mode, code_coverage_mode, debug::Coverage::kBestEffort)  \
+  V(int, last_stack_frame_info_id, 0)                                         \
   ISOLATE_INIT_SIMULATOR_LIST(V)
 
 #define THREAD_LOCAL_TOP_ACCESSOR(type, name)                        \

src/objects-inl.h

@@ -5899,6 +5899,7 @@ SMI_ACCESSORS(StackFrameInfo, flag, kFlagIndex)
 BOOL_ACCESSORS(StackFrameInfo, flag, is_eval, kIsEvalBit)
 BOOL_ACCESSORS(StackFrameInfo, flag, is_constructor, kIsConstructorBit)
 BOOL_ACCESSORS(StackFrameInfo, flag, is_wasm, kIsWasmBit)
+SMI_ACCESSORS(StackFrameInfo, id, kIdIndex)
 
 ACCESSORS(SourcePositionTableWithFrameCache, source_position_table, ByteArray,
           kSourcePositionTableIndex)

src/objects.h

@@ -10150,6 +10150,7 @@ class StackFrameInfo : public Struct {
   DECL_BOOLEAN_ACCESSORS(is_constructor)
   DECL_BOOLEAN_ACCESSORS(is_wasm)
   DECL_INT_ACCESSORS(flag)
+  DECL_INT_ACCESSORS(id)
 
   DECLARE_CAST(StackFrameInfo)
 
@@ -10166,7 +10167,8 @@ class StackFrameInfo : public Struct {
   static const int kFunctionNameIndex =
       kScriptNameOrSourceUrlIndex + kPointerSize;
   static const int kFlagIndex = kFunctionNameIndex + kPointerSize;
-  static const int kSize = kFlagIndex + kPointerSize;
+  static const int kIdIndex = kFlagIndex + kPointerSize;
+  static const int kSize = kIdIndex + kPointerSize;
 
  private:
   // Bit position in the flag, from least significant bit position.