[vm] Update Megamorphic::filled_entry_count_ under the megamorphic lock.
Otherwise the background compiler may see 0 when the mutator grows the megamorphic cache.
Bug: https://github.com/dart-lang/sdk/issues/37257
Change-Id: I64a31937391ad6c0f086f8f175501ca4ef06c305
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105969
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc
index 932b94e..17e1b33 100644
--- a/runtime/vm/compiler/backend/il.cc
+++ b/runtime/vm/compiler/backend/il.cc
@@ -776,7 +776,7 @@
if (ic_data.is_megamorphic()) {
const MegamorphicCache& cache =
MegamorphicCache::Handle(zone, ic_data.AsMegamorphicCache());
- SafepointMutexLocker ml(Isolate::Current()->megamorphic_lookup_mutex());
+ SafepointMutexLocker ml(Isolate::Current()->megamorphic_mutex());
MegamorphicCacheEntries entries(Array::Handle(zone, cache.buckets()));
for (intptr_t i = 0; i < entries.Length(); i++) {
const intptr_t id =
@@ -787,8 +787,10 @@
if (include_targets) {
Function& function = Function::ZoneHandle(zone);
function ^= entries[i].Get<MegamorphicCache::kTargetFunctionIndex>();
+ const intptr_t filled_entry_count = cache.filled_entry_count();
+ ASSERT(filled_entry_count > 0);
cid_ranges_.Add(new (zone) TargetInfo(
- id, id, &function, Usage(function) / cache.filled_entry_count(),
+ id, id, &function, Usage(function) / filled_entry_count,
StaticTypeExactnessState::NotTracking()));
} else {
cid_ranges_.Add(new (zone) CidRange(id, id));
diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index e68d40c..544424a 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -892,8 +892,7 @@
NOT_IN_PRODUCT("Isolate::type_canonicalization_mutex_")),
constant_canonicalization_mutex_(
NOT_IN_PRODUCT("Isolate::constant_canonicalization_mutex_")),
- megamorphic_lookup_mutex_(
- NOT_IN_PRODUCT("Isolate::megamorphic_lookup_mutex_")),
+ megamorphic_mutex_(NOT_IN_PRODUCT("Isolate::megamorphic_mutex_")),
kernel_data_lib_cache_mutex_(
NOT_IN_PRODUCT("Isolate::kernel_data_lib_cache_mutex_")),
kernel_data_class_cache_mutex_(
diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h
index 4813d27..6e8cb9e 100644
--- a/runtime/vm/isolate.h
+++ b/runtime/vm/isolate.h
@@ -355,7 +355,7 @@
Mutex* constant_canonicalization_mutex() {
return &constant_canonicalization_mutex_;
}
- Mutex* megamorphic_lookup_mutex() { return &megamorphic_lookup_mutex_; }
+ Mutex* megamorphic_mutex() { return &megamorphic_mutex_; }
Mutex* kernel_data_lib_cache_mutex() { return &kernel_data_lib_cache_mutex_; }
Mutex* kernel_data_class_cache_mutex() {
@@ -1027,7 +1027,8 @@
Mutex symbols_mutex_; // Protects concurrent access to the symbol table.
Mutex type_canonicalization_mutex_; // Protects type canonicalization.
Mutex constant_canonicalization_mutex_; // Protects const canonicalization.
- Mutex megamorphic_lookup_mutex_; // Protects megamorphic table lookup.
+ Mutex megamorphic_mutex_; // Protects the table of megamorphic caches and
+ // their entries.
Mutex kernel_data_lib_cache_mutex_;
Mutex kernel_data_class_cache_mutex_;
Mutex kernel_constants_mutex_;
diff --git a/runtime/vm/megamorphic_cache_table.cc b/runtime/vm/megamorphic_cache_table.cc
index c07452f..8b9bf6a 100644
--- a/runtime/vm/megamorphic_cache_table.cc
+++ b/runtime/vm/megamorphic_cache_table.cc
@@ -16,7 +16,7 @@
const String& name,
const Array& descriptor) {
// Multiple compilation threads could access this lookup.
- SafepointMutexLocker ml(isolate->megamorphic_lookup_mutex());
+ SafepointMutexLocker ml(isolate->megamorphic_mutex());
ASSERT(name.IsSymbol());
// TODO(rmacnak): ASSERT(descriptor.IsCanonical());
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index ffea8e7..4294af9 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -14175,7 +14175,7 @@
if (is_megamorphic()) {
const MegamorphicCache& cache =
MegamorphicCache::Handle(AsMegamorphicCache());
- SafepointMutexLocker ml(Isolate::Current()->megamorphic_lookup_mutex());
+ SafepointMutexLocker ml(Isolate::Current()->megamorphic_mutex());
MegamorphicCacheEntries entries(Array::Handle(cache.buckets()));
for (intptr_t i = 0; i < entries.Length(); i++) {
const intptr_t id =
@@ -15672,7 +15672,14 @@
return result.raw();
}
-void MegamorphicCache::EnsureCapacity() const {
+void MegamorphicCache::Insert(const Smi& class_id, const Object& target) const {
+ SafepointMutexLocker ml(Isolate::Current()->megamorphic_mutex());
+ EnsureCapacityLocked();
+ InsertLocked(class_id, target);
+}
+
+void MegamorphicCache::EnsureCapacityLocked() const {
+ ASSERT(Isolate::Current()->megamorphic_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) {
@@ -15696,16 +15703,18 @@
class_id ^= GetClassId(old_buckets, i);
if (class_id.Value() != kIllegalCid) {
target = GetTargetFunction(old_buckets, i);
- Insert(class_id, target);
+ InsertLocked(class_id, target);
}
}
}
}
-void MegamorphicCache::Insert(const Smi& class_id, const Object& target) const {
+void MegamorphicCache::InsertLocked(const Smi& class_id,
+ const Object& target) const {
+ ASSERT(Isolate::Current()->megamorphic_mutex()->IsOwnedByCurrentThread());
+ ASSERT(Thread::Current()->IsMutatorThread());
ASSERT(static_cast<double>(filled_entry_count() + 1) <=
(kLoadFactor * static_cast<double>(mask() + 1)));
- SafepointMutexLocker ml(Isolate::Current()->megamorphic_lookup_mutex());
const Array& backing_array = Array::Handle(buckets());
intptr_t id_mask = mask();
intptr_t index = (class_id.Value() * kSpreadFactor) & id_mask;
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
index c1cc1a4..a5a5731 100644
--- a/runtime/vm/object.h
+++ b/runtime/vm/object.h
@@ -5836,8 +5836,6 @@
static RawMegamorphicCache* New(const String& target_name,
const Array& arguments_descriptor);
- void EnsureCapacity() const;
-
void Insert(const Smi& class_id, const Object& target) const;
void SwitchToBareInstructions();
@@ -5856,6 +5854,10 @@
void set_target_name(const String& value) const;
void set_arguments_descriptor(const Array& value) const;
+ // The caller must hold Isolate::megamorphic_mutex().
+ void EnsureCapacityLocked() const;
+ void InsertLocked(const Smi& class_id, const Object& target) const;
+
static inline void SetEntry(const Array& array,
intptr_t index,
const Smi& class_id,
diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc
index c2257d0..f13950a 100644
--- a/runtime/vm/runtime_entry.cc
+++ b/runtime/vm/runtime_entry.cc
@@ -1786,7 +1786,6 @@
} else {
const MegamorphicCache& cache = MegamorphicCache::Cast(ic_data_or_cache);
// Insert function found into cache and return it.
- cache.EnsureCapacity();
const Smi& class_id = Smi::Handle(zone, Smi::New(cls.id()));
cache.Insert(class_id, target_function);
}
