Revert "[inspector] Allow limiting the total size of collected scripts."

This reverts commit 5a61630d1d.

Reason for revert: Breaking gc stress bot - https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20gc%20stress/21477

Original change's description:
> [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.
> 
> BUG=v8:8988
> 
> Change-Id: I94d52866494102add91ca2d569a2044b08c9c593
> 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>
> Cr-Commit-Position: refs/heads/master@{#60227}

TBR=dgozman@chromium.org,alph@chromium.org,kozyatinskiy@chromium.org

Change-Id: I26de645e425f0f7d5aa8212eeefda76dad695b78
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8988
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1522988
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60229}
This commit is contained in:
Maya Lekova 2019-03-14 08:23:01 +00:00 committed by Commit Bot
parent 1f6bccf428
commit 71206891a4
12 changed files with 153 additions and 155 deletions

View File

@ -3,12 +3,11 @@ include_rules = [
"-include/v8-debug.h",
"+src/base/atomicops.h",
"+src/base/compiler-specific.h",
"+src/base/logging.h",
"+src/base/macros.h",
"+src/base/logging.h",
"+src/base/v8-fallthrough.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,10 +165,6 @@ 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 maxCollectedScriptsSize
returns
# Unique identifier of the debugger.
experimental Runtime.UniqueDebuggerId debuggerId

View File

@ -6,7 +6,6 @@
#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"
@ -58,6 +57,8 @@ 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,
@ -219,6 +220,8 @@ String16 breakLocationType(v8::debug::BreakLocationType type) {
return String16();
}
} // namespace
String16 scopeType(v8::debug::ScopeIterator::ScopeType type) {
switch (type) {
case v8::debug::ScopeIterator::ScopeTypeGlobal:
@ -244,6 +247,8 @@ 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) {
@ -319,11 +324,10 @@ 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(), this);
for (auto& script : compiledScripts) {
didParseSource(std::move(script), true);
}
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);
m_breakpointsActive = true;
m_debugger->setBreakpointsActive(true);
@ -334,10 +338,7 @@ void V8DebuggerAgentImpl::enableImpl() {
}
}
Response V8DebuggerAgentImpl::enable(Maybe<double> maxCollectedScriptsSize,
String16* outDebuggerId) {
m_maxCollectedScriptsSize = v8::base::saturated_cast<size_t>(
maxCollectedScriptsSize.fromMaybe(std::numeric_limits<double>::max()));
Response V8DebuggerAgentImpl::enable(String16* outDebuggerId) {
*outDebuggerId = debuggerIdToString(
m_debugger->debuggerIdFor(m_session->contextGroupId()));
if (enabled()) return Response::OK();
@ -369,8 +370,6 @@ 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);
}
@ -1418,17 +1417,7 @@ void V8DebuggerAgentImpl::didParseSource(
stack && !stack->isEmpty()
? stack->buildInspectorObjectImpl(m_debugger, 0)
: nullptr;
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) {
// 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()) {
@ -1445,6 +1434,22 @@ void V8DebuggerAgentImpl::didParseSource(
isLiveEditParam, std::move(sourceMapURLParam), hasSourceURLParam,
isModuleParam, scriptRef->length(), std::move(stackTrace));
}
} else {
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));
}
if (!success) {
if (scriptURL.isEmpty()) {
m_failedToParseAnonymousScriptIds.push_back(scriptId);
cleanupOldFailedToParseAnonymousScriptsIfNeeded();
}
return;
}
std::vector<protocol::DictionaryValue*> potentialBreakpoints;
if (!scriptURL.isEmpty()) {
@ -1641,24 +1646,20 @@ void V8DebuggerAgentImpl::reset() {
m_blackboxedPositions.clear();
resetBlackboxedStateCache();
m_scripts.clear();
m_collectedScriptIds.clear();
m_collectedScriptsSize = 0;
m_breakpointIdToDebuggerBreakpointIds.clear();
}
void V8DebuggerAgentImpl::scriptCollected(const String16& scriptId) {
auto it = m_scripts.find(scriptId);
DCHECK_NE(it, m_scripts.end());
m_collectedScriptIds.push_back(scriptId);
m_collectedScriptsSize += it->second->length() * sizeof(uint16_t);
while (m_collectedScriptsSize > m_maxCollectedScriptsSize) {
const String16& scriptIdToRemove = m_collectedScriptIds.front();
size_t scriptSize =
m_scripts[scriptIdToRemove]->length() * sizeof(uint16_t);
DCHECK_GE(m_collectedScriptsSize, scriptSize);
m_collectedScriptsSize -= scriptSize;
m_scripts.erase(scriptIdToRemove);
m_collectedScriptIds.pop_front();
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();
m_scripts.erase(scriptId);
}
}

View File

@ -41,8 +41,7 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
void restore();
// Part of the protocol.
Response enable(Maybe<double> maxCollectedScriptsSize,
String16* outDebuggerId) override;
Response enable(String16* outDebuggerId) override;
Response disable() override;
Response setBreakpointsActive(bool active) override;
Response setSkipAllPauses(bool skip) override;
@ -152,8 +151,6 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
bool acceptsPause(bool isOOMBreak) const;
void scriptCollected(const String16& scriptId);
v8::Isolate* isolate() { return m_isolate; }
private:
@ -202,9 +199,8 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
BreakpointIdToDebuggerBreakpointIdsMap m_breakpointIdToDebuggerBreakpointIds;
DebuggerBreakpointIdToBreakpointIdMap m_debuggerBreakpointIdToBreakpointId;
size_t m_maxCollectedScriptsSize = 0;
size_t m_collectedScriptsSize = 0;
std::deque<String16> m_collectedScriptIds;
std::deque<String16> m_failedToParseAnonymousScriptIds;
void cleanupOldFailedToParseAnonymousScriptsIfNeeded();
using BreakReason =
std::pair<String16, std::unique_ptr<protocol::DictionaryValue>>;
@ -225,6 +221,8 @@ 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,7 +6,6 @@
#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"
@ -117,11 +116,9 @@ class ActualScript : public V8DebuggerScript {
public:
ActualScript(v8::Isolate* isolate, v8::Local<v8::debug::Script> script,
bool isLiveEdit, V8DebuggerAgentImpl* agent,
V8InspectorClient* client)
bool isLiveEdit, V8InspectorClient* client)
: V8DebuggerScript(isolate, String16::fromInteger(script->Id()),
GetScriptURL(isolate, script, client)),
m_agent(agent),
m_isLiveEdit(isLiveEdit) {
Initialize(script);
}
@ -149,7 +146,8 @@ class ActualScript : public V8DebuggerScript {
int length() const override {
v8::HandleScope scope(m_isolate);
v8::Local<v8::String> v8Source;
return script()->Source().ToLocal(&v8Source) ? v8Source->Length() : 0;
if (!script()->Source().ToLocal(&v8Source)) return 0;
return v8Source->Length();
}
const String16& sourceMappingURL() const override {
@ -236,18 +234,19 @@ class ActualScript : public V8DebuggerScript {
}
const String16& hash() const override {
if (!m_hash.isEmpty()) return m_hash;
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);
}
}
DCHECK(!m_hash.isEmpty());
return m_hash;
}
private:
static String16 GetScriptURL(v8::Isolate* isolate,
String16 GetScriptURL(v8::Isolate* isolate,
v8::Local<v8::debug::Script> script,
V8InspectorClient* client) {
v8::Local<v8::String> sourceURL;
@ -295,21 +294,9 @@ class ActualScript : public V8DebuggerScript {
m_isModule = script->IsModule();
m_script.Reset(m_isolate, script);
m_script.SetWeak(
this,
[](const v8::WeakCallbackInfo<ActualScript>& data) {
data.GetParameter()->WeakCallback();
},
v8::WeakCallbackType::kFinalizer);
}
void WeakCallback() {
m_script.ClearWeak();
m_script.AnnotateStrongRetainer(kGlobalDebuggerScriptHandleLabel);
m_agent->scriptCollected(m_id);
}
V8DebuggerAgentImpl* m_agent;
String16 m_sourceMappingURL;
bool m_isLiveEdit = false;
bool m_isModule = false;
@ -449,18 +436,18 @@ class WasmVirtualScript : public V8DebuggerScript {
std::unique_ptr<V8DebuggerScript> V8DebuggerScript::Create(
v8::Isolate* isolate, v8::Local<v8::debug::Script> scriptObj,
bool isLiveEdit, V8DebuggerAgentImpl* agent, V8InspectorClient* client) {
return v8::base::make_unique<ActualScript>(isolate, scriptObj, isLiveEdit,
agent, client);
bool isLiveEdit, V8InspectorClient* client) {
return std::unique_ptr<ActualScript>(
new ActualScript(isolate, scriptObj, isLiveEdit, 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 v8::base::make_unique<WasmVirtualScript>(
isolate, wasmTranslation, underlyingScript, std::move(id), std::move(url),
functionIndex);
return std::unique_ptr<WasmVirtualScript>(
new WasmVirtualScript(isolate, wasmTranslation, underlyingScript,
std::move(id), std::move(url), functionIndex));
}
V8DebuggerScript::V8DebuggerScript(v8::Isolate* isolate, String16 id,
@ -481,5 +468,4 @@ 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 {
class V8DebuggerAgentImpl;
// Forward declaration.
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, V8DebuggerAgentImpl* agent, V8InspectorClient* client);
bool isLiveEdit, V8InspectorClient* client);
static std::unique_ptr<V8DebuggerScript> CreateWasm(
v8::Isolate* isolate, WasmTranslation* wasmTranslation,
v8::Local<v8::debug::WasmScript> underlyingScript, String16 id,

View File

@ -120,24 +120,26 @@ bool V8Debugger::isPausedInContextGroup(int contextGroupId) const {
bool V8Debugger::enabled() const { return m_enableCount > 0; }
std::vector<std::unique_ptr<V8DebuggerScript>> V8Debugger::getCompiledScripts(
int contextGroupId, V8DebuggerAgentImpl* agent) {
std::vector<std::unique_ptr<V8DebuggerScript>> result;
void V8Debugger::getCompiledScripts(
int contextGroupId,
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()) {
if (script->IsEmbedded()) {
result.push_back(V8DebuggerScript::Create(m_isolate, script, false,
m_inspector->client()));
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, agent,
result.push_back(V8DebuggerScript::Create(m_isolate, script, false,
m_inspector->client()));
}
return result;
}
void V8Debugger::setBreakpointsActive(bool active) {
@ -488,8 +490,7 @@ 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,
session->debuggerAgent(), client),
V8DebuggerScript::Create(isolate, script, is_live_edited, 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.
std::vector<std::unique_ptr<V8DebuggerScript>> getCompiledScripts(
int contextGroupId, V8DebuggerAgentImpl* agent);
void getCompiledScripts(int contextGroupId,
std::vector<std::unique_ptr<V8DebuggerScript>>&);
void enable();
void disable();

View File

@ -0,0 +1,17 @@
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

@ -0,0 +1,49 @@
// 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

@ -1,8 +0,0 @@
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

@ -1,41 +0,0 @@
// 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 maxCollectedScriptsSize = 10e6;
Protocol.Debugger.enable({maxCollectedScriptsSize});
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();
})();