diff --git a/runtime/vm/clustered_snapshot.cc b/runtime/vm/clustered_snapshot.cc index 6ecbffd984c..d8a4e7773d8 100644 --- a/runtime/vm/clustered_snapshot.cc +++ b/runtime/vm/clustered_snapshot.cc @@ -5619,8 +5619,6 @@ FullSnapshotWriter::FullSnapshotWriter(Snapshot::Kind kind, isolate()->ValidateClassTable(); isolate()->ValidateConstants(); #endif // DEBUG - // Can't have any mutation happening while we're serializing. - ASSERT(isolate()->background_compiler() == NULL); // TODO(rmacnak): The special case for AOT causes us to always generate the // same VM isolate snapshot for every app. AOT snapshots should be cleaned up diff --git a/runtime/vm/compiler/jit/compiler.cc b/runtime/vm/compiler/jit/compiler.cc index 96478dfa202..30356770a7e 100644 --- a/runtime/vm/compiler/jit/compiler.cc +++ b/runtime/vm/compiler/jit/compiler.cc @@ -1604,7 +1604,8 @@ RawObject* Compiler::CompileOptimizedFunction(Thread* thread, // this is either an OSR compilation or background compilation is // not currently allowed. ASSERT(!thread->IsMutatorThread() || (osr_id != kNoOSRDeoptId) || - !FLAG_background_compilation || BackgroundCompiler::IsDisabled()); + !FLAG_background_compilation || + BackgroundCompiler::IsDisabled(Isolate::Current())); CompilationPipeline* pipeline = CompilationPipeline::New(thread->zone(), function); return CompileFunctionHelper(pipeline, function, true, /* optimized */ @@ -2016,22 +2017,18 @@ class BackgroundCompilationQueue { BackgroundCompiler::BackgroundCompiler(Isolate* isolate) : isolate_(isolate), - running_(true), - done_(new bool()), queue_monitor_(new Monitor()), + function_queue_(new BackgroundCompilationQueue()), done_monitor_(new Monitor()), - function_queue_(new BackgroundCompilationQueue()) { - *done_ = false; -} + running_(false), + done_(true), + disabled_depth_(0) {} // Fields all deleted in ::Stop; here clear them. BackgroundCompiler::~BackgroundCompiler() { - isolate_ = NULL; - running_ = false; - done_ = NULL; - queue_monitor_ = NULL; - done_monitor_ = NULL; - function_queue_ = NULL; + delete queue_monitor_; + delete function_queue_; + delete done_monitor_; } void BackgroundCompiler::Run() { @@ -2099,7 +2096,7 @@ void BackgroundCompiler::Run() { { // Notify that the thread is done. MonitorLocker ml_done(done_monitor_); - *done_ = true; + done_ = true; ml_done.Notify(); } } @@ -2127,74 +2124,25 @@ void BackgroundCompiler::VisitPointers(ObjectPointerVisitor* visitor) { function_queue_->VisitObjectPointers(visitor); } -void BackgroundCompiler::Stop(Isolate* isolate) { - BackgroundCompiler* task = isolate->background_compiler(); - if (task == NULL) { - // Nothing to stop. - return; - } - BackgroundCompilationQueue* function_queue = task->function_queue(); +class BackgroundCompilerTask : public ThreadPool::Task { + public: + explicit BackgroundCompilerTask(BackgroundCompiler* background_compiler) + : background_compiler_(background_compiler) {} + virtual ~BackgroundCompilerTask() {} - Monitor* queue_monitor = task->queue_monitor_; - Monitor* done_monitor = task->done_monitor_; - bool* task_done = task->done_; - // Wake up compiler task and stop it. - { - MonitorLocker ml(queue_monitor); - task->running_ = false; - function_queue->Clear(); - // 'task' will be deleted by thread pool. - task = NULL; - ml.Notify(); // Stop waiting for the queue. - } + private: + virtual void Run() { background_compiler_->Run(); } - { - MonitorLocker ml_done(done_monitor); - while (!(*task_done)) { - ml_done.WaitWithSafepointCheck(Thread::Current()); - } - } - delete task_done; - delete done_monitor; - delete queue_monitor; - delete function_queue; - isolate->set_background_compiler(NULL); -} + BackgroundCompiler* background_compiler_; -void BackgroundCompiler::Disable() { + DISALLOW_COPY_AND_ASSIGN(BackgroundCompilerTask); +}; + +void BackgroundCompiler::Start() { Thread* thread = Thread::Current(); - ASSERT(thread != NULL); - Isolate* isolate = thread->isolate(); - MutexLocker ml(isolate->mutex()); - BackgroundCompiler* task = isolate->background_compiler(); - if (task != NULL) { - // We should only ever have to stop the task if this is the first call to - // Disable. - ASSERT(!isolate->is_background_compiler_disabled()); - BackgroundCompiler::Stop(isolate); - } - ASSERT(isolate->background_compiler() == NULL); - isolate->disable_background_compiler(); -} - -bool BackgroundCompiler::IsDisabled() { - Thread* thread = Thread::Current(); - ASSERT(thread != NULL); - Isolate* isolate = thread->isolate(); - MutexLocker ml(isolate->mutex()); - return isolate->is_background_compiler_disabled(); -} - -void BackgroundCompiler::Enable() { - Thread* thread = Thread::Current(); - ASSERT(thread != NULL); - Isolate* isolate = thread->isolate(); - MutexLocker ml(isolate->mutex()); - isolate->enable_background_compiler(); -} - -void BackgroundCompiler::EnsureInit(Thread* thread) { ASSERT(thread->IsMutatorThread()); + ASSERT(!thread->IsAtSafepoint()); + // Finalize NoSuchMethodError, _Mint; occasionally needed in optimized // compilation. Class& cls = Class::Handle( @@ -2207,21 +2155,54 @@ void BackgroundCompiler::EnsureInit(Thread* thread) { error = cls.EnsureIsFinalized(thread); ASSERT(error.IsNull()); - bool start_task = false; - Isolate* isolate = thread->isolate(); + MonitorLocker ml(done_monitor_); + if (running_ || !done_) return; + running_ = true; + done_ = false; + bool task_started = + Dart::thread_pool()->Run(new BackgroundCompilerTask(this)); + if (!task_started) { + running_ = false; + done_ = true; + } +} + +void BackgroundCompiler::Stop() { + Thread* thread = Thread::Current(); + ASSERT(thread->IsMutatorThread()); + ASSERT(!thread->IsAtSafepoint()); + { - MutexLocker ml(isolate->mutex()); - if (isolate->background_compiler() == NULL) { - BackgroundCompiler* task = new BackgroundCompiler(isolate); - isolate->set_background_compiler(task); - start_task = true; + MonitorLocker ml(queue_monitor_); + running_ = false; + function_queue_->Clear(); + ml.Notify(); // Stop waiting for the queue. + } + + { + MonitorLocker ml_done(done_monitor_); + while (!done_) { + ml_done.WaitWithSafepointCheck(thread); } } - if (start_task) { - Dart::thread_pool()->Run(isolate->background_compiler()); +} + +void BackgroundCompiler::Enable() { + disabled_depth_--; + if (disabled_depth_ < 0) { + FATAL("Mismatched number of calls to BackgroundCompiler::Enable/Disable."); } } +void BackgroundCompiler::Disable() { + Stop(); + disabled_depth_++; +} + +bool BackgroundCompiler::IsDisabled() { + return disabled_depth_ > 0; +} + #else // DART_PRECOMPILED_RUNTIME bool UseKernelFrontEndFor(ParsedFunction* parsed_function) { @@ -2329,15 +2310,11 @@ void BackgroundCompiler::VisitPointers(ObjectPointerVisitor* visitor) { UNREACHABLE(); } -void BackgroundCompiler::Stop(Isolate* isolate) { +void BackgroundCompiler::Start() { UNREACHABLE(); } -void BackgroundCompiler::EnsureInit(Thread* thread) { - UNREACHABLE(); -} - -void BackgroundCompiler::Disable() { +void BackgroundCompiler::Stop() { UNREACHABLE(); } @@ -2345,6 +2322,10 @@ void BackgroundCompiler::Enable() { UNREACHABLE(); } +void BackgroundCompiler::Disable() { + UNREACHABLE(); +} + bool BackgroundCompiler::IsDisabled() { UNREACHABLE(); return true; diff --git a/runtime/vm/compiler/jit/compiler.h b/runtime/vm/compiler/jit/compiler.h index f3de477a616..38946f130b9 100644 --- a/runtime/vm/compiler/jit/compiler.h +++ b/runtime/vm/compiler/jit/compiler.h @@ -161,21 +161,49 @@ class Compiler : public AllStatic { // Current implementation: one task per isolate, it dies with the owning // isolate. // No OSR compilation in the background compiler. -class BackgroundCompiler : public ThreadPool::Task { +class BackgroundCompiler { public: + explicit BackgroundCompiler(Isolate* isolate); virtual ~BackgroundCompiler(); - static void EnsureInit(Thread* thread); - - // Stops background compiler of the given isolate. - // TODO(turnidge): Give Stop and Disable more distinct names. - static void Stop(Isolate* isolate); - - static void Disable(); - - static void Enable(); - - static bool IsDisabled(); + static void Start(Isolate* isolate) { + ASSERT(Thread::Current()->IsMutatorThread()); + if (isolate->background_compiler() != NULL) { + isolate->background_compiler()->Start(); + } + } + static void Stop(Isolate* isolate) { + ASSERT(Thread::Current()->IsMutatorThread()); + if (isolate->background_compiler() != NULL) { + isolate->background_compiler()->Stop(); + } + } + static void Enable(Isolate* isolate) { + ASSERT(Thread::Current()->IsMutatorThread()); + if (isolate->background_compiler() != NULL) { + isolate->background_compiler()->Enable(); + } + } + static void Disable(Isolate* isolate) { + ASSERT(Thread::Current()->IsMutatorThread()); + if (isolate->background_compiler() != NULL) { + isolate->background_compiler()->Disable(); + } + } + static bool IsDisabled(Isolate* isolate) { + ASSERT(Thread::Current()->IsMutatorThread()); + if (isolate->background_compiler() != NULL) { + return isolate->background_compiler()->IsDisabled(); + } + return false; + } + static bool IsRunning(Isolate* isolate) { + ASSERT(Thread::Current()->IsMutatorThread()); + if (isolate->background_compiler() != NULL) { + return isolate->background_compiler()->IsRunning(); + } + return false; + } // Call to optimize a function in the background, enters the function in the // compilation queue. @@ -186,19 +214,27 @@ class BackgroundCompiler : public ThreadPool::Task { BackgroundCompilationQueue* function_queue() const { return function_queue_; } bool is_running() const { return running_; } - private: - explicit BackgroundCompiler(Isolate* isolate); + void Run(); - virtual void Run(); + private: + void Start(); + void Stop(); + void Enable(); + void Disable(); + bool IsDisabled(); + bool IsRunning() { return !done_; } Isolate* isolate_; - bool running_; // While true, will try to read queue and compile. - bool* done_; // True if the thread is done. - Monitor* queue_monitor_; // Controls access to the queue. - Monitor* done_monitor_; // Notify/wait that the thread is done. + Monitor* queue_monitor_; // Controls access to the queue. BackgroundCompilationQueue* function_queue_; + 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. + + int16_t disabled_depth_; + DISALLOW_IMPLICIT_CONSTRUCTORS(BackgroundCompiler); }; diff --git a/runtime/vm/compiler_test.cc b/runtime/vm/compiler_test.cc index 6e0e6641953..091dd5ddc38 100644 --- a/runtime/vm/compiler_test.cc +++ b/runtime/vm/compiler_test.cc @@ -95,9 +95,8 @@ ISOLATE_UNIT_TEST_CASE(CompileFunctionOnHelperThread) { // Constant in product mode. FLAG_background_compilation = true; #endif - BackgroundCompiler::EnsureInit(thread); Isolate* isolate = thread->isolate(); - ASSERT(isolate->background_compiler() != NULL); + BackgroundCompiler::Start(isolate); isolate->background_compiler()->CompileOptimized(func); Monitor* m = new Monitor(); { diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index e7ba61d42c2..1f5959f2644 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -1490,7 +1490,7 @@ Dart_CreateSnapshot(uint8_t** vm_snapshot_data_buffer, if (::Dart_IsError(state)) { return state; } - I->StopBackgroundCompiler(); + BackgroundCompiler::Stop(I); #if defined(DEBUG) I->heap()->CollectAllGarbage(); @@ -6671,7 +6671,7 @@ DART_EXPORT Dart_Handle Dart_CreateCoreJITSnapshotAsBlobs( if (::Dart_IsError(state)) { return state; } - I->StopBackgroundCompiler(); + BackgroundCompiler::Stop(I); ProgramVisitor::Dedup(); Symbols::Compact(I); @@ -6726,7 +6726,7 @@ Dart_CreateAppJITSnapshotAsBlobs(uint8_t** isolate_snapshot_data_buffer, if (::Dart_IsError(state)) { return state; } - I->StopBackgroundCompiler(); + BackgroundCompiler::Stop(I); ProgramVisitor::Dedup(); Symbols::Compact(I); diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index 607add035a4..d7abe08473b 100644 --- a/runtime/vm/isolate.cc +++ b/runtime/vm/isolate.cc @@ -234,7 +234,7 @@ void Isolate::ValidateConstants() { } // Verify that all canonical instances are correctly setup in the // corresponding canonical tables. - StopBackgroundCompiler(); + BackgroundCompiler::Stop(this); heap()->CollectAllGarbage(); Thread* thread = Thread::Current(); HeapIterationScope iteration(thread); @@ -880,7 +880,6 @@ Isolate::Isolate(const Dart_IsolateFlags& api_flags) class_table_(), single_step_(false), isolate_flags_(0), - background_compiler_disabled_depth_(0), background_compiler_(NULL), #if !defined(PRODUCT) debugger_(NULL), @@ -928,7 +927,7 @@ Isolate::Isolate(const Dart_IsolateFlags& api_flags) message_handler_(NULL), spawn_state_(NULL), defer_finalization_count_(0), - pending_deopts_(new MallocGrowableArray), + pending_deopts_(new MallocGrowableArray()), deopt_context_(NULL), tag_table_(GrowableObjectArray::null()), deoptimized_code_array_(GrowableObjectArray::null()), @@ -959,14 +958,20 @@ Isolate::Isolate(const Dart_IsolateFlags& api_flags) "which violates the Dart standard.\n" " See dartbug.com/30524 for more information.\n"); } + + NOT_IN_PRECOMPILED(background_compiler_ = new BackgroundCompiler(this)); } #undef REUSABLE_HANDLE_SCOPE_INIT #undef REUSABLE_HANDLE_INITIALIZERS Isolate::~Isolate() { + delete background_compiler_; + background_compiler_ = NULL; + #if !defined(PRODUCT) delete debugger_; + debugger_ = NULL; if (FLAG_support_service) { delete object_id_ring_; } @@ -1766,13 +1771,6 @@ void Isolate::LowLevelShutdown() { #endif // !defined(PRODUCT) } -void Isolate::StopBackgroundCompiler() { - // Wait until all background compilation has finished. - if (background_compiler_ != NULL) { - BackgroundCompiler::Stop(this); - } -} - #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME) void Isolate::MaybeIncreaseReloadEveryNStackOverflowChecks() { if (FLAG_reload_every_back_off) { @@ -1791,7 +1789,9 @@ void Isolate::MaybeIncreaseReloadEveryNStackOverflowChecks() { void Isolate::Shutdown() { ASSERT(this == Isolate::Current()); - StopBackgroundCompiler(); + BackgroundCompiler::Stop(this); + delete background_compiler_; + background_compiler_ = NULL; #if defined(DEBUG) if (heap_ != NULL && FLAG_verify_on_transition) { diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h index f3c16cf6eae..f91655f3731 100644 --- a/runtime/vm/isolate.h +++ b/runtime/vm/isolate.h @@ -450,26 +450,6 @@ class Isolate : public BaseIsolate { BackgroundCompiler* background_compiler() const { return background_compiler_; } - void set_background_compiler(BackgroundCompiler* value) { - // Do not overwrite a background compiler (memory leak). - ASSERT((value == NULL) || (background_compiler_ == NULL)); - background_compiler_ = value; - } - - void enable_background_compiler() { - background_compiler_disabled_depth_--; - if (background_compiler_disabled_depth_ < 0) { - FATAL( - "Mismatched number of calls to disable_background_compiler and " - "enable_background_compiler."); - } - } - - void disable_background_compiler() { background_compiler_disabled_depth_++; } - - bool is_background_compiler_disabled() const { - return background_compiler_disabled_depth_ > 0; - } #if !defined(PRODUCT) void UpdateLastAllocationProfileAccumulatorResetTimestamp() { @@ -754,8 +734,6 @@ class Isolate : public BaseIsolate { static bool IsolateCreationEnabled(); static bool IsVMInternalIsolate(Isolate* isolate); - void StopBackgroundCompiler(); - #if !defined(PRODUCT) intptr_t reload_every_n_stack_overflow_checks() const { return reload_every_n_stack_overflow_checks_; @@ -877,7 +855,6 @@ class Isolate : public BaseIsolate { uint32_t isolate_flags_; // Background compilation. - int16_t background_compiler_disabled_depth_; BackgroundCompiler* background_compiler_; // Fields that aren't needed in a product build go here with boolean flags at diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc index 2c080f5083f..5ebbd9e6970 100644 --- a/runtime/vm/isolate_reload.cc +++ b/runtime/vm/isolate_reload.cc @@ -624,7 +624,7 @@ void IsolateReloadContext::Reload(bool force_reload, become_enum_mappings_ = GrowableObjectArray::New(Heap::kOld); // Disable the background compiler while we are performing the reload. - BackgroundCompiler::Disable(); + BackgroundCompiler::Disable(I); // Ensure all functions on the stack have unoptimized code. EnsuredUnoptimizedCodeForStack(); @@ -682,7 +682,7 @@ void IsolateReloadContext::Reload(bool force_reload, // WEIRD CONTROL FLOW ENDS. // Re-enable the background compiler. Do this before propagating any errors. - BackgroundCompiler::Enable(); + BackgroundCompiler::Enable(I); if (result.IsUnwindError()) { if (thread->top_exit_frame_info() == 0) { diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index 05c9f166e74..cb2a16a7527 100644 --- a/runtime/vm/runtime_entry.cc +++ b/runtime/vm/runtime_entry.cc @@ -1811,7 +1811,7 @@ DEFINE_RUNTIME_ENTRY(OptimizeInvokedFunction, 1) { if (FLAG_enable_inlining_annotations) { FATAL("Cannot enable inlining annotations and background compilation"); } - if (!BackgroundCompiler::IsDisabled()) { + if (!BackgroundCompiler::IsDisabled(isolate)) { if (FLAG_background_compilation_stop_alot) { BackgroundCompiler::Stop(isolate); } @@ -1820,8 +1820,7 @@ DEFINE_RUNTIME_ENTRY(OptimizeInvokedFunction, 1) { // takes long time to trigger optimization. // Note that the background compilation queue rejects duplicate entries. function.SetUsageCounter(INT_MIN); - BackgroundCompiler::EnsureInit(thread); - ASSERT(isolate->background_compiler() != NULL); + BackgroundCompiler::Start(isolate); isolate->background_compiler()->CompileOptimized(function); // Continue in the same code. arguments.SetReturn(function); diff --git a/runtime/vm/symbols.cc b/runtime/vm/symbols.cc index b668a3bb3cb..f14c39b05a1 100644 --- a/runtime/vm/symbols.cc +++ b/runtime/vm/symbols.cc @@ -308,7 +308,6 @@ RawArray* Symbols::UnifiedSymbolTable() { Zone* zone = thread->zone(); ASSERT(thread->IsMutatorThread()); - ASSERT(isolate->background_compiler() == NULL); SymbolTable vm_table(zone, Dart::vm_isolate()->object_store()->symbol_table());