diff --git a/runtime/vm/class_finalizer.cc b/runtime/vm/class_finalizer.cc index ec47b195614..0dfec5c5678 100644 --- a/runtime/vm/class_finalizer.cc +++ b/runtime/vm/class_finalizer.cc @@ -1314,7 +1314,7 @@ void ClassFinalizer::SortClasses() { // Prevent background compiler from adding deferred classes or canonicalizing // new types while classes are being sorted and type hashes are modified. - BackgroundCompiler::Stop(I); + NoBackgroundCompilerScope no_bg_compiler(T); SafepointWriteRwLocker ml(T, T->isolate_group()->program_lock()); ClassTable* table = IG->class_table(); diff --git a/runtime/vm/compiler/jit/compiler.cc b/runtime/vm/compiler/jit/compiler.cc index e83ce88bbd4..eeb7cd794a2 100644 --- a/runtime/vm/compiler/jit/compiler.cc +++ b/runtime/vm/compiler/jit/compiler.cc @@ -326,7 +326,6 @@ class CompileParsedFunctionHelper : public ValueObject { CodePtr FinalizeCompilation(compiler::Assembler* assembler, FlowGraphCompiler* graph_compiler, FlowGraph* flow_graph); - void CheckIfBackgroundCompilerIsBeingStopped(bool optimizing_compiler); ParsedFunction* parsed_function_; const bool optimized_; @@ -472,18 +471,6 @@ CodePtr CompileParsedFunctionHelper::FinalizeCompilation( return code.ptr(); } -void CompileParsedFunctionHelper::CheckIfBackgroundCompilerIsBeingStopped( - bool optimizing_compiler) { - ASSERT(Compiler::IsBackgroundCompilation()); - if (optimizing_compiler) { - if (!isolate()->optimizing_background_compiler()->is_running()) { - // The background compiler is being stopped. - Compiler::AbortBackgroundCompilation( - DeoptId::kNone, "Optimizing Background compilation is being stopped"); - } - } -} - // Return null if bailed out. CodePtr CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) { ASSERT(!FLAG_precompiled_mode); @@ -614,10 +601,6 @@ CodePtr CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) { FinalizeCompilation(&assembler, &graph_compiler, flow_graph); }; - if (Compiler::IsBackgroundCompilation()) { - CheckIfBackgroundCompilerIsBeingStopped(optimized()); - } - // Grab write program_lock outside of potential safepoint, that lock // can't be waited for inside the safepoint. // Initially read lock was added to guard direct_subclasses field @@ -1113,14 +1096,27 @@ class BackgroundCompilationQueue { DISALLOW_COPY_AND_ASSIGN(BackgroundCompilationQueue); }; -BackgroundCompiler::BackgroundCompiler(Isolate* isolate, bool optimizing) +class BackgroundCompilerTask : public ThreadPool::Task { + public: + explicit BackgroundCompilerTask(BackgroundCompiler* background_compiler) + : background_compiler_(background_compiler) {} + virtual ~BackgroundCompilerTask() {} + + private: + virtual void Run() { background_compiler_->Run(); } + + BackgroundCompiler* background_compiler_; + + DISALLOW_COPY_AND_ASSIGN(BackgroundCompilerTask); +}; + +BackgroundCompiler::BackgroundCompiler(Isolate* isolate) : isolate_(isolate), queue_monitor_(), function_queue_(new BackgroundCompilationQueue()), done_monitor_(), running_(false), done_(true), - optimizing_(optimizing), disabled_depth_(0) {} // Fields all deleted in ::Stop; here clear them. @@ -1141,19 +1137,18 @@ void BackgroundCompiler::Run() { HANDLESCOPE(thread); Function& function = Function::Handle(zone); { - MonitorLocker ml(&queue_monitor_); + SafepointMonitorLocker ml(&queue_monitor_); if (running_) { function = function_queue()->PeekFunction(); } } while (!function.IsNull()) { - ASSERT(is_optimizing()); Compiler::CompileOptimizedFunction(thread, function, Compiler::kNoOSRDeoptId); QueueElement* qelem = NULL; { - MonitorLocker ml(&queue_monitor_); + SafepointMonitorLocker ml(&queue_monitor_); if (!running_ || function_queue()->IsEmpty()) { // We are shutting down, queue was cleared. function = Function::null(); @@ -1162,8 +1157,7 @@ void BackgroundCompiler::Run() { const Function& old = Function::Handle(qelem->Function()); // If an optimizable method is not optimized, put it back on // the background queue (unless it was passed to foreground). - if ((is_optimizing() && !old.HasOptimizedCode() && - old.IsOptimizable()) || + if ((!old.HasOptimizedCode() && old.IsOptimizable()) || FLAG_stress_test_background_compilation) { if (old.is_background_optimizable() && Compiler::CanOptimizeFunction(thread, old)) { @@ -1197,54 +1191,40 @@ void BackgroundCompiler::Run() { } } -void BackgroundCompiler::Compile(const Function& function) { - ASSERT(Thread::Current()->IsMutatorThread()); - MonitorLocker ml(&queue_monitor_); - ASSERT(running_); - if (function_queue()->ContainsObj(function)) { - return; - } - QueueElement* elem = new QueueElement(function); - function_queue()->Add(elem); - ml.Notify(); -} - -void BackgroundCompiler::VisitPointers(ObjectPointerVisitor* visitor) { - function_queue_->VisitObjectPointers(visitor); -} - -class BackgroundCompilerTask : public ThreadPool::Task { - public: - explicit BackgroundCompilerTask(BackgroundCompiler* background_compiler) - : background_compiler_(background_compiler) {} - virtual ~BackgroundCompilerTask() {} - - private: - virtual void Run() { background_compiler_->Run(); } - - BackgroundCompiler* background_compiler_; - - DISALLOW_COPY_AND_ASSIGN(BackgroundCompilerTask); -}; - -void BackgroundCompiler::Start() { +bool BackgroundCompiler::EnqueueCompilation(const Function& function) { Thread* thread = Thread::Current(); ASSERT(thread->IsMutatorThread()); ASSERT(!thread->IsAtSafepoint()); - MonitorLocker ml(&done_monitor_); - if (running_ || !done_) return; - running_ = true; - done_ = false; - // If we ever wanted to run the BG compiler on the - // `IsolateGroup::mutator_pool()` we would need to ensure the BG compiler - // stops when it's idle - otherwise the [MutatorThreadPool]-based idle - // notification would not work anymore. - bool task_started = Dart::thread_pool()->Run(this); - if (!task_started) { - running_ = false; - done_ = true; + SafepointMonitorLocker ml_done(&done_monitor_); + if (disabled_depth_ > 0) return false; + if (!running_ && done_) { + running_ = true; + done_ = false; + // If we ever wanted to run the BG compiler on the + // `IsolateGroup::mutator_pool()` we would need to ensure the BG compiler + // stops when it's idle - otherwise the [MutatorThreadPool]-based idle + // notification would not work anymore. + if (!Dart::thread_pool()->Run(this)) { + running_ = false; + done_ = true; + return false; + } } + + SafepointMonitorLocker ml(&queue_monitor_); + ASSERT(running_); + if (function_queue()->ContainsObj(function)) { + return true; + } + QueueElement* elem = new QueueElement(function); + function_queue()->Add(elem); + ml.NotifyAll(); + return true; +} + +void BackgroundCompiler::VisitPointers(ObjectPointerVisitor* visitor) { + function_queue_->VisitObjectPointers(visitor); } void BackgroundCompiler::Stop() { @@ -1252,22 +1232,30 @@ void BackgroundCompiler::Stop() { ASSERT(thread->IsMutatorThread()); ASSERT(!thread->IsAtSafepoint()); + SafepointMonitorLocker ml_done(&done_monitor_); + StopLocked(thread, &ml_done); +} + +void BackgroundCompiler::StopLocked(Thread* thread, + SafepointMonitorLocker* done_locker) { { - MonitorLocker ml(&queue_monitor_); + SafepointMonitorLocker ml(&queue_monitor_); running_ = false; function_queue_->Clear(); - ml.Notify(); // Stop waiting for the queue. + ml.NotifyAll(); // Stop waiting for the queue. } - { - MonitorLocker ml_done(&done_monitor_); - while (!done_) { - ml_done.WaitWithSafepointCheck(thread); - } + while (!done_) { + done_locker->Wait(); } } void BackgroundCompiler::Enable() { + Thread* thread = Thread::Current(); + ASSERT(thread->IsMutatorThread()); + ASSERT(!thread->IsAtSafepoint()); + + MonitorLocker ml_done(&done_monitor_); disabled_depth_--; if (disabled_depth_ < 0) { FATAL("Mismatched number of calls to BackgroundCompiler::Enable/Disable."); @@ -1275,12 +1263,14 @@ void BackgroundCompiler::Enable() { } void BackgroundCompiler::Disable() { - Stop(); - disabled_depth_++; -} + Thread* thread = Thread::Current(); + ASSERT(thread->IsMutatorThread()); + ASSERT(!thread->IsAtSafepoint()); -bool BackgroundCompiler::IsDisabled() { - return disabled_depth_ > 0; + SafepointMonitorLocker ml_done(&done_monitor_); + disabled_depth_++; + if (done_) return; + StopLocked(thread, &ml_done); } #else // DART_PRECOMPILED_RUNTIME @@ -1339,33 +1329,25 @@ void Compiler::AbortBackgroundCompilation(intptr_t deopt_id, const char* msg) { UNREACHABLE(); } -void BackgroundCompiler::Compile(const Function& function) { +bool BackgroundCompiler::EnqueueCompilation(const Function& function) { UNREACHABLE(); + return false; } void BackgroundCompiler::VisitPointers(ObjectPointerVisitor* visitor) { UNREACHABLE(); } -void BackgroundCompiler::Start() { - UNREACHABLE(); -} - void BackgroundCompiler::Stop() { UNREACHABLE(); } void BackgroundCompiler::Enable() { - UNREACHABLE(); + // NOP } void BackgroundCompiler::Disable() { - UNREACHABLE(); -} - -bool BackgroundCompiler::IsDisabled() { - UNREACHABLE(); - return true; + // NOP } #endif // DART_PRECOMPILED_RUNTIME diff --git a/runtime/vm/compiler/jit/compiler.h b/runtime/vm/compiler/jit/compiler.h index 2e2d563179e..5ed55beae6f 100644 --- a/runtime/vm/compiler/jit/compiler.h +++ b/runtime/vm/compiler/jit/compiler.h @@ -119,61 +119,33 @@ class Compiler : public AllStatic { // No OSR compilation in the background compiler. class BackgroundCompiler { public: - explicit BackgroundCompiler(Isolate* isolate, bool optimizing); + explicit BackgroundCompiler(Isolate* isolate); virtual ~BackgroundCompiler(); - static void Start(Isolate* isolate) { - ASSERT(Thread::Current()->IsMutatorThread()); - if (isolate->optimizing_background_compiler() != NULL) { - isolate->optimizing_background_compiler()->Start(); - } - } static void Stop(Isolate* isolate) { ASSERT(Thread::Current()->IsMutatorThread()); - if (isolate->optimizing_background_compiler() != NULL) { - isolate->optimizing_background_compiler()->Stop(); - } - } - static void Enable(Isolate* isolate) { - ASSERT(Thread::Current()->IsMutatorThread()); - if (isolate->optimizing_background_compiler() != NULL) { - isolate->optimizing_background_compiler()->Enable(); - } - } - static void Disable(Isolate* isolate) { - ASSERT(Thread::Current()->IsMutatorThread()); - if (isolate->optimizing_background_compiler() != NULL) { - isolate->optimizing_background_compiler()->Disable(); - } - } - static bool IsDisabled(Isolate* isolate, bool optimizing_compiler) { - ASSERT(Thread::Current()->IsMutatorThread()); - if (optimizing_compiler) { - if (isolate->optimizing_background_compiler() != NULL) { - return isolate->optimizing_background_compiler()->IsDisabled(); - } - } - return false; + isolate->background_compiler()->Stop(); } - // Call to compile (unoptimized or optimized) a function in the background, - // enters the function in the compilation queue. - void Compile(const Function& function); + // Enqueues a function to be compiled in the background. + // + // Return `true` if successful. + bool EnqueueCompilation(const Function& function); void VisitPointers(ObjectPointerVisitor* visitor); BackgroundCompilationQueue* function_queue() const { return function_queue_; } bool is_running() const { return running_; } - bool is_optimizing() const { return optimizing_; } void Run(); private: - void Start(); + friend class NoBackgroundCompilerScope; + void Stop(); + void StopLocked(Thread* thread, SafepointMonitorLocker* done_locker); void Enable(); void Disable(); - bool IsDisabled(); bool IsRunning() { return !done_; } Isolate* isolate_; @@ -184,13 +156,24 @@ class BackgroundCompiler { Monitor done_monitor_; // Notify/wait that the thread is done. bool running_; // While true, will try to read queue and compile. bool done_; // True if the thread is done. - bool optimizing_; int16_t disabled_depth_; DISALLOW_IMPLICIT_CONSTRUCTORS(BackgroundCompiler); }; +class NoBackgroundCompilerScope : public StackResource { + public: + explicit NoBackgroundCompilerScope(Thread* thread) + : StackResource(thread), isolate_(thread->isolate()) { + isolate_->background_compiler()->Disable(); + } + ~NoBackgroundCompilerScope() { isolate_->background_compiler()->Enable(); } + + private: + Isolate* isolate_; +}; + } // namespace dart #endif // RUNTIME_VM_COMPILER_JIT_COMPILER_H_ diff --git a/runtime/vm/compiler_test.cc b/runtime/vm/compiler_test.cc index 9dcd0041478..d5d9a5e7e86 100644 --- a/runtime/vm/compiler_test.cc +++ b/runtime/vm/compiler_test.cc @@ -89,17 +89,15 @@ ISOLATE_UNIT_TEST_CASE(OptimizeCompileFunctionOnHelperThread) { FLAG_background_compilation = true; #endif Isolate* isolate = thread->isolate(); - BackgroundCompiler::Start(isolate); - isolate->optimizing_background_compiler()->Compile(func); + isolate->background_compiler()->EnqueueCompilation(func); Monitor* m = new Monitor(); { - MonitorLocker ml(m); + SafepointMonitorLocker ml(m); while (!func.HasOptimizedCode()) { - ml.WaitWithSafepointCheck(thread, 1); + ml.Wait(1); } } delete m; - BackgroundCompiler::Stop(isolate); } ISOLATE_UNIT_TEST_CASE(CompileFunctionOnHelperThread) { diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index 50521d8d68b..96d9b1bc9ea 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -1820,7 +1820,6 @@ Dart_CreateSnapshot(uint8_t** vm_snapshot_data_buffer, #else DARTSCOPE(Thread::Current()); API_TIMELINE_DURATION(T); - auto I = T->isolate(); if (vm_snapshot_data_buffer != nullptr) { CHECK_NULL(vm_snapshot_data_size); } @@ -1831,7 +1830,7 @@ Dart_CreateSnapshot(uint8_t** vm_snapshot_data_buffer, if (Api::IsError(state)) { return state; } - BackgroundCompiler::Stop(I); + NoBackgroundCompilerScope no_bg_compiler(T); #if defined(DEBUG) T->isolate_group()->heap()->CollectAllGarbage(); @@ -6532,9 +6531,11 @@ DART_EXPORT Dart_Handle Dart_SortClasses() { return Api::NewError("%s: Cannot compile on an AOT runtime.", CURRENT_FUNC); #else DARTSCOPE(Thread::Current()); + // Prevent background compiler from running while code is being cleared and // adding new code. - BackgroundCompiler::Stop(Isolate::Current()); + NoBackgroundCompilerScope no_bg_compiler(T); + // We don't have mechanisms to change class-ids that are embedded in code and // ICData. ClassFinalizer::ClearAllCode(); @@ -6930,7 +6931,6 @@ DART_EXPORT Dart_Handle Dart_CreateCoreJITSnapshotAsBlobs( #else DARTSCOPE(Thread::Current()); API_TIMELINE_DURATION(T); - Isolate* I = T->isolate(); CHECK_NULL(vm_snapshot_data_buffer); CHECK_NULL(vm_snapshot_data_size); CHECK_NULL(vm_snapshot_instructions_buffer); @@ -6944,7 +6944,9 @@ DART_EXPORT Dart_Handle Dart_CreateCoreJITSnapshotAsBlobs( if (Api::IsError(state)) { return state; } - BackgroundCompiler::Stop(I); + + NoBackgroundCompilerScope no_bg_compiler(T); + DropRegExpMatchCode(Z); ProgramVisitor::Dedup(T); @@ -7031,7 +7033,7 @@ Dart_CreateAppJITSnapshotAsBlobs(uint8_t** isolate_snapshot_data_buffer, // Kill off any auxiliary isolates before starting with deduping. KillNonMainIsolatesSlow(T, I); - BackgroundCompiler::Stop(I); + NoBackgroundCompilerScope no_bg_compiler(T); DropRegExpMatchCode(Z); ProgramVisitor::Dedup(T); diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc index dc6463f16bf..61be4a2c554 100644 --- a/runtime/vm/debugger.cc +++ b/runtime/vm/debugger.cc @@ -1629,7 +1629,7 @@ void Debugger::DeoptimizeWorld() { #if defined(DART_PRECOMPILED_RUNTIME) UNREACHABLE(); #else - BackgroundCompiler::Stop(isolate_); + NoBackgroundCompilerScope no_bg_compiler(Thread::Current()); if (FLAG_trace_deoptimization) { THR_Print("Deopt for debugger\n"); } diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index 30c3fac00ad..da8abfc1cee 100644 --- a/runtime/vm/isolate.cc +++ b/runtime/vm/isolate.cc @@ -972,7 +972,7 @@ void Isolate::ValidateConstants() { // Verify that all canonical instances are correctly setup in the // corresponding canonical tables. - BackgroundCompiler::Stop(this); + NoBackgroundCompilerScope no_bg_compiler(Thread::Current()); group()->heap()->CollectAllGarbage(); Thread* thread = Thread::Current(); HeapIterationScope iteration(thread); @@ -1717,8 +1717,7 @@ Isolate::Isolate(IsolateGroup* isolate_group, " See dartbug.com/30524 for more information.\n"); } - NOT_IN_PRECOMPILED(optimizing_background_compiler_ = - new BackgroundCompiler(this, /* optimizing = */ true)); + NOT_IN_PRECOMPILED(background_compiler_ = new BackgroundCompiler(this)); } #undef REUSABLE_HANDLE_SCOPE_INIT @@ -1730,8 +1729,8 @@ Isolate::~Isolate() { // RELEASE_ASSERT(program_reload_context_ == NULL); #endif // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME) - delete optimizing_background_compiler_; - optimizing_background_compiler_ = nullptr; + delete background_compiler_; + background_compiler_ = nullptr; #if !defined(PRODUCT) delete debugger_; @@ -2433,9 +2432,9 @@ void Isolate::set_forward_table_old(WeakTable* table) { void Isolate::Shutdown() { ASSERT(this == Isolate::Current()); - BackgroundCompiler::Stop(this); - delete optimizing_background_compiler_; - optimizing_background_compiler_ = nullptr; + NOT_IN_PRECOMPILED(BackgroundCompiler::Stop(this)); + NOT_IN_PRECOMPILED(delete background_compiler_); + background_compiler_ = nullptr; Thread* thread = Thread::Current(); @@ -2598,9 +2597,6 @@ void Isolate::VisitObjectPointers(ObjectPointerVisitor* visitor, if (background_compiler() != nullptr) { background_compiler()->VisitPointers(visitor); } - if (optimizing_background_compiler() != nullptr) { - optimizing_background_compiler()->VisitPointers(visitor); - } #if !defined(PRODUCT) // Visit objects in the debugger. diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h index 3e87c33f55f..ebf7e59aa47 100644 --- a/runtime/vm/isolate.h +++ b/runtime/vm/isolate.h @@ -1230,10 +1230,6 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry { return background_compiler_; } - BackgroundCompiler* optimizing_background_compiler() const { - return optimizing_background_compiler_; - } - intptr_t BlockClassFinalization() { ASSERT(defer_finalization_count_ >= 0); return defer_finalization_count_++; @@ -1591,12 +1587,8 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry { uint32_t isolate_flags_ = 0; - // Unoptimized background compilation. BackgroundCompiler* background_compiler_ = nullptr; - // Optimized background compilation. - BackgroundCompiler* optimizing_background_compiler_ = nullptr; - // Fields that aren't needed in a product build go here with boolean flags at // the top. #if !defined(PRODUCT) diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc index 6e20c6e6487..f5b44986f47 100644 --- a/runtime/vm/isolate_reload.cc +++ b/runtime/vm/isolate_reload.cc @@ -664,8 +664,11 @@ bool IsolateGroupReloadContext::Reload(bool force_reload, [&](Isolate* isolate) { number_of_isolates++; }); // Disable the background compiler while we are performing the reload. - ForEachIsolate( - [&](Isolate* isolate) { BackgroundCompiler::Disable(isolate); }); + ForEachIsolate([&](Isolate* isolate) { + // TODO(dartbug.com/36097): Once the BG compiler moves from Isolate to + // IsolateGroup this scope should cover most of this function. + NoBackgroundCompilerScope stop_bg_compiler(isolate->mutator_thread()); + }); // Wait for any concurrent marking tasks to finish and turn off the // concurrent marker during reload as we might be allocating new instances @@ -874,10 +877,6 @@ bool IsolateGroupReloadContext::Reload(bool force_reload, } } - // Re-enable the background compiler. Do this before propagating any errors. - ForEachIsolate( - [&](Isolate* isolate) { BackgroundCompiler::Enable(isolate); }); - // Reenable concurrent marking if it was initially on. if (old_concurrent_mark_flag) { heap->old_space()->set_enable_concurrent_mark(true); diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index 32ed89febd4..2c78a12b77c 100644 --- a/runtime/vm/runtime_entry.cc +++ b/runtime/vm/runtime_entry.cc @@ -2774,17 +2774,14 @@ DEFINE_RUNTIME_ENTRY(OptimizeInvokedFunction, 1) { if (Compiler::CanOptimizeFunction(thread, function)) { if (FLAG_background_compilation) { - if (!BackgroundCompiler::IsDisabled(isolate, - /* optimizing_compiler = */ true) && - function.is_background_optimizable()) { - // Ensure background compiler is running, if not start it. - BackgroundCompiler::Start(isolate); + if (function.is_background_optimizable() && + isolate->background_compiler()->EnqueueCompilation(function)) { // Reduce the chance of triggering a compilation while the function is // being compiled in the background. INT32_MIN should ensure that it // takes long time to trigger a compilation. // Note that the background compilation queue rejects duplicate entries. function.SetUsageCounter(INT32_MIN); - isolate->optimizing_background_compiler()->Compile(function); + // Continue in the same code. arguments.SetReturn(function); return;