Reland: [inspector] Allow limiting the total size of collected scripts.

Introduces the setMaxCollectedScriptsSize Debugger protocol method.
If the max size is set, the debugger will hold collected (not referenced by other v8 heap objects)
scripts up to the specified total size of their sources.

> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1518556
> Commit-Queue: Alexei Filippov <alph@chromium.org>
> Reviewed-by: Dmitry Gozman <dgozman@chromium.org>

BUG=v8:8988
TBR=dgozman@chromium.org

Change-Id: I6f7da07c4c9ae35b5252aabddb98b693ec77b4e8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1524662
Reviewed-by: Alexei Filippov <alph@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60255}
This commit is contained in:
Alexei Filippov 2019-03-14 16:51:21 -07:00 committed by Commit Bot
parent 4b796a9093
commit ba00d8b776
12 changed files with 162 additions and 152 deletions

View File

@ -3,11 +3,12 @@ include_rules = [
"-include/v8-debug.h",
"+src/base/atomicops.h",
"+src/base/compiler-specific.h",
"+src/base/macros.h",
"+src/base/logging.h",
"+src/base/v8-fallthrough.h",
"+src/base/macros.h",
"+src/base/platform/platform.h",
"+src/base/platform/mutex.h",
"+src/base/safe_conversions.h",
"+src/base/v8-fallthrough.h",
"+src/conversions.h",
"+src/v8memory.h",
"+src/inspector",

View File

@ -165,6 +165,10 @@ domain Debugger
# Enables debugger for the given page. Clients should not assume that the debugging has been
# enabled until the result for this command is received.
command enable
parameters
# The maximum size in bytes of collected scripts (not referenced by other heap objects)
# the debugger can hold. Puts no limit if paramter is omitted.
experimental optional number maxScriptsCacheSize
returns
# Unique identifier of the debugger.
experimental Runtime.UniqueDebuggerId debuggerId

View File

@ -6,6 +6,7 @@
#include <algorithm>
#include "src/base/safe_conversions.h"
#include "src/debug/debug-interface.h"
#include "src/inspector/injected-script.h"
#include "src/inspector/inspected-context.h"
@ -57,8 +58,6 @@ static const char kDebuggerNotPaused[] =
static const size_t kBreakpointHintMaxLength = 128;
static const intptr_t kBreakpointHintMaxSearchOffset = 80 * 10;
static const int kMaxScriptFailedToParseScripts = 1000;
namespace {
void TranslateLocation(protocol::Debugger::Location* location,
@ -220,8 +219,6 @@ String16 breakLocationType(v8::debug::BreakLocationType type) {
return String16();
}
} // namespace
String16 scopeType(v8::debug::ScopeIterator::ScopeType type) {
switch (type) {
case v8::debug::ScopeIterator::ScopeTypeGlobal:
@ -247,8 +244,6 @@ String16 scopeType(v8::debug::ScopeIterator::ScopeType type) {
return String16();
}
namespace {
Response buildScopes(v8::Isolate* isolate, v8::debug::ScopeIterator* iterator,
InjectedScript* injectedScript,
std::unique_ptr<Array<Scope>>* scopes) {
@ -324,10 +319,11 @@ void V8DebuggerAgentImpl::enableImpl() {
m_state->setBoolean(DebuggerAgentState::debuggerEnabled, true);
m_debugger->enable();
std::vector<std::unique_ptr<V8DebuggerScript>> compiledScripts;
m_debugger->getCompiledScripts(m_session->contextGroupId(), compiledScripts);
for (size_t i = 0; i < compiledScripts.size(); i++)
didParseSource(std::move(compiledScripts[i]), true);
std::vector<std::unique_ptr<V8DebuggerScript>> compiledScripts =
m_debugger->getCompiledScripts(m_session->contextGroupId(), this);
for (auto& script : compiledScripts) {
didParseSource(std::move(script), true);
}
m_breakpointsActive = true;
m_debugger->setBreakpointsActive(true);
@ -338,7 +334,10 @@ void V8DebuggerAgentImpl::enableImpl() {
}
}
Response V8DebuggerAgentImpl::enable(String16* outDebuggerId) {
Response V8DebuggerAgentImpl::enable(Maybe<double> maxScriptsCacheSize,
String16* outDebuggerId) {
m_maxScriptsCacheSize = v8::base::saturated_cast<size_t>(
maxScriptsCacheSize.fromMaybe(std::numeric_limits<double>::max()));
*outDebuggerId = debuggerIdToString(
m_debugger->debuggerIdFor(m_session->contextGroupId()));
if (enabled()) return Response::OK();
@ -370,6 +369,8 @@ Response V8DebuggerAgentImpl::disable() {
m_blackboxPattern.reset();
resetBlackboxedStateCache();
m_scripts.clear();
m_collectedScriptIds.clear();
m_collectedScriptsSize = 0;
for (const auto& it : m_debuggerBreakpointIdToBreakpointId) {
v8::debug::RemoveBreakpoint(m_isolate, it.first);
}
@ -1394,6 +1395,9 @@ void V8DebuggerAgentImpl::didParseSource(
String16 scriptURL = script->sourceURL();
m_scripts[scriptId] = std::move(script);
// Release the strong reference to get notified when debugger is the only
// one that holds the script.
m_scripts[scriptId]->MakeWeak();
ScriptsMap::iterator scriptIterator = m_scripts.find(scriptId);
DCHECK(scriptIterator != m_scripts.end());
@ -1417,38 +1421,32 @@ void V8DebuggerAgentImpl::didParseSource(
stack && !stack->isEmpty()
? stack->buildInspectorObjectImpl(m_debugger, 0)
: nullptr;
if (success) {
// TODO(herhut, dgozman): Report correct length for WASM if needed for
// coverage. Or do not send the length at all and change coverage instead.
if (scriptRef->isSourceLoadedLazily()) {
m_frontend.scriptParsed(
scriptId, scriptURL, 0, 0, 0, 0, contextId, scriptRef->hash(),
std::move(executionContextAuxDataParam), isLiveEditParam,
std::move(sourceMapURLParam), hasSourceURLParam, isModuleParam, 0,
std::move(stackTrace));
} else {
m_frontend.scriptParsed(
scriptId, scriptURL, scriptRef->startLine(), scriptRef->startColumn(),
scriptRef->endLine(), scriptRef->endColumn(), contextId,
scriptRef->hash(), std::move(executionContextAuxDataParam),
isLiveEditParam, std::move(sourceMapURLParam), hasSourceURLParam,
isModuleParam, scriptRef->length(), std::move(stackTrace));
}
} else {
if (!success) {
m_frontend.scriptFailedToParse(
scriptId, scriptURL, scriptRef->startLine(), scriptRef->startColumn(),
scriptRef->endLine(), scriptRef->endColumn(), contextId,
scriptRef->hash(), std::move(executionContextAuxDataParam),
std::move(sourceMapURLParam), hasSourceURLParam, isModuleParam,
scriptRef->length(), std::move(stackTrace));
return;
}
if (!success) {
if (scriptURL.isEmpty()) {
m_failedToParseAnonymousScriptIds.push_back(scriptId);
cleanupOldFailedToParseAnonymousScriptsIfNeeded();
}
return;
// TODO(herhut, dgozman): Report correct length for WASM if needed for
// coverage. Or do not send the length at all and change coverage instead.
if (scriptRef->isSourceLoadedLazily()) {
m_frontend.scriptParsed(
scriptId, scriptURL, 0, 0, 0, 0, contextId, scriptRef->hash(),
std::move(executionContextAuxDataParam), isLiveEditParam,
std::move(sourceMapURLParam), hasSourceURLParam, isModuleParam, 0,
std::move(stackTrace));
} else {
m_frontend.scriptParsed(
scriptId, scriptURL, scriptRef->startLine(), scriptRef->startColumn(),
scriptRef->endLine(), scriptRef->endColumn(), contextId,
scriptRef->hash(), std::move(executionContextAuxDataParam),
isLiveEditParam, std::move(sourceMapURLParam), hasSourceURLParam,
isModuleParam, scriptRef->length(), std::move(stackTrace));
}
std::vector<protocol::DictionaryValue*> potentialBreakpoints;
@ -1646,20 +1644,23 @@ void V8DebuggerAgentImpl::reset() {
m_blackboxedPositions.clear();
resetBlackboxedStateCache();
m_scripts.clear();
m_collectedScriptIds.clear();
m_collectedScriptsSize = 0;
m_breakpointIdToDebuggerBreakpointIds.clear();
}
void V8DebuggerAgentImpl::cleanupOldFailedToParseAnonymousScriptsIfNeeded() {
if (m_failedToParseAnonymousScriptIds.size() <=
kMaxScriptFailedToParseScripts)
return;
static_assert(kMaxScriptFailedToParseScripts > 100,
"kMaxScriptFailedToParseScripts should be greater then 100");
while (m_failedToParseAnonymousScriptIds.size() >
kMaxScriptFailedToParseScripts - 100 + 1) {
String16 scriptId = m_failedToParseAnonymousScriptIds.front();
m_failedToParseAnonymousScriptIds.pop_front();
void V8DebuggerAgentImpl::ScriptCollected(const V8DebuggerScript* script) {
DCHECK_NE(m_scripts.find(script->scriptId()), m_scripts.end());
m_collectedScriptIds.push_back(script->scriptId());
m_collectedScriptsSize += script->length() * sizeof(uint16_t);
while (m_collectedScriptsSize > m_maxScriptsCacheSize) {
const String16& scriptId = m_collectedScriptIds.front();
size_t scriptSize = m_scripts[scriptId]->length() * sizeof(uint16_t);
DCHECK_GE(m_collectedScriptsSize, scriptSize);
m_collectedScriptsSize -= scriptSize;
m_scripts.erase(scriptId);
m_collectedScriptIds.pop_front();
}
}

View File

@ -41,7 +41,8 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
void restore();
// Part of the protocol.
Response enable(String16* outDebuggerId) override;
Response enable(Maybe<double> maxScriptsCacheSize,
String16* outDebuggerId) override;
Response disable() override;
Response setBreakpointsActive(bool active) override;
Response setSkipAllPauses(bool skip) override;
@ -151,6 +152,8 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
bool acceptsPause(bool isOOMBreak) const;
void ScriptCollected(const V8DebuggerScript* script);
v8::Isolate* isolate() { return m_isolate; }
private:
@ -199,8 +202,9 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
BreakpointIdToDebuggerBreakpointIdsMap m_breakpointIdToDebuggerBreakpointIds;
DebuggerBreakpointIdToBreakpointIdMap m_debuggerBreakpointIdToBreakpointId;
std::deque<String16> m_failedToParseAnonymousScriptIds;
void cleanupOldFailedToParseAnonymousScriptsIfNeeded();
size_t m_maxScriptsCacheSize = 0;
size_t m_collectedScriptsSize = 0;
std::deque<String16> m_collectedScriptIds;
using BreakReason =
std::pair<String16, std::unique_ptr<protocol::DictionaryValue>>;
@ -221,8 +225,6 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
DISALLOW_COPY_AND_ASSIGN(V8DebuggerAgentImpl);
};
String16 scopeType(v8::debug::ScopeIterator::ScopeType type);
} // namespace v8_inspector
#endif // V8_INSPECTOR_V8_DEBUGGER_AGENT_IMPL_H_

View File

@ -6,6 +6,7 @@
#include "src/inspector/inspected-context.h"
#include "src/inspector/string-util.h"
#include "src/inspector/v8-debugger-agent-impl.h"
#include "src/inspector/v8-inspector-impl.h"
#include "src/inspector/wasm-translation.h"
#include "src/v8memory.h"
@ -116,9 +117,11 @@ class ActualScript : public V8DebuggerScript {
public:
ActualScript(v8::Isolate* isolate, v8::Local<v8::debug::Script> script,
bool isLiveEdit, V8InspectorClient* client)
bool isLiveEdit, V8DebuggerAgentImpl* agent,
V8InspectorClient* client)
: V8DebuggerScript(isolate, String16::fromInteger(script->Id()),
GetScriptURL(isolate, script, client)),
m_agent(agent),
m_isLiveEdit(isLiveEdit) {
Initialize(script);
}
@ -146,8 +149,7 @@ class ActualScript : public V8DebuggerScript {
int length() const override {
v8::HandleScope scope(m_isolate);
v8::Local<v8::String> v8Source;
if (!script()->Source().ToLocal(&v8Source)) return 0;
return v8Source->Length();
return script()->Source().ToLocal(&v8Source) ? v8Source->Length() : 0;
}
const String16& sourceMappingURL() const override {
@ -234,21 +236,20 @@ class ActualScript : public V8DebuggerScript {
}
const String16& hash() const override {
if (m_hash.isEmpty()) {
v8::HandleScope scope(m_isolate);
v8::Local<v8::String> v8Source;
if (script()->Source().ToLocal(&v8Source)) {
m_hash = calculateHash(m_isolate, v8Source);
}
if (!m_hash.isEmpty()) return m_hash;
v8::HandleScope scope(m_isolate);
v8::Local<v8::String> v8Source;
if (script()->Source().ToLocal(&v8Source)) {
m_hash = calculateHash(m_isolate, v8Source);
}
DCHECK(!m_hash.isEmpty());
return m_hash;
}
private:
String16 GetScriptURL(v8::Isolate* isolate,
v8::Local<v8::debug::Script> script,
V8InspectorClient* client) {
static String16 GetScriptURL(v8::Isolate* isolate,
v8::Local<v8::debug::Script> script,
V8InspectorClient* client) {
v8::Local<v8::String> sourceURL;
if (script->SourceURL().ToLocal(&sourceURL) && sourceURL->Length() > 0)
return toProtocolString(isolate, sourceURL);
@ -297,6 +298,21 @@ class ActualScript : public V8DebuggerScript {
m_script.AnnotateStrongRetainer(kGlobalDebuggerScriptHandleLabel);
}
void MakeWeak() override {
m_script.SetWeak(
this,
[](const v8::WeakCallbackInfo<ActualScript>& data) {
data.GetParameter()->WeakCallback();
},
v8::WeakCallbackType::kFinalizer);
}
void WeakCallback() {
m_script.ClearWeak();
m_agent->ScriptCollected(this);
}
V8DebuggerAgentImpl* m_agent;
String16 m_sourceMappingURL;
bool m_isLiveEdit = false;
bool m_isModule = false;
@ -415,6 +431,8 @@ class WasmVirtualScript : public V8DebuggerScript {
return m_hash;
}
void MakeWeak() override {}
private:
static const String16& emptyString() {
// On the heap and leaked so that no destructor needs to run at exit time.
@ -436,18 +454,18 @@ class WasmVirtualScript : public V8DebuggerScript {
std::unique_ptr<V8DebuggerScript> V8DebuggerScript::Create(
v8::Isolate* isolate, v8::Local<v8::debug::Script> scriptObj,
bool isLiveEdit, V8InspectorClient* client) {
return std::unique_ptr<ActualScript>(
new ActualScript(isolate, scriptObj, isLiveEdit, client));
bool isLiveEdit, V8DebuggerAgentImpl* agent, V8InspectorClient* client) {
return v8::base::make_unique<ActualScript>(isolate, scriptObj, isLiveEdit,
agent, client);
}
std::unique_ptr<V8DebuggerScript> V8DebuggerScript::CreateWasm(
v8::Isolate* isolate, WasmTranslation* wasmTranslation,
v8::Local<v8::debug::WasmScript> underlyingScript, String16 id,
String16 url, int functionIndex) {
return std::unique_ptr<WasmVirtualScript>(
new WasmVirtualScript(isolate, wasmTranslation, underlyingScript,
std::move(id), std::move(url), functionIndex));
return v8::base::make_unique<WasmVirtualScript>(
isolate, wasmTranslation, underlyingScript, std::move(id), std::move(url),
functionIndex);
}
V8DebuggerScript::V8DebuggerScript(v8::Isolate* isolate, String16 id,
@ -468,4 +486,5 @@ bool V8DebuggerScript::setBreakpoint(const String16& condition,
v8::HandleScope scope(m_isolate);
return script()->SetBreakpoint(toV8String(m_isolate, condition), loc, id);
}
} // namespace v8_inspector

View File

@ -39,7 +39,7 @@
namespace v8_inspector {
// Forward declaration.
class V8DebuggerAgentImpl;
class V8InspectorClient;
class WasmTranslation;
@ -47,7 +47,7 @@ class V8DebuggerScript {
public:
static std::unique_ptr<V8DebuggerScript> Create(
v8::Isolate* isolate, v8::Local<v8::debug::Script> script,
bool isLiveEdit, V8InspectorClient* client);
bool isLiveEdit, V8DebuggerAgentImpl* agent, V8InspectorClient* client);
static std::unique_ptr<V8DebuggerScript> CreateWasm(
v8::Isolate* isolate, WasmTranslation* wasmTranslation,
v8::Local<v8::debug::WasmScript> underlyingScript, String16 id,
@ -89,6 +89,7 @@ class V8DebuggerScript {
virtual bool setBreakpoint(const String16& condition,
v8::debug::Location* location, int* id) const = 0;
virtual void MakeWeak() = 0;
protected:
V8DebuggerScript(v8::Isolate*, String16 id, String16 url);

View File

@ -120,26 +120,24 @@ bool V8Debugger::isPausedInContextGroup(int contextGroupId) const {
bool V8Debugger::enabled() const { return m_enableCount > 0; }
void V8Debugger::getCompiledScripts(
int contextGroupId,
std::vector<std::unique_ptr<V8DebuggerScript>>& result) {
std::vector<std::unique_ptr<V8DebuggerScript>> V8Debugger::getCompiledScripts(
int contextGroupId, V8DebuggerAgentImpl* agent) {
std::vector<std::unique_ptr<V8DebuggerScript>> result;
v8::HandleScope scope(m_isolate);
v8::PersistentValueVector<v8::debug::Script> scripts(m_isolate);
v8::debug::GetLoadedScripts(m_isolate, scripts);
for (size_t i = 0; i < scripts.Size(); ++i) {
v8::Local<v8::debug::Script> script = scripts.Get(i);
if (!script->WasCompiled()) continue;
if (script->IsEmbedded()) {
result.push_back(V8DebuggerScript::Create(m_isolate, script, false,
m_inspector->client()));
continue;
if (!script->IsEmbedded()) {
int contextId;
if (!script->ContextId().To(&contextId)) continue;
if (m_inspector->contextGroupId(contextId) != contextGroupId) continue;
}
int contextId;
if (!script->ContextId().To(&contextId)) continue;
if (m_inspector->contextGroupId(contextId) != contextGroupId) continue;
result.push_back(V8DebuggerScript::Create(m_isolate, script, false,
result.push_back(V8DebuggerScript::Create(m_isolate, script, false, agent,
m_inspector->client()));
}
return result;
}
void V8Debugger::setBreakpointsActive(bool active) {
@ -490,7 +488,8 @@ void V8Debugger::ScriptCompiled(v8::Local<v8::debug::Script> script,
&client](V8InspectorSessionImpl* session) {
if (!session->debuggerAgent()->enabled()) return;
session->debuggerAgent()->didParseSource(
V8DebuggerScript::Create(isolate, script, is_live_edited, client),
V8DebuggerScript::Create(isolate, script, is_live_edited,
session->debuggerAgent(), client),
!has_compile_error);
});
}

View File

@ -73,8 +73,8 @@ class V8Debugger : public v8::debug::DebugDelegate,
// compiled.
// Only scripts whose debug data matches |contextGroupId| will be reported.
// Passing 0 will result in reporting all scripts.
void getCompiledScripts(int contextGroupId,
std::vector<std::unique_ptr<V8DebuggerScript>>&);
std::vector<std::unique_ptr<V8DebuggerScript>> getCompiledScripts(
int contextGroupId, V8DebuggerAgentImpl* agent);
void enable();
void disable();

View File

@ -1,17 +0,0 @@
Checks that inspector collects old faied to parse anonymous scripts.
Generate 1000 scriptFailedToParse events
error:0
success:1000
Generate three scriptFailedToParse event for non anonymous script
error:0
success:1003
Generate one more scriptFailedToParse event for anonymous script
error:100
success:904
Check that latest script is still available
{
id : <messageId>
result : {
scriptSource : }
}
}

View File

@ -1,49 +0,0 @@
// Copyright 2018 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
let {session, contextGroup, Protocol} = InspectorTest.start(
'Checks that inspector collects old faied to parse anonymous scripts.');
(async function main() {
Protocol.Debugger.enable();
const scriptIds = [];
Protocol.Debugger.onScriptFailedToParse(
message => scriptIds.push(message.params.scriptId));
InspectorTest.log('Generate 1000 scriptFailedToParse events');
await Protocol.Runtime.evaluate({
expression: `for (var i = 0; i < 1000; ++i) {
try { JSON.parse('}'); } catch(e) {}
}`
});
await dumpScriptIdsStats(scriptIds);
InspectorTest.log(
'Generate three scriptFailedToParse event for non anonymous script');
for (var i = 0; i < 3; ++i) {
await Protocol.Runtime.evaluate({expression: '}//# sourceURL=foo.js'});
}
await dumpScriptIdsStats(scriptIds);
InspectorTest.log(
'Generate one more scriptFailedToParse event for anonymous script');
await Protocol.Runtime.evaluate(
{expression: `try {JSON.parse('}');} catch(e){}`});
await dumpScriptIdsStats(scriptIds);
InspectorTest.log('Check that latest script is still available');
InspectorTest.logMessage(await Protocol.Debugger.getScriptSource(
{scriptId: scriptIds[scriptIds.length - 1]}));
InspectorTest.completeTest();
})();
async function dumpScriptIdsStats(scriptIds) {
let errors = 0;
let success = 0;
for (let scriptId of scriptIds) {
const result =
await Protocol.Debugger.getScriptSource({scriptId: scriptId});
if (result.error)
++errors;
else
++success;
}
InspectorTest.log(`error:${errors}\nsuccess:${success}`);
}

View File

@ -0,0 +1,8 @@
Checks that inspector collects old collected scripts.
Generate 5 scripts 1MB each
Generate 30 more scripts 1MB each
Check that latest script is still available
Last script length: 500009
Check that an earlier script is not available
Script is not found: true

View File

@ -0,0 +1,41 @@
// Copyright 2019 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
let {session, contextGroup, Protocol} = InspectorTest.start(
'Checks that inspector collects old collected scripts.\n');
(async function main() {
const maxScriptsCacheSize = 10e6;
Protocol.Debugger.enable({maxScriptsCacheSize});
const scriptIds = [];
Protocol.Debugger.onScriptParsed(message => scriptIds.push(message.params.scriptId));
InspectorTest.log('Generate 5 scripts 1MB each');
await Protocol.Runtime.evaluate({
expression: `for (let i = 0; i < 5; ++i) {
eval("'" + new Array(1e5).fill(12345).join('') + "'.length");
}`
});
const aScriptId = scriptIds[scriptIds.length - 1];
InspectorTest.log('Generate 30 more scripts 1MB each');
await Protocol.Runtime.evaluate({
expression: `for (let i = 0; i < 30; ++i) {
eval("'" + new Array(1e5).fill(12345).join('') + "'.length");
}`
});
await Protocol.HeapProfiler.collectGarbage();
InspectorTest.log('Check that latest script is still available');
let result = await Protocol.Debugger.getScriptSource({scriptId: scriptIds[scriptIds.length - 1]});
InspectorTest.logMessage(`Last script length: ${result.result && result.result.scriptSource.length}`);
InspectorTest.log('Check that an earlier script is not available');
result = await Protocol.Debugger.getScriptSource({scriptId: aScriptId});
InspectorTest.logMessage(`Script is not found: ${result.error && result.error.message.includes('No script for id')}`);
InspectorTest.completeTest();
})();