dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[vm/concurrency] Simplify logic around background compiler, allow concurrent disabling

Browse Source 
The logic for background compiler can be simplified due to:

  * we only have one bg compiler
  * it always optimizes
  * it is allocated at isolate startup and freed on isolate shutdown
  * an existing check in the compiler for whether the BG compiler is
    disabled is racy and therefore not helpful
  * when a mutator communicates with BG compiler it will use
    SafepointMutexLocker instead of MutexLocker

This CL also ecapsulates disabling the BG compiler via a
NoBackgroundCompilerScope object. It allows nested disabling and guards
against races.

Issue https://github.com/dart-lang/sdk/issues/36097

TEST=Refactoring of existing code.

Change-Id: I36ed6592c9a8b91e94181698b7de662b4ee984e9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183400
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
This commit is contained in:
Martin Kustermann
2021-02-10 19:42:55 +00:00
committed by commit-bot@chromium.org
parent b948c7ee26
commit 75e0837f3e
10 changed files with 122 additions and 173 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runtime/vm/class_finalizer.cc
+1 -1
View File
@@ -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();
runtime/vm/compiler/jit/compiler.cc
+73 -91
View File
@@ -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<BackgroundCompilerTask>(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<BackgroundCompilerTask>(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
runtime/vm/compiler/jit/compiler.h
+21 -38
View File
@@ -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_
runtime/vm/compiler_test.cc
+3 -5
View File
@@ -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) {
runtime/vm/dart_api_impl.cc
+8 -6
View File
@@ -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);
runtime/vm/debugger.cc
+1 -1
View File
@@ -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");
}
runtime/vm/isolate.cc
+7 -11
View File
@@ -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.
runtime/vm/isolate.h
-8
View File
@@ -1230,10 +1230,6 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
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<Isolate> {
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)
runtime/vm/isolate_reload.cc
+5 -6
View File
@@ -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);
runtime/vm/runtime_entry.cc
+3 -6
View File
@@ -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;