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

[vm/concurrency] Add IsolateGroup::RunWithStoppedMutators and use it in various places

Browse Source 
When installing new code, we need to ensure the mutator is stopped for a
variety of reasons.

Right now we only need to distuinguish mutator and background compiler
when installing code: The mutator can install code without any
synchronization whereas the bg compiler needs to get the mutator to a safepoint.

Yet once all isolates within one isolate group share a heap, a mutator
might need to stop all other mutators before installing code (since they
operate on pages on which we flip page protection bits)

This CL adds IsolateGroup::RunWithStoppedMutators which will get other
mutators to a safepoint (if there are multiple or we are on a bg
compiler thread).

Along with this we add a read-write lock and use it for the protection
of the IsolateGroup::isolate_: While we make assumptions about the
number of isolates in a group we force all pending additions of new
isolates to wait. Later on this will also be used to allow iterating the
list of isolates during GC and prevent new isolates from being added at
the same time.

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

Change-Id: I6e761fa51d36b2f2b4b67995cac954898ce7fd69
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-android-release-arm-try,vm-dartkb-linux-release-x64-abi-try,vm-kernel-precomp-bare-linux-release-x64-try,vm-kernel-precomp-mac-debug-simarm_x64-try,vm-kernel-precomp-win-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116767
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
This commit is contained in:
Martin Kustermann
2019-09-18 12:24:31 +00:00
committed by commit-bot@chromium.org
parent 97b561a8de
commit 38c6152884
10 changed files with 281 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runtime/vm/code_patcher_ia32.cc
+23 -12
View File
@@ -64,8 +64,10 @@ class NativeCall : public UnoptimizedCall {
}
void set_native_function(NativeFunction func) const {
WritableInstructionsScope writable(start_ + 1, sizeof(func));
*reinterpret_cast<NativeFunction*>(start_ + 1) = func;
Thread::Current()->isolate_group()->RunWithStoppedMutators([&]() {
WritableInstructionsScope writable(start_ + 1, sizeof(func));
*reinterpret_cast<NativeFunction*>(start_ + 1) = func;
});
}
private:
@@ -179,11 +181,15 @@ RawCode* CodePatcher::GetStaticCallTargetAt(uword return_address,
void CodePatcher::PatchStaticCallAt(uword return_address,
const Code& code,
const Code& new_target) {
const Instructions& instrs = Instructions::Handle(code.instructions());
WritableInstructionsScope writable(instrs.PayloadStart(), instrs.Size());
ASSERT(code.ContainsInstructionAt(return_address));
StaticCall call(return_address);
call.set_target(new_target);
auto thread = Thread::Current();
auto zone = thread->zone();
const Instructions& instrs = Instructions::Handle(zone, code.instructions());
thread->isolate_group()->RunWithStoppedMutators([&]() {
WritableInstructionsScope writable(instrs.PayloadStart(), instrs.Size());
ASSERT(code.ContainsInstructionAt(return_address));
StaticCall call(return_address);
call.set_target(new_target);
});
}
void CodePatcher::InsertDeoptimizationCallAt(uword start) {
@@ -205,12 +211,17 @@ void CodePatcher::PatchInstanceCallAt(uword return_address,
const Code& caller_code,
const Object& data,
const Code& target) {
auto thread = Thread::Current();
auto zone = thread->zone();
ASSERT(caller_code.ContainsInstructionAt(return_address));
const Instructions& instrs = Instructions::Handle(caller_code.instructions());
WritableInstructionsScope writable(instrs.PayloadStart(), instrs.Size());
InstanceCall call(return_address);
call.set_data(data);
call.set_target(target);
const Instructions& instrs =
Instructions::Handle(zone, caller_code.instructions());
thread->isolate_group()->RunWithStoppedMutators([&]() {
WritableInstructionsScope writable(instrs.PayloadStart(), instrs.Size());
InstanceCall call(return_address);
call.set_data(data);
call.set_target(target);
});
}
RawFunction* CodePatcher::GetUnoptimizedStaticCallAt(uword return_address,
runtime/vm/compiler/jit/compiler.cc
+24 -15
View File
@@ -646,25 +646,34 @@ RawCode* CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
}
{
TIMELINE_DURATION(thread(), CompilerVerbose, "FinalizeCompilation");
if (thread()->IsMutatorThread()) {
auto mutator_fun = [&]() {
*result =
FinalizeCompilation(&assembler, &graph_compiler, flow_graph);
} else {
// This part of compilation must be at a safepoint.
// Stop mutator thread before creating the instruction object and
// installing code.
// Mutator thread may not run code while we are creating the
// instruction object, since the creation of instruction object
// changes code page access permissions (makes them temporary not
// executable).
{
};
auto bg_compiler_fun = [&]() {
if (Compiler::IsBackgroundCompilation()) {
CheckIfBackgroundCompilerIsBeingStopped(optimized());
ForceGrowthSafepointOperationScope safepoint_scope(thread());
CheckIfBackgroundCompilerIsBeingStopped(optimized());
*result =
FinalizeCompilation(&assembler, &graph_compiler, flow_graph);
}
}
*result =
FinalizeCompilation(&assembler, &graph_compiler, flow_graph);
};
// We have to ensure no mutators are running, because:
//
// a) We allocate an instructions object, which might cause us to
// temporarily flip page protections (RX -> RW -> RX).
//
// b) We have to ensure the code generated does not violate
// assumptions (e.g. CHA, field guards), the validation has to
// happen while mutator is stopped.
//
// b) We update the [Function] object with a new [Code] which
// requires updating several pointers: We have to ensure all of
// those writes are observed atomically.
//
thread()->isolate_group()->RunWithStoppedMutators(
mutator_fun, bg_compiler_fun, /*use_force_growth=*/true);
// We notify code observers after finalizing the code in order to be
// outside a [SafepointOperationScope].
runtime/vm/debugger_dbc.cc
+12 -8
View File
@@ -29,9 +29,11 @@ static Instr* FastSmiInstructionFromReturnAddress(uword pc) {
void CodeBreakpoint::PatchCode() {
ASSERT(!is_enabled_);
const Code& code = Code::Handle(code_);
const Instructions& instrs = Instructions::Handle(code.instructions());
{
auto thread = Thread::Current();
auto zone = thread->zone();
const Code& code = Code::Handle(zone, code_);
const Instructions& instrs = Instructions::Handle(zone, code.instructions());
thread->isolate_group()->RunWithStoppedMutators([&]() {
WritableInstructionsScope writable(instrs.PayloadStart(), instrs.Size());
saved_value_ = *CallInstructionFromReturnAddress(pc_);
switch (breakpoint_kind_) {
@@ -67,15 +69,17 @@ void CodeBreakpoint::PatchCode() {
} else {
saved_value_fastsmi_ = SimulatorBytecode::kTrap;
}
}
});
is_enabled_ = true;
}
void CodeBreakpoint::RestoreCode() {
ASSERT(is_enabled_);
const Code& code = Code::Handle(code_);
const Instructions& instrs = Instructions::Handle(code.instructions());
{
auto thread = Thread::Current();
auto zone = thread->zone();
const Code& code = Code::Handle(zone, code_);
const Instructions& instrs = Instructions::Handle(zone, code.instructions());
thread->isolate_group()->RunWithStoppedMutators([&]() {
WritableInstructionsScope writable(instrs.PayloadStart(), instrs.Size());
switch (breakpoint_kind_) {
case RawPcDescriptors::kIcCall:
@@ -94,7 +98,7 @@ void CodeBreakpoint::RestoreCode() {
SimulatorBytecode::kNop);
*FastSmiInstructionFromReturnAddress(pc_) = saved_value_fastsmi_;
}
}
});
is_enabled_ = false;
}
runtime/vm/debugger_ia32.cc
+13 -9
View File
@@ -25,10 +25,12 @@ RawCode* CodeBreakpoint::OrigStubAddress() const {
void CodeBreakpoint::PatchCode() {
ASSERT(!is_enabled_);
const Code& code = Code::Handle(code_);
const Instructions& instrs = Instructions::Handle(code.instructions());
Code& stub_target = Code::Handle();
{
auto thread = Thread::Current();
auto zone = thread->zone();
const Code& code = Code::Handle(zone, code_);
const Instructions& instrs = Instructions::Handle(zone, code.instructions());
Code& stub_target = Code::Handle(zone);
thread->isolate_group()->RunWithStoppedMutators([&]() {
WritableInstructionsScope writable(instrs.PayloadStart(), instrs.Size());
switch (breakpoint_kind_) {
case RawPcDescriptors::kIcCall: {
@@ -49,15 +51,17 @@ void CodeBreakpoint::PatchCode() {
}
saved_value_ = CodePatcher::GetStaticCallTargetAt(pc_, code);
CodePatcher::PatchStaticCallAt(pc_, code, stub_target);
}
});
is_enabled_ = true;
}
void CodeBreakpoint::RestoreCode() {
ASSERT(is_enabled_);
const Code& code = Code::Handle(code_);
const Instructions& instrs = Instructions::Handle(code.instructions());
{
auto thread = Thread::Current();
auto zone = thread->zone();
const Code& code = Code::Handle(zone, code_);
const Instructions& instrs = Instructions::Handle(zone, code.instructions());
thread->isolate_group()->RunWithStoppedMutators([&]() {
WritableInstructionsScope writable(instrs.PayloadStart(), instrs.Size());
switch (breakpoint_kind_) {
case RawPcDescriptors::kIcCall:
@@ -69,7 +73,7 @@ void CodeBreakpoint::RestoreCode() {
default:
UNREACHABLE();
}
}
});
is_enabled_ = false;
}
runtime/vm/isolate.cc
+31 -7
View File
@@ -132,17 +132,17 @@ static RawInstance* DeserializeMessage(Thread* thread, Message* message) {
IsolateGroup::IsolateGroup(std::unique_ptr<IsolateGroupSource> source,
void* embedder_data)
: source_(std::move(source)),
embedder_data_(embedder_data),
: embedder_data_(embedder_data),
isolates_rwlock_(new RwLock()),
isolates_(),
source_(std::move(source)),
thread_registry_(new ThreadRegistry()),
safepoint_handler_(new SafepointHandler(this)),
isolates_monitor_(new Monitor()),
isolates_() {}
safepoint_handler_(new SafepointHandler(this)) {}
IsolateGroup::~IsolateGroup() {}
void IsolateGroup::RegisterIsolate(Isolate* isolate) {
MonitorLocker ml(isolates_monitor_.get());
WriteRwLocker wl(ThreadState::Current(), isolates_rwlock_.get());
isolates_.Append(isolate);
isolate_count_++;
}
@@ -150,7 +150,7 @@ void IsolateGroup::RegisterIsolate(Isolate* isolate) {
void IsolateGroup::UnregisterIsolate(Isolate* isolate) {
bool is_last_isolate = false;
{
MonitorLocker ml(isolates_monitor_.get());
WriteRwLocker wl(ThreadState::Current(), isolates_rwlock_.get());
isolates_.Remove(isolate);
isolate_count_--;
is_last_isolate = isolate_count_ == 0;
@@ -2226,6 +2226,30 @@ void Isolate::DisableIncrementalBarrier() {
ASSERT(!Thread::Current()->is_marking());
}
void IsolateGroup::RunWithStoppedMutators(
std::function<void()> single_current_mutator,
std::function<void()> otherwise,
bool use_force_growth_in_otherwise) {
auto thread = Thread::Current();
ReadRwLocker wl(thread, isolates_rwlock_.get());
const bool only_one_isolate = isolates_.First() == isolates_.Last();
if (thread->IsMutatorThread() && only_one_isolate) {
single_current_mutator();
} else {
// We use the more strict safepoint operation scope here (which ensures that
// all other threads, including auxiliary threads are at a safepoint), even
// though we only need to ensure that the mutator threads are stopped.
if (use_force_growth_in_otherwise) {
ForceGrowthSafepointOperationScope safepoint_scope(thread);
otherwise();
} else {
SafepointOperationScope safepoint_scope(thread);
otherwise();
}
}
}
RawClass* Isolate::GetClassForHeapWalkAt(intptr_t cid) {
RawClass* raw_class = nullptr;
#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
runtime/vm/isolate.h
+26 -4
View File
@@ -9,6 +9,7 @@
#error "Should not include runtime"
#endif
#include <functional>
#include <memory>
#include <utility>
@@ -78,6 +79,7 @@ class RawFloat32x4;
class RawInt32x4;
class RawUserTag;
class ReversePcLookupCache;
class RwLock;
class SafepointHandler;
class SampleBuffer;
class SendPort;
@@ -262,16 +264,36 @@ class IsolateGroup {
library_tag_handler_ = handler;
}
// Ensures mutators are stopped during execution of the provided function.
//
// If the current thread is the only mutator in the isolate group,
// [single_current_mutator] will be called. Otherwise [otherwise] will be
// called inside a [SafepointOperationsScope] (or
// [ForceGrowthSafepointOperationScope] if [use_force_growth_in_otherwise]
// is set).
//
// During the duration of this function, no new isolates can be added to the
// isolate group.
void RunWithStoppedMutators(std::function<void()> single_current_mutator,
std::function<void()> otherwise,
bool use_force_growth_in_otherwise = false);
void RunWithStoppedMutators(std::function<void()> function) {
RunWithStoppedMutators(function, function);
}
private:
std::unique_ptr<IsolateGroupSource> source_;
void* embedder_data_ = nullptr;
std::unique_ptr<ThreadRegistry> thread_registry_;
std::unique_ptr<SafepointHandler> safepoint_handler_;
std::unique_ptr<Monitor> isolates_monitor_;
std::unique_ptr<RwLock> isolates_rwlock_;
IntrusiveDList<Isolate> isolates_;
intptr_t isolate_count_ = 0;
bool initial_spawn_successful_ = false;
Dart_LibraryTagHandler library_tag_handler_ = nullptr;
std::unique_ptr<IsolateGroupSource> source_;
std::unique_ptr<ThreadRegistry> thread_registry_;
std::unique_ptr<SafepointHandler> safepoint_handler_;
};
class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
runtime/vm/lockers.cc
+18
View File
@@ -101,4 +101,22 @@ Monitor::WaitResult SafepointMonitorLocker::Wait(int64_t millis) {
}
}
ReadRwLocker::ReadRwLocker(ThreadState* thread_state, RwLock* rw_lock)
: StackResource(thread_state), rw_lock_(rw_lock) {
rw_lock_->EnterRead();
}
ReadRwLocker::~ReadRwLocker() {
rw_lock_->LeaveRead();
}
WriteRwLocker::WriteRwLocker(ThreadState* thread_state, RwLock* rw_lock)
: StackResource(thread_state), rw_lock_(rw_lock) {
rw_lock_->EnterWrite();
}
WriteRwLocker::~WriteRwLocker() {
rw_lock_->LeaveWrite();
}
} // namespace dart
runtime/vm/lockers.h
+85
View File
@@ -252,6 +252,91 @@ class SafepointMonitorLocker : public ValueObject {
DISALLOW_COPY_AND_ASSIGN(SafepointMonitorLocker);
};
class RwLock {
public:
RwLock() {}
~RwLock() {}
private:
friend class ReadRwLocker;
friend class WriteRwLocker;
void EnterRead() {
MonitorLocker ml(&monitor_);
while (state_ == -1) {
ml.Wait();
}
++state_;
}
void LeaveRead() {
MonitorLocker ml(&monitor_);
ASSERT(state_ > 0);
if (--state_ == 0) {
ml.NotifyAll();
}
}
void EnterWrite() {
MonitorLocker ml(&monitor_);
while (state_ != 0) {
ml.Wait();
}
state_ = -1;
}
void LeaveWrite() {
MonitorLocker ml(&monitor_);
ASSERT(state_ == -1);
state_ = 0;
ml.NotifyAll();
}
Monitor monitor_;
// [state_] > 0 : The lock is held by multiple readers.
// [state_] == 0 : The lock is free (no readers/writers).
// [state_] == -1: The lock is held by a single writer.
intptr_t state_ = 0;
};
/*
* Locks a given [RwLock] for reading purposes.
*
* It will block while the lock is held by a writer.
*
* If this locker is long'jmped over (e.g. on a background compiler thread) the
* lock will be freed.
*
* NOTE: If the locking operation blocks (due to a writer) it will not check
* for a pending safepoint operation.
*/
class ReadRwLocker : public StackResource {
public:
ReadRwLocker(ThreadState* thread_state, RwLock* rw_lock);
~ReadRwLocker();
private:
RwLock* rw_lock_;
};
/*
* Locks a given [RwLock] for writing purposes.
*
* It will block while the lock is held by one or more readers.
*
* If this locker is long'jmped over (e.g. on a background compiler thread) the
* lock will be freed.
*
* NOTE: If the locking operation blocks (due to a writer) it will not check
* for a pending safepoint operation.
*/
class WriteRwLocker : public StackResource {
public:
WriteRwLocker(ThreadState* thread_state, RwLock* rw_lock);
~WriteRwLocker();
private:
RwLock* rw_lock_;
};
} // namespace dart
#endif // RUNTIME_VM_LOCKERS_H_
runtime/vm/stub_code.cc
+30 -27
View File
@@ -178,40 +178,43 @@ RawCode* StubCode::GetAllocationStubForClass(const Class& cls) {
const char* name = cls.ToCString();
compiler::StubCodeCompiler::GenerateAllocationStubForClass(&assembler, cls);
if (thread->IsMutatorThread()) {
stub = Code::FinalizeCodeAndNotify(name, nullptr, &assembler,
pool_attachment,
/*optimized1*/ false);
auto mutator_fun = [&]() {
stub = Code::FinalizeCode(nullptr, &assembler, pool_attachment,
/*optimized=*/false,
/*stats=*/nullptr);
// Check if background compilation thread has not already added the stub.
if (cls.allocation_stub() == Code::null()) {
stub.set_owner(cls);
cls.set_allocation_stub(stub);
}
} else {
// This part of stub code generation must be at a safepoint.
// Stop mutator thread before creating the instruction object and
// installing code.
// Mutator thread may not run code while we are creating the
// instruction object, since the creation of instruction object
// changes code page access permissions (makes them temporary not
// executable).
{
ForceGrowthSafepointOperationScope safepoint_scope(thread);
stub = cls.allocation_stub();
// Check if stub was already generated.
if (!stub.IsNull()) {
return stub.raw();
}
stub = Code::FinalizeCode(nullptr, &assembler, pool_attachment,
/*optimized=*/false, /*stats=*/nullptr);
stub.set_owner(cls);
cls.set_allocation_stub(stub);
};
auto bg_compiler_fun = [&]() {
ForceGrowthSafepointOperationScope safepoint_scope(thread);
stub = cls.allocation_stub();
// Check if stub was already generated.
if (!stub.IsNull()) {
return;
}
stub = Code::FinalizeCode(nullptr, &assembler, pool_attachment,
/*optimized=*/false, /*stats=*/nullptr);
stub.set_owner(cls);
cls.set_allocation_stub(stub);
};
// We notify code observers after finalizing the code in order to be
// outside a [SafepointOperationScope].
Code::NotifyCodeObservers(name, stub, /*optimized=*/false);
}
// We have to ensure no mutators are running, because:
//
// a) We allocate an instructions object, which might cause us to
// temporarily flip page protections from (RX -> RW -> RX).
//
// b) To ensure only one thread succeeds installing an allocation for the
// given class.
//
thread->isolate_group()->RunWithStoppedMutators(
mutator_fun, bg_compiler_fun, /*use_force_growth=*/true);
// We notify code observers after finalizing the code in order to be
// outside a [SafepointOperationScope].
Code::NotifyCodeObservers(name, stub, /*optimized=*/false);
#ifndef PRODUCT
if (FLAG_support_disassembler && FLAG_disassemble_stubs) {
LogBlock lb;
runtime/vm/type_testing_stubs.cc
+19 -3
View File
@@ -176,7 +176,8 @@ RawCode* TypeTestingStubGenerator::OptimizedCodeForType(
#if !defined(DART_PRECOMPILED_RUNTIME)
RawCode* TypeTestingStubGenerator::BuildCodeForType(const Type& type) {
HierarchyInfo* hi = Thread::Current()->hierarchy_info();
auto thread = Thread::Current();
HierarchyInfo* hi = thread->hierarchy_info();
ASSERT(hi != NULL);
if (!hi->CanUseSubtypeRangeCheckFor(type) &&
@@ -195,8 +196,23 @@ RawCode* TypeTestingStubGenerator::BuildCodeForType(const Type& type) {
const auto pool_attachment = FLAG_use_bare_instructions
? Code::PoolAttachment::kNotAttachPool
: Code::PoolAttachment::kAttachPool;
const Code& code = Code::Handle(Code::FinalizeCodeAndNotify(
name, nullptr, &assembler, pool_attachment, false /* optimized */));
Code& code = Code::Handle(thread->zone());
auto install_code_fun = [&]() {
code = Code::FinalizeCode(nullptr, &assembler, pool_attachment,
/*optimized=*/false, /*stats=*/nullptr);
};
// We have to ensure no mutators are running, because:
//
// a) We allocate an instructions object, which might cause us to
// temporarily flip page protections from (RX -> RW -> RX).
//
thread->isolate_group()->RunWithStoppedMutators(
install_code_fun, install_code_fun, /*use_force_growth=*/true);
Code::NotifyCodeObservers(name, code, /*optimized=*/false);
code.set_owner(type);
#ifndef PRODUCT
if (FLAG_support_disassembler && FLAG_disassemble_stubs) {