Commit e60dc99e authored by Benedikt Meurer's avatar Benedikt Meurer Committed by V8 LUCI CQ

[inspector] Cache StackFrames by script, line and column number.

This introduces a stack frame cache on the V8Debugger level, which
de-duplicates StackFrame instances based on their scriptId, line and
column number.

This greatly reduces the memory pressure when debugging huge Web
applications that have a lot of async activity (and potentially
have scripts with huge URLs). This is guided by the observation
that even in huge applications, there are only a very limited
number of call sites that initiate async activity and hence we
only have a limited number of distinct StackFrames to worry
about (despite having to maintain a large number of async stack
traces overall).

As a nice side effect, this CL also greatly reduces the negative
performance impact of collecting async stack traces in these
huge applications.

Generally speaking this is mostly duct tape however, and we might
want to follow up with changes to make capturing (and storing)
stack frames even cheaper.

Fixed: chromium:1268436
Change-Id: Ib212b3c97dce2bb7ca47d5875d45cf20b9b97afe
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3272577
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77835}
parent f9116dee
...@@ -1124,6 +1124,7 @@ void V8Debugger::collectOldAsyncStacksIfNeeded() { ...@@ -1124,6 +1124,7 @@ void V8Debugger::collectOldAsyncStacksIfNeeded() {
m_allAsyncStacks.pop_front(); m_allAsyncStacks.pop_front();
} }
cleanupExpiredWeakPointers(m_asyncTaskStacks); cleanupExpiredWeakPointers(m_asyncTaskStacks);
cleanupExpiredWeakPointers(m_cachedStackFrames);
cleanupExpiredWeakPointers(m_storedStackTraces); cleanupExpiredWeakPointers(m_storedStackTraces);
for (auto it = m_recurringTasks.begin(); it != m_recurringTasks.end();) { for (auto it = m_recurringTasks.begin(); it != m_recurringTasks.end();) {
if (m_asyncTaskStacks.find(*it) == m_asyncTaskStacks.end()) { if (m_asyncTaskStacks.find(*it) == m_asyncTaskStacks.end()) {
...@@ -1136,8 +1137,31 @@ void V8Debugger::collectOldAsyncStacksIfNeeded() { ...@@ -1136,8 +1137,31 @@ void V8Debugger::collectOldAsyncStacksIfNeeded() {
std::shared_ptr<StackFrame> V8Debugger::symbolize( std::shared_ptr<StackFrame> V8Debugger::symbolize(
v8::Local<v8::StackFrame> v8Frame) { v8::Local<v8::StackFrame> v8Frame) {
CHECK(!v8Frame.IsEmpty()); int scriptId = v8Frame->GetScriptId();
return std::make_shared<StackFrame>(isolate(), v8Frame); int lineNumber = v8Frame->GetLineNumber() - 1;
int columnNumber = v8Frame->GetColumn() - 1;
CachedStackFrameKey key{scriptId, lineNumber, columnNumber};
auto it = m_cachedStackFrames.find(key);
if (it != m_cachedStackFrames.end() && !it->second.expired()) {
auto stackFrame = it->second.lock();
DCHECK_EQ(
stackFrame->functionName(),
toProtocolString(isolate(), v8::debug::GetFunctionDebugName(v8Frame)));
DCHECK_EQ(stackFrame->sourceURL(),
toProtocolString(isolate(), v8Frame->GetScriptNameOrSourceURL()));
return stackFrame;
}
auto functionName =
toProtocolString(isolate(), v8::debug::GetFunctionDebugName(v8Frame));
auto sourceURL =
toProtocolString(isolate(), v8Frame->GetScriptNameOrSourceURL());
auto hasSourceURLComment =
v8Frame->GetScriptName() != v8Frame->GetScriptNameOrSourceURL();
auto stackFrame = std::make_shared<StackFrame>(
std::move(functionName), scriptId, std::move(sourceURL), lineNumber,
columnNumber, hasSourceURLComment);
m_cachedStackFrames.emplace(key, stackFrame);
return stackFrame;
} }
void V8Debugger::setMaxAsyncTaskStacksForTest(int limit) { void V8Debugger::setMaxAsyncTaskStacksForTest(int limit) {
......
...@@ -210,6 +210,36 @@ class V8Debugger : public v8::debug::DebugDelegate, ...@@ -210,6 +210,36 @@ class V8Debugger : public v8::debug::DebugDelegate,
String16 m_continueToLocationTargetCallFrames; String16 m_continueToLocationTargetCallFrames;
std::unique_ptr<V8StackTraceImpl> m_continueToLocationStack; std::unique_ptr<V8StackTraceImpl> m_continueToLocationStack;
// We cache symbolized stack frames by (scriptId,lineNumber,columnNumber)
// to reduce memory pressure for huge web apps with lots of deep async
// stacks.
struct CachedStackFrameKey {
int scriptId;
int lineNumber;
int columnNumber;
struct Equal {
bool operator()(CachedStackFrameKey const& a,
CachedStackFrameKey const& b) const {
return a.scriptId == b.scriptId && a.lineNumber == b.lineNumber &&
a.columnNumber == b.columnNumber;
}
};
struct Hash {
size_t operator()(CachedStackFrameKey const& key) const {
size_t code = 0;
code = code * 31 + key.scriptId;
code = code * 31 + key.lineNumber;
code = code * 31 + key.columnNumber;
return code;
}
};
};
std::unordered_map<CachedStackFrameKey, std::weak_ptr<StackFrame>,
CachedStackFrameKey::Hash, CachedStackFrameKey::Equal>
m_cachedStackFrames;
using AsyncTaskToStackTrace = using AsyncTaskToStackTrace =
std::unordered_map<void*, std::weak_ptr<AsyncStackTrace>>; std::unordered_map<void*, std::weak_ptr<AsyncStackTrace>>;
AsyncTaskToStackTrace m_asyncTaskStacks; AsyncTaskToStackTrace m_asyncTaskStacks;
......
...@@ -168,16 +168,15 @@ std::unique_ptr<StringBuffer> V8StackTraceId::ToString() { ...@@ -168,16 +168,15 @@ std::unique_ptr<StringBuffer> V8StackTraceId::ToString() {
return StringBufferFrom(std::move(json)); return StringBufferFrom(std::move(json));
} }
StackFrame::StackFrame(v8::Isolate* isolate, v8::Local<v8::StackFrame> v8Frame) StackFrame::StackFrame(String16&& functionName, int scriptId,
: m_functionName( String16&& sourceURL, int lineNumber, int columnNumber,
toProtocolString(isolate, v8::debug::GetFunctionDebugName(v8Frame))), bool hasSourceURLComment)
m_scriptId(v8Frame->GetScriptId()), : m_functionName(std::move(functionName)),
m_sourceURL( m_scriptId(scriptId),
toProtocolString(isolate, v8Frame->GetScriptNameOrSourceURL())), m_sourceURL(std::move(sourceURL)),
m_lineNumber(v8Frame->GetLineNumber() - 1), m_lineNumber(lineNumber),
m_columnNumber(v8Frame->GetColumn() - 1), m_columnNumber(columnNumber),
m_hasSourceURLComment(v8Frame->GetScriptName() != m_hasSourceURLComment(hasSourceURLComment) {
v8Frame->GetScriptNameOrSourceURL()) {
DCHECK_NE(v8::Message::kNoLineNumberInfo, m_lineNumber + 1); DCHECK_NE(v8::Message::kNoLineNumberInfo, m_lineNumber + 1);
DCHECK_NE(v8::Message::kNoColumnInfo, m_columnNumber + 1); DCHECK_NE(v8::Message::kNoColumnInfo, m_columnNumber + 1);
} }
......
...@@ -27,7 +27,8 @@ struct V8StackTraceId; ...@@ -27,7 +27,8 @@ struct V8StackTraceId;
class StackFrame { class StackFrame {
public: public:
explicit StackFrame(v8::Isolate* isolate, v8::Local<v8::StackFrame> frame); StackFrame(String16&& functionName, int scriptId, String16&& sourceURL,
int lineNumber, int columnNumber, bool hasSourceURLComment);
~StackFrame() = default; ~StackFrame() = default;
const String16& functionName() const; const String16& functionName() const;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment