Commit 26d4699a authored by Josh Karlin's avatar Josh Karlin Committed by Commit Bot

Revert "Optimize SourceLocation::Capture"

This reverts commit b818b458.

Reason for revert: My measurement was wrong. The call to v8::Isolate isn't nearly as slow as previously though. Reverting the optimization as it just adds unnecessary complexity.

Original change's description:
> Optimize SourceLocation::Capture
> 
> Determining the v8::Isolate can take ~25% of the time
> required to capture the top of the stack. This CL gets the Isolate
> from the ExecutionContext when it's available to speed things up.
> 
> Bug: 851531
> Change-Id: Ia823fbb0643c62f1a4da3cc289b23ca6fb43da2b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2122568
> Commit-Queue: Josh Karlin <jkarlin@chromium.org>
> Reviewed-by: Nate Chapin <japhet@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#754054}

TBR=japhet@chromium.org,jkarlin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 851531
Change-Id: I36e9be81b06598ed29484df2997c22cbd4dc170f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2128090Reviewed-by: default avatarJosh Karlin <jkarlin@chromium.org>
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754678}
parent 1830f4ba
...@@ -23,17 +23,15 @@ namespace blink { ...@@ -23,17 +23,15 @@ namespace blink {
namespace { namespace {
std::unique_ptr<v8_inspector::V8StackTrace> CaptureStackTrace( std::unique_ptr<v8_inspector::V8StackTrace> CaptureStackTrace(bool full) {
v8::Isolate* isolate, v8::Isolate* isolate = v8::Isolate::GetCurrent();
bool full) {
ThreadDebugger* debugger = ThreadDebugger::From(isolate); ThreadDebugger* debugger = ThreadDebugger::From(isolate);
if (!debugger || !isolate->InContext()) if (!debugger || !isolate->InContext())
return nullptr; return nullptr;
ScriptForbiddenScope::AllowUserAgentScript allow_scripting; ScriptForbiddenScope::AllowUserAgentScript allow_scripting;
return debugger->GetV8Inspector()->captureStackTrace(full); return debugger->GetV8Inspector()->captureStackTrace(full);
} }
}
} // namespace
// static // static
std::unique_ptr<SourceLocation> SourceLocation::Capture( std::unique_ptr<SourceLocation> SourceLocation::Capture(
...@@ -41,7 +39,7 @@ std::unique_ptr<SourceLocation> SourceLocation::Capture( ...@@ -41,7 +39,7 @@ std::unique_ptr<SourceLocation> SourceLocation::Capture(
unsigned line_number, unsigned line_number,
unsigned column_number) { unsigned column_number) {
std::unique_ptr<v8_inspector::V8StackTrace> stack_trace = std::unique_ptr<v8_inspector::V8StackTrace> stack_trace =
CaptureStackTrace(v8::Isolate::GetCurrent(), false); CaptureStackTrace(false);
if (stack_trace && !stack_trace->isEmpty()) if (stack_trace && !stack_trace->isEmpty())
return SourceLocation::CreateFromNonEmptyV8StackTrace( return SourceLocation::CreateFromNonEmptyV8StackTrace(
std::move(stack_trace), 0); std::move(stack_trace), 0);
...@@ -52,14 +50,8 @@ std::unique_ptr<SourceLocation> SourceLocation::Capture( ...@@ -52,14 +50,8 @@ std::unique_ptr<SourceLocation> SourceLocation::Capture(
// static // static
std::unique_ptr<SourceLocation> SourceLocation::Capture( std::unique_ptr<SourceLocation> SourceLocation::Capture(
ExecutionContext* execution_context) { ExecutionContext* execution_context) {
// v8::Isolate::GetCurrent is slow so get it from the ExecutionContext when
// available.
// TODO(jkarlin): Force all callers of all Capture() methods to call with a
// valid ExecutionContext.
v8::Isolate* isolate = execution_context ? execution_context->GetIsolate()
: v8::Isolate::GetCurrent();
std::unique_ptr<v8_inspector::V8StackTrace> stack_trace = std::unique_ptr<v8_inspector::V8StackTrace> stack_trace =
CaptureStackTrace(isolate, false); CaptureStackTrace(false);
if (stack_trace && !stack_trace->isEmpty()) if (stack_trace && !stack_trace->isEmpty())
return SourceLocation::CreateFromNonEmptyV8StackTrace( return SourceLocation::CreateFromNonEmptyV8StackTrace(
std::move(stack_trace), 0); std::move(stack_trace), 0);
...@@ -145,7 +137,7 @@ std::unique_ptr<SourceLocation> SourceLocation::FromFunction( ...@@ -145,7 +137,7 @@ std::unique_ptr<SourceLocation> SourceLocation::FromFunction(
// static // static
std::unique_ptr<SourceLocation> SourceLocation::CaptureWithFullStackTrace() { std::unique_ptr<SourceLocation> SourceLocation::CaptureWithFullStackTrace() {
std::unique_ptr<v8_inspector::V8StackTrace> stack_trace = std::unique_ptr<v8_inspector::V8StackTrace> stack_trace =
CaptureStackTrace(v8::Isolate::GetCurrent(), true); CaptureStackTrace(true);
if (stack_trace && !stack_trace->isEmpty()) if (stack_trace && !stack_trace->isEmpty())
return SourceLocation::CreateFromNonEmptyV8StackTrace( return SourceLocation::CreateFromNonEmptyV8StackTrace(
std::move(stack_trace), 0); std::move(stack_trace), 0);
......
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