Skip to content

Commit

Permalink
[vm/concurrency] Add patchable_call_mutex and take it in miss handlers
Browse files Browse the repository at this point in the history 
We serialize all changes to patchable call state to ensure we always
make progress (e.g. transition from Monomorphic -> Polymorphic).

Only if the miss handler actually needs to patch the patchable call do
we stop mutators - if it only updates ICData we don't have to do that.
=> In the future we might introduce a more clever mechanism to avoid
   stopping mutators.

The CL also moves some locks from Isolate to IsolateGroup to ensure
all mutators use the same lock, since they operate on shared data
structures.

Issue dart-lang/sdk#36097

TEST=Existing isolate tests that --enable-isolate-groups.

Change-Id: I4a11a8b8bb65581c6aba11cdf73acdea0f24a502
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/174469
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
  • Loading branch information
@mkustermann
mkustermann authored and commit-bot@chromium.org committed Dec 2, 2020
1 parent 8cba879 commit 759caec
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 79 deletions.
2 changes: 1 addition & 1 deletion runtime/vm/compiler/backend/il.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,7 @@ void CallTargets::CreateHelper(Zone* zone, const ICData& ic_data) {
const auto& cache = MegamorphicCache::Handle(
zone, MegamorphicCacheTable::Lookup(thread, name, descriptor));
{
SafepointMutexLocker ml(thread->isolate()->type_feedback_mutex());
SafepointMutexLocker ml(thread->isolate_group()->type_feedback_mutex());
MegamorphicCacheEntries entries(Array::Handle(zone, cache.buckets()));
for (intptr_t i = 0, n = entries.Length(); i < n; i++) {
const intptr_t id =
Expand Down
9 changes: 6 additions & 3 deletions runtime/vm/isolate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,12 @@ IsolateGroup::IsolateGroup(std::shared_ptr<IsolateGroupSource> source,
"IsolateGroup::type_arguments_canonicalization_mutex_")),
subtype_test_cache_mutex_(
NOT_IN_PRODUCT("IsolateGroup::subtype_test_cache_mutex_")),
megamorphic_table_mutex_(
NOT_IN_PRODUCT("IsolateGroup::megamorphic_table_mutex_")),
type_feedback_mutex_(
NOT_IN_PRODUCT("IsolateGroup::type_feedback_mutex_")),
patchable_call_mutex_(
NOT_IN_PRODUCT("IsolateGroup::patchable_call_mutex_")),
program_lock_(new SafepointRwLock()),
active_mutators_monitor_(new Monitor()),
max_active_mutators_(Scavenger::MaxMutatorThreadCount()) {
Expand Down Expand Up @@ -1648,9 +1654,6 @@ Isolate::Isolate(IsolateGroup* isolate_group,
mutex_(NOT_IN_PRODUCT("Isolate::mutex_")),
constant_canonicalization_mutex_(
NOT_IN_PRODUCT("Isolate::constant_canonicalization_mutex_")),
megamorphic_table_mutex_(
NOT_IN_PRODUCT("Isolate::megamorphic_table_mutex_")),
type_feedback_mutex_(NOT_IN_PRODUCT("Isolate::type_feedback_mutex_")),
kernel_data_lib_cache_mutex_(
NOT_IN_PRODUCT("Isolate::kernel_data_lib_cache_mutex_")),
kernel_data_class_cache_mutex_(
Expand Down
11 changes: 6 additions & 5 deletions runtime/vm/isolate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,9 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
return &type_arguments_canonicalization_mutex_;
}
Mutex* subtype_test_cache_mutex() { return &subtype_test_cache_mutex_; }
Mutex* megamorphic_table_mutex() { return &megamorphic_table_mutex_; }
Mutex* type_feedback_mutex() { return &type_feedback_mutex_; }
Mutex* patchable_call_mutex() { return &patchable_call_mutex_; }

#if defined(DART_PRECOMPILED_RUNTIME)
Mutex* unlinked_call_map_mutex() { return &unlinked_call_map_mutex_; }
Expand Down Expand Up @@ -722,6 +725,9 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
Mutex type_canonicalization_mutex_;
Mutex type_arguments_canonicalization_mutex_;
Mutex subtype_test_cache_mutex_;
Mutex megamorphic_table_mutex_;
Mutex type_feedback_mutex_;
Mutex patchable_call_mutex_;

#if defined(DART_PRECOMPILED_RUNTIME)
Mutex unlinked_call_map_mutex_;
Expand Down Expand Up @@ -964,9 +970,6 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
Mutex* constant_canonicalization_mutex() {
return &constant_canonicalization_mutex_;
}
Mutex* megamorphic_table_mutex() { return &megamorphic_table_mutex_; }
Mutex* type_feedback_mutex() { return &type_feedback_mutex_; }

Mutex* kernel_data_lib_cache_mutex() { return &kernel_data_lib_cache_mutex_; }
Mutex* kernel_data_class_cache_mutex() {
return &kernel_data_class_cache_mutex_;
Expand Down Expand Up @@ -1601,8 +1604,6 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
Simulator* simulator_ = nullptr;
Mutex mutex_; // Protects compiler stats.
Mutex constant_canonicalization_mutex_; // Protects const canonicalization.
Mutex megamorphic_table_mutex_;
Mutex type_feedback_mutex_;
Mutex kernel_data_lib_cache_mutex_;
Mutex kernel_data_class_cache_mutex_;
Mutex kernel_constants_mutex_;
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/megamorphic_cache_table.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ MegamorphicCachePtr MegamorphicCacheTable::Lookup(Thread* thread,
const String& name,
const Array& descriptor) {
Isolate* isolate = thread->isolate();
SafepointMutexLocker ml(isolate->megamorphic_table_mutex());
SafepointMutexLocker ml(thread->isolate_group()->megamorphic_table_mutex());

ASSERT(name.IsSymbol());
// TODO(rmacnak): ASSERT(descriptor.IsCanonical());
Expand Down Expand Up @@ -46,7 +46,7 @@ MegamorphicCachePtr MegamorphicCacheTable::Lookup(Thread* thread,

void MegamorphicCacheTable::PrintSizes(Isolate* isolate) {
auto thread = Thread::Current();
SafepointMutexLocker ml(thread->isolate()->megamorphic_table_mutex());
SafepointMutexLocker ml(thread->isolate_group()->megamorphic_table_mutex());

StackZone zone(thread);
intptr_t size = 0;
Expand Down
37 changes: 22 additions & 15 deletions runtime/vm/object.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15239,7 +15239,7 @@ bool ICData::ValidateInterceptor(const Function& target) const {
void ICData::EnsureHasCheck(const GrowableArray<intptr_t>& class_ids,
const Function& target,
intptr_t count) const {
SafepointMutexLocker ml(Isolate::Current()->type_feedback_mutex());
SafepointMutexLocker ml(IsolateGroup::Current()->type_feedback_mutex());

if (FindCheck(class_ids) != -1) return;
AddCheckInternal(class_ids, target, count);
Expand All @@ -15248,14 +15248,15 @@ void ICData::EnsureHasCheck(const GrowableArray<intptr_t>& class_ids,
void ICData::AddCheck(const GrowableArray<intptr_t>& class_ids,
const Function& target,
intptr_t count) const {
SafepointMutexLocker ml(Isolate::Current()->type_feedback_mutex());
SafepointMutexLocker ml(IsolateGroup::Current()->type_feedback_mutex());
AddCheckInternal(class_ids, target, count);
}

void ICData::AddCheckInternal(const GrowableArray<intptr_t>& class_ids,
const Function& target,
intptr_t count) const {
ASSERT(Isolate::Current()->type_feedback_mutex()->IsOwnedByCurrentThread());
ASSERT(
IsolateGroup::Current()->type_feedback_mutex()->IsOwnedByCurrentThread());

ASSERT(!is_tracking_exactness());
ASSERT(!target.IsNull());
Expand Down Expand Up @@ -15343,7 +15344,7 @@ void ICData::EnsureHasReceiverCheck(intptr_t receiver_class_id,
const Function& target,
intptr_t count,
StaticTypeExactnessState exactness) const {
SafepointMutexLocker ml(Isolate::Current()->type_feedback_mutex());
SafepointMutexLocker ml(IsolateGroup::Current()->type_feedback_mutex());

GrowableArray<intptr_t> class_ids(1);
class_ids.Add(receiver_class_id);
Expand All @@ -15356,7 +15357,7 @@ void ICData::AddReceiverCheck(intptr_t receiver_class_id,
const Function& target,
intptr_t count,
StaticTypeExactnessState exactness) const {
SafepointMutexLocker ml(Isolate::Current()->type_feedback_mutex());
SafepointMutexLocker ml(IsolateGroup::Current()->type_feedback_mutex());
AddReceiverCheckInternal(receiver_class_id, target, count, exactness);
}

Expand Down Expand Up @@ -17174,7 +17175,7 @@ MegamorphicCachePtr MegamorphicCache::New(const String& target_name,

void MegamorphicCache::EnsureContains(const Smi& class_id,
const Object& target) const {
SafepointMutexLocker ml(Isolate::Current()->type_feedback_mutex());
SafepointMutexLocker ml(IsolateGroup::Current()->type_feedback_mutex());

if (LookupLocked(class_id) == Object::null()) {
InsertLocked(class_id, target);
Expand All @@ -17195,15 +17196,16 @@ void MegamorphicCache::EnsureContains(const Smi& class_id,
}

ObjectPtr MegamorphicCache::Lookup(const Smi& class_id) const {
SafepointMutexLocker ml(Isolate::Current()->type_feedback_mutex());
SafepointMutexLocker ml(IsolateGroup::Current()->type_feedback_mutex());
return LookupLocked(class_id);
}

ObjectPtr MegamorphicCache::LookupLocked(const Smi& class_id) const {
auto thread = Thread::Current();
auto isolate_group = thread->isolate_group();
auto zone = thread->zone();
ASSERT(thread->IsMutatorThread());
ASSERT(thread->isolate()->type_feedback_mutex()->IsOwnedByCurrentThread());
ASSERT(isolate_group->type_feedback_mutex()->IsOwnedByCurrentThread());

const auto& backing_array = Array::Handle(zone, buckets());
intptr_t id_mask = mask();
Expand All @@ -17225,7 +17227,7 @@ ObjectPtr MegamorphicCache::LookupLocked(const Smi& class_id) const {
void MegamorphicCache::InsertLocked(const Smi& class_id,
const Object& target) const {
auto isolate_group = IsolateGroup::Current();
ASSERT(Isolate::Current()->type_feedback_mutex()->IsOwnedByCurrentThread());
ASSERT(isolate_group->type_feedback_mutex()->IsOwnedByCurrentThread());

// As opposed to ICData we are stopping mutator threads from other isolates
// while modifying the megamorphic cache, since updates are not atomic.
Expand All @@ -17242,17 +17244,20 @@ void MegamorphicCache::InsertLocked(const Smi& class_id,
}

void MegamorphicCache::EnsureCapacityLocked() const {
ASSERT(Isolate::Current()->type_feedback_mutex()->IsOwnedByCurrentThread());
auto thread = Thread::Current();
auto zone = thread->zone();
auto isolate_group = thread->isolate_group();
ASSERT(isolate_group->type_feedback_mutex()->IsOwnedByCurrentThread());

intptr_t old_capacity = mask() + 1;
double load_limit = kLoadFactor * static_cast<double>(old_capacity);
if (static_cast<double>(filled_entry_count() + 1) > load_limit) {
const Array& old_buckets = Array::Handle(buckets());
const Array& old_buckets = Array::Handle(zone, buckets());
intptr_t new_capacity = old_capacity * 2;
const Array& new_buckets =
Array::Handle(Array::New(kEntryLength * new_capacity));
Array::Handle(zone, Array::New(kEntryLength * new_capacity));

auto& target = Object::Handle();
auto& target = Object::Handle(zone);
for (intptr_t i = 0; i < new_capacity; ++i) {
SetEntry(new_buckets, i, smi_illegal_cid(), target);
}
Expand All @@ -17261,7 +17266,7 @@ void MegamorphicCache::EnsureCapacityLocked() const {
set_filled_entry_count(0);

// Rehash the valid entries.
Smi& class_id = Smi::Handle();
Smi& class_id = Smi::Handle(zone);
for (intptr_t i = 0; i < old_capacity; ++i) {
class_id ^= GetClassId(old_buckets, i);
if (class_id.Value() != kIllegalCid) {
Expand All @@ -17274,7 +17279,9 @@ void MegamorphicCache::EnsureCapacityLocked() const {

void MegamorphicCache::InsertEntryLocked(const Smi& class_id,
const Object& target) const {
ASSERT(Isolate::Current()->type_feedback_mutex()->IsOwnedByCurrentThread());
auto thread = Thread::Current();
auto isolate_group = thread->isolate_group();
ASSERT(isolate_group->type_feedback_mutex()->IsOwnedByCurrentThread());

ASSERT(Thread::Current()->IsMutatorThread());
ASSERT(static_cast<double>(filled_entry_count() + 1) <=
Expand Down
Loading

0 comments on commit 759caec

Please sign in to comment.