[vm/concurrency] Make thread_registry not depend on [Isolate], move mutator thread from thread registry to isolate

The thread registry is creating [Thread] objects when threads enter an
isolate (as mutator or helper). Once threads exit an isolate, the
[Thread] structure is returned and the thread registry will put it
into a cache for later re-use.

The mutator [Thread] object is an exception: Exiting and entering the
isolate as mutator will always use the same cached [Thread] object.

We want to eventually use one thread registry for an entire isolate
group. There will therefore be multiple mutator threads per thread registry.
We therefore move the cached mutator thread from the thread registry to the
[Isolate] object.

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

Change-Id: Id27dff886d79ca76f6e05320151aeb72c8ba5140
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/108720
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
This commit is contained in:
Martin Kustermann
2019-07-11 13:36:02 +00:00
committed by commit-bot@chromium.org
parent 97587b554f
commit 14ea27e65b
7 changed files with 122 additions and 116 deletions
+18 -2
View File
@@ -31,13 +31,29 @@ class BaseIsolate {
#endif
protected:
BaseIsolate() : scheduled_mutator_thread_(NULL) {}
BaseIsolate() {}
~BaseIsolate() {
// Do not delete stack resources: top_resource_ and current_zone_.
}
Thread* scheduled_mutator_thread_;
Thread* scheduled_mutator_thread_ = nullptr;
// TODO(asiva): Currently we treat a mutator thread as a special thread
// and always schedule execution of Dart code on the same mutator thread
// object. The ApiLocalScope has been made thread specific but we still
// have scenarios where we do a temporary exit of an Isolate with live
// zones/handles in the API scope :
// - Dart_RunLoop()
// - IsolateSaver in Dart_NewNativePort
// Similarly, tracking async_stack_trace requires that we always reschedule
// on the same thread.
// We probably need a mechanism to return to the specific thread only
// for these specific cases. We should also determine if the embedder
// should allow exiting an isolate with live state in zones/handles in
// which case a new API for returning to the specific thread needs to be
// added.
Thread* mutator_thread_ = nullptr;
private:
DISALLOW_COPY_AND_ASSIGN(BaseIsolate);
+42 -42
View File
@@ -138,13 +138,13 @@ static constexpr dart::compiler::target::word ICData_owner_offset = 20;
static constexpr dart::compiler::target::word ICData_state_bits_offset = 28;
static constexpr dart::compiler::target::word
ICData_receivers_static_type_offset = 16;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 32;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 16;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 20;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 24;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 28;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 52;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 12;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 36;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 20;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 24;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 28;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 32;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 56;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 16;
static constexpr dart::compiler::target::word LinkedHashMap_data_offset = 16;
static constexpr dart::compiler::target::word
LinkedHashMap_deleted_keys_offset = 24;
@@ -491,13 +491,13 @@ static constexpr dart::compiler::target::word ICData_owner_offset = 40;
static constexpr dart::compiler::target::word ICData_state_bits_offset = 52;
static constexpr dart::compiler::target::word
ICData_receivers_static_type_offset = 32;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 64;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 32;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 40;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 48;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 56;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 104;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 24;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 72;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 40;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 48;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 56;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 64;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 112;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 32;
static constexpr dart::compiler::target::word LinkedHashMap_data_offset = 32;
static constexpr dart::compiler::target::word
LinkedHashMap_deleted_keys_offset = 48;
@@ -845,13 +845,13 @@ static constexpr dart::compiler::target::word ICData_owner_offset = 20;
static constexpr dart::compiler::target::word ICData_state_bits_offset = 28;
static constexpr dart::compiler::target::word
ICData_receivers_static_type_offset = 16;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 32;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 16;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 20;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 24;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 28;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 52;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 12;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 36;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 20;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 24;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 28;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 32;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 56;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 16;
static constexpr dart::compiler::target::word LinkedHashMap_data_offset = 16;
static constexpr dart::compiler::target::word
LinkedHashMap_deleted_keys_offset = 24;
@@ -1194,13 +1194,13 @@ static constexpr dart::compiler::target::word ICData_owner_offset = 40;
static constexpr dart::compiler::target::word ICData_state_bits_offset = 52;
static constexpr dart::compiler::target::word
ICData_receivers_static_type_offset = 32;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 64;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 32;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 40;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 48;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 56;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 104;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 24;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 72;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 40;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 48;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 56;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 64;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 112;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 32;
static constexpr dart::compiler::target::word LinkedHashMap_data_offset = 32;
static constexpr dart::compiler::target::word
LinkedHashMap_deleted_keys_offset = 48;
@@ -1551,13 +1551,13 @@ static constexpr dart::compiler::target::word ICData_owner_offset = 40;
static constexpr dart::compiler::target::word ICData_state_bits_offset = 52;
static constexpr dart::compiler::target::word
ICData_receivers_static_type_offset = 32;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 64;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 32;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 40;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 48;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 56;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 104;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 24;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 72;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 40;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 48;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 56;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 64;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 112;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 32;
static constexpr dart::compiler::target::word LinkedHashMap_data_offset = 32;
static constexpr dart::compiler::target::word
LinkedHashMap_deleted_keys_offset = 48;
@@ -1835,13 +1835,13 @@ static constexpr dart::compiler::target::word ICData_owner_offset = 20;
static constexpr dart::compiler::target::word ICData_state_bits_offset = 28;
static constexpr dart::compiler::target::word
ICData_receivers_static_type_offset = 16;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 32;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 16;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 20;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 24;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 28;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 52;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 12;
static constexpr dart::compiler::target::word Isolate_class_table_offset = 36;
static constexpr dart::compiler::target::word Isolate_current_tag_offset = 20;
static constexpr dart::compiler::target::word Isolate_default_tag_offset = 24;
static constexpr dart::compiler::target::word Isolate_ic_miss_code_offset = 28;
static constexpr dart::compiler::target::word Isolate_object_store_offset = 32;
static constexpr dart::compiler::target::word Isolate_single_step_offset = 56;
static constexpr dart::compiler::target::word Isolate_user_tag_offset = 16;
static constexpr dart::compiler::target::word LinkedHashMap_data_offset = 16;
static constexpr dart::compiler::target::word
LinkedHashMap_deleted_keys_offset = 24;
+44 -8
View File
@@ -1021,6 +1021,13 @@ Isolate::~Isolate() {
nullptr); // No deopt in progress when isolate deleted.
ASSERT(spawn_count_ == 0);
delete safepoint_handler_;
// We have cached the mutator thread, delete it.
ASSERT(scheduled_mutator_thread_ == nullptr);
mutator_thread_->isolate_ = nullptr;
delete mutator_thread_;
mutator_thread_ = nullptr;
delete thread_registry_;
if (obfuscation_map_ != nullptr) {
@@ -1177,7 +1184,7 @@ void Isolate::RetainKernelBlob(const ExternalTypedData& kernel_blob) {
Thread* Isolate::mutator_thread() const {
ASSERT(thread_registry() != nullptr);
return thread_registry()->mutator_thread();
return mutator_thread_;
}
RawObject* Isolate::CallTagHandler(Dart_LibraryTag tag,
@@ -2051,6 +2058,12 @@ void Isolate::VisitStackPointers(ObjectPointerVisitor* visitor,
ValidationPolicy validate_frames) {
// Visit objects in all threads (e.g., Dart stack, handles in zones).
thread_registry()->VisitObjectPointers(visitor, validate_frames);
// Visit mutator thread, even if the isolate isn't entered/scheduled (there
// might be live API handles to visit).
if (mutator_thread_ != nullptr) {
mutator_thread_->VisitObjectPointers(visitor, validate_frames);
}
}
void Isolate::VisitWeakPersistentHandles(HandleVisitor* visitor) {
@@ -2887,8 +2900,25 @@ Thread* Isolate::ScheduleThread(bool is_mutator, bool bypass_safepoint) {
ml.Wait();
}
// Now get a free Thread structure.
thread = thread_registry()->GetFreeThreadLocked(this, is_mutator);
// NOTE: We cannot just use `Dart::vm_isolate() == this` here, since during
// VM startup it might not have been set at this point.
const bool is_vm_isolate =
Dart::vm_isolate() == nullptr || Dart::vm_isolate() == this;
if (is_mutator) {
if (mutator_thread_ == nullptr) {
// Allocate a new [Thread] structure for the mutator thread.
thread = thread_registry()->GetFreeThreadLocked(is_vm_isolate);
mutator_thread_ = thread;
} else {
// Reuse the existing cached [Thread] structure for the mutator thread.,
// see comment in 'base_isolate.h'.
thread_registry()->AddToActiveListLocked(mutator_thread_);
thread = mutator_thread_;
}
} else {
thread = thread_registry()->GetFreeThreadLocked(is_vm_isolate);
}
ASSERT(thread != nullptr);
thread->ResetHighWatermark();
@@ -2945,9 +2975,7 @@ void Isolate::UnscheduleThread(Thread* thread,
os_thread->DisableThreadInterrupts();
os_thread->set_thread(nullptr);
OSThread::SetCurrent(os_thread);
if (is_mutator) {
scheduled_mutator_thread_ = nullptr;
}
// Even if we unschedule the mutator thread, e.g. via calling
// `Dart_ExitIsolate()` inside a native, we might still have one or more Dart
// stacks active, which e.g. GC marker threads want to visit. So we don't
@@ -2967,8 +2995,16 @@ void Isolate::UnscheduleThread(Thread* thread,
thread->set_safepoint_state(Thread::SetAtSafepoint(true, 0));
thread->clear_pending_functions();
ASSERT(thread->no_safepoint_scope_depth() == 0);
// Return thread structure.
thread_registry()->ReturnThreadLocked(is_mutator, thread);
if (is_mutator) {
ASSERT(mutator_thread_ == thread);
ASSERT(mutator_thread_ == scheduled_mutator_thread_);
thread_registry()->RemoveFromActiveListLocked(thread);
scheduled_mutator_thread_ = nullptr;
} else {
// Return thread structure.
thread_registry()->ReturnThreadLocked(thread);
}
}
static const char* NewConstChar(const char* chars) {
+2 -2
View File
@@ -56,7 +56,7 @@ Thread::~Thread() {
#define REUSABLE_HANDLE_INITIALIZERS(object) object##_handle_(NULL),
Thread::Thread(Isolate* isolate)
Thread::Thread(bool is_vm_isolate)
: ThreadState(false),
stack_limit_(0),
stack_overflow_flags_(0),
@@ -133,7 +133,7 @@ Thread::Thread(Isolate* isolate)
// We cannot initialize the VM constants here for the vm isolate thread
// due to boot strapping issues.
if ((Dart::vm_isolate() != NULL) && (isolate != Dart::vm_isolate())) {
if (!is_vm_isolate) {
InitVMConstants();
}
}
+1 -1
View File
@@ -938,7 +938,7 @@ class Thread : public ThreadState {
Thread* next_; // Used to chain the thread structures in an isolate.
explicit Thread(Isolate* isolate);
explicit Thread(bool is_vm_isolate);
void StoreBufferRelease(
StoreBuffer::ThresholdPolicy policy = StoreBuffer::kCheckThreshold);
+11 -35
View File
@@ -4,7 +4,6 @@
#include "vm/thread_registry.h"
#include "vm/isolate.h"
#include "vm/json_stream.h"
#include "vm/lockers.h"
@@ -17,12 +16,6 @@ ThreadRegistry::~ThreadRegistry() {
// At this point the active list should be empty.
ASSERT(active_list_ == NULL);
// The [mutator_thread_] is kept alive until the destruction of the isolate.
mutator_thread_->isolate_ = nullptr;
// We have cached the mutator thread, delete it.
delete mutator_thread_;
mutator_thread_ = NULL;
// Now delete all the threads in the free list.
while (free_list_ != NULL) {
Thread* thread = free_list_;
@@ -32,51 +25,34 @@ ThreadRegistry::~ThreadRegistry() {
}
}
// Gets a free Thread structure, we special case the mutator thread
// by reusing the cached structure, see comment in 'thread_registry.h'.
Thread* ThreadRegistry::GetFreeThreadLocked(Isolate* isolate, bool is_mutator) {
Thread* ThreadRegistry::GetFreeThreadLocked(bool is_vm_isolate) {
ASSERT(threads_lock()->IsOwnedByCurrentThread());
Thread* thread;
if (is_mutator) {
if (mutator_thread_ == NULL) {
mutator_thread_ = GetFromFreelistLocked(isolate);
}
thread = mutator_thread_;
} else {
thread = GetFromFreelistLocked(isolate);
ASSERT(thread->api_top_scope() == NULL);
}
Thread* thread = GetFromFreelistLocked(is_vm_isolate);
ASSERT(thread->api_top_scope() == NULL);
// Now add this Thread to the active list for the isolate.
AddToActiveListLocked(thread);
return thread;
}
void ThreadRegistry::ReturnThreadLocked(bool is_mutator, Thread* thread) {
void ThreadRegistry::ReturnThreadLocked(Thread* thread) {
ASSERT(threads_lock()->IsOwnedByCurrentThread());
// Remove thread from the active list for the isolate.
RemoveFromActiveListLocked(thread);
if (!is_mutator) {
ReturnToFreelistLocked(thread);
}
ReturnToFreelistLocked(thread);
}
void ThreadRegistry::VisitObjectPointers(ObjectPointerVisitor* visitor,
ValidationPolicy validate_frames) {
MonitorLocker ml(threads_lock());
bool mutator_thread_visited = false;
Thread* thread = active_list_;
while (thread != NULL) {
thread->VisitObjectPointers(visitor, validate_frames);
if (mutator_thread_ == thread) {
mutator_thread_visited = true;
// The mutator thread is visited by the isolate itself (see
// [Isolate::VisitStackPointers]).
if (!thread->IsMutatorThread()) {
thread->VisitObjectPointers(visitor, validate_frames);
}
thread = thread->next_;
}
// Visit mutator thread even if it is not in the active list because of
// api handles.
if (!mutator_thread_visited && (mutator_thread_ != NULL)) {
mutator_thread_->VisitObjectPointers(visitor, validate_frames);
}
}
void ThreadRegistry::ReleaseStoreBuffers() {
@@ -174,12 +150,12 @@ void ThreadRegistry::RemoveFromActiveListLocked(Thread* thread) {
}
}
Thread* ThreadRegistry::GetFromFreelistLocked(Isolate* isolate) {
Thread* ThreadRegistry::GetFromFreelistLocked(bool is_vm_isolate) {
ASSERT(threads_lock()->IsOwnedByCurrentThread());
Thread* thread = NULL;
// Get thread structure from free list or create a new one.
if (free_list_ == NULL) {
thread = new Thread(isolate);
thread = new Thread(is_vm_isolate);
} else {
thread = free_list_;
free_list_ = thread->next_;
+4 -26
View File
@@ -22,11 +22,7 @@ class JSONArray;
// Unordered collection of threads relating to a particular isolate.
class ThreadRegistry {
public:
ThreadRegistry()
: threads_lock_(),
active_list_(NULL),
free_list_(NULL),
mutator_thread_(NULL) {}
ThreadRegistry() : threads_lock_(), active_list_(NULL), free_list_(NULL) {}
~ThreadRegistry();
void VisitObjectPointers(ObjectPointerVisitor* visitor,
@@ -36,8 +32,6 @@ class ThreadRegistry {
void AcquireMarkingStacks();
void ReleaseMarkingStacks();
Thread* mutator_thread() const { return mutator_thread_; }
#ifndef PRODUCT
void PrintJSON(JSONStream* stream) const;
#endif
@@ -52,11 +46,11 @@ class ThreadRegistry {
Thread* active_list() const { return active_list_; }
Monitor* threads_lock() const { return &threads_lock_; }
Thread* GetFreeThreadLocked(Isolate* isolate, bool is_mutator);
void ReturnThreadLocked(bool is_mutator, Thread* thread);
Thread* GetFreeThreadLocked(bool is_vm_isolate);
void ReturnThreadLocked(Thread* thread);
void AddToActiveListLocked(Thread* thread);
void RemoveFromActiveListLocked(Thread* thread);
Thread* GetFromFreelistLocked(Isolate* isolate);
Thread* GetFromFreelistLocked(bool is_vm_isolate);
void ReturnToFreelistLocked(Thread* thread);
// This monitor protects the threads list for an isolate, it is used whenever
@@ -65,22 +59,6 @@ class ThreadRegistry {
Thread* active_list_; // List of active threads in the isolate.
Thread* free_list_; // Free list of Thread objects that can be reused.
// TODO(asiva): Currently we treat a mutator thread as a special thread
// and always schedule execution of Dart code on the same mutator thread
// object. The ApiLocalScope has been made thread specific but we still
// have scenarios where we do a temporary exit of an Isolate with live
// zones/handles in the API scope :
// - Dart_RunLoop()
// - IsolateSaver in Dart_NewNativePort
// Similarly, tracking async_stack_trace requires that we always reschedule
// on the same thread.
// We probably need a mechanism to return to the specific thread only
// for these specific cases. We should also determine if the embedder
// should allow exiting an isolate with live state in zones/handles in
// which case a new API for returning to the specific thread needs to be
// added.
Thread* mutator_thread_;
friend class Isolate;
friend class SafepointHandler;
friend class Scavenger;