From aab49f372f48354a32248de858a8347275517062 Mon Sep 17 00:00:00 2001 From: Alexey Kozyatinskiy Date: Wed, 9 May 2018 14:25:07 -0700 Subject: [PATCH] [inspector] do not allocate scope inside CallStackDepth Allocation is super slow and produce big performance regression on blink side. Bug: chromium:839567,chromium:839809 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: I3e9989435515ecfaedaee60c1f0c6939b9053e95 Reviewed-on: https://chromium-review.googlesource.com/1053105 Commit-Queue: Aleksey Kozyatinskiy Reviewed-by: Clemens Hammacher Cr-Commit-Position: refs/heads/master@{#53115} --- src/api.cc | 23 ++++++++++------------- src/execution.cc | 2 ++ src/isolate.h | 9 +++++---- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/api.cc b/src/api.cc index f956a44fd4..5f77cee68b 100644 --- a/src/api.cc +++ b/src/api.cc @@ -234,20 +234,17 @@ class CallDepthScope { : isolate_(isolate), context_(context), escaped_(false), - save_for_termination_(isolate->next_v8_call_is_safe_for_termination()) { + safe_for_termination_(isolate->next_v8_call_is_safe_for_termination()), + interrupts_scope_(isolate_, i::StackGuard::TERMINATE_EXECUTION, + isolate_->only_terminate_in_safe_scope() + ? (safe_for_termination_ + ? i::InterruptsScope::kRunInterrupts + : i::InterruptsScope::kPostponeInterrupts) + : i::InterruptsScope::kNoop) { // TODO(dcarney): remove this when blink stops crashing. DCHECK(!isolate_->external_caught_exception()); isolate_->handle_scope_implementer()->IncrementCallDepth(); isolate_->set_next_v8_call_is_safe_for_termination(false); - if (isolate_->only_terminate_in_safe_scope()) { - if (save_for_termination_) { - interrupts_scope_.reset(new i::SafeForInterruptsScope( - isolate_, i::StackGuard::TERMINATE_EXECUTION)); - } else { - interrupts_scope_.reset(new i::PostponeInterruptsScope( - isolate_, i::StackGuard::TERMINATE_EXECUTION)); - } - } if (!context.IsEmpty()) { i::Handle env = Utils::OpenHandle(*context); i::HandleScopeImplementer* impl = isolate->handle_scope_implementer(); @@ -272,7 +269,7 @@ class CallDepthScope { #ifdef V8_CHECK_MICROTASKS_SCOPES_CONSISTENCY if (do_callback) CheckMicrotasksScopesConsistency(isolate_); #endif - isolate_->set_next_v8_call_is_safe_for_termination(save_for_termination_); + isolate_->set_next_v8_call_is_safe_for_termination(safe_for_termination_); } void Escape() { @@ -289,8 +286,8 @@ class CallDepthScope { Local context_; bool escaped_; bool do_callback_; - bool save_for_termination_; - std::unique_ptr interrupts_scope_; + bool safe_for_termination_; + i::InterruptsScope interrupts_scope_; }; } // namespace diff --git a/src/execution.cc b/src/execution.cc index 7abc742600..e375067418 100644 --- a/src/execution.cc +++ b/src/execution.cc @@ -317,6 +317,7 @@ void StackGuard::DisableInterrupts() { void StackGuard::PushInterruptsScope(InterruptsScope* scope) { ExecutionAccess access(isolate_); + DCHECK_NE(scope->mode_, InterruptsScope::kNoop); if (scope->mode_ == InterruptsScope::kPostponeInterrupts) { // Intercept already requested interrupts. int intercepted = thread_local_.interrupt_flags_ & scope->intercept_mask_; @@ -342,6 +343,7 @@ void StackGuard::PushInterruptsScope(InterruptsScope* scope) { void StackGuard::PopInterruptsScope() { ExecutionAccess access(isolate_); InterruptsScope* top = thread_local_.interrupt_scopes_; + DCHECK_NE(top->mode_, InterruptsScope::kNoop); if (top->mode_ == InterruptsScope::kPostponeInterrupts) { // Make intercepted interrupts active. DCHECK_EQ(thread_local_.interrupt_flags_ & top->intercept_mask_, 0); diff --git a/src/isolate.h b/src/isolate.h index fd05526b4e..7c0153f74a 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -1819,9 +1819,11 @@ class StackLimitCheck BASE_EMBEDDED { // not affect other interrupts. class InterruptsScope { public: - enum Mode { kPostponeInterrupts, kRunInterrupts }; + enum Mode { kPostponeInterrupts, kRunInterrupts, kNoop }; - virtual ~InterruptsScope() { stack_guard_->PopInterruptsScope(); } + virtual ~InterruptsScope() { + if (mode_ != kNoop) stack_guard_->PopInterruptsScope(); + } // Find the scope that intercepts this interrupt. // It may be outermost PostponeInterruptsScope or innermost @@ -1829,13 +1831,12 @@ class InterruptsScope { // Return whether the interrupt has been intercepted. bool Intercept(StackGuard::InterruptFlag flag); - protected: InterruptsScope(Isolate* isolate, int intercept_mask, Mode mode) : stack_guard_(isolate->stack_guard()), intercept_mask_(intercept_mask), intercepted_flags_(0), mode_(mode) { - stack_guard_->PushInterruptsScope(this); + if (mode_ != kNoop) stack_guard_->PushInterruptsScope(this); } private: