diff --git a/runtime/vm/heap/heap.cc b/runtime/vm/heap/heap.cc index 2e9d7f44f85..33487fbd093 100644 --- a/runtime/vm/heap/heap.cc +++ b/runtime/vm/heap/heap.cc @@ -885,10 +885,10 @@ void Heap::ForwardWeakEntries(RawObject* before_object, for (int sel = 0; sel < Heap::kNumWeakSelectors; sel++) { const auto selector = static_cast(sel); auto before_table = GetWeakTable(before_space, selector); - intptr_t entry = before_table->RemoveValue(before_object); + intptr_t entry = before_table->RemoveValueExclusive(before_object); if (entry != 0) { auto after_table = GetWeakTable(after_space, selector); - after_table->SetValue(after_object, entry); + after_table->SetValueExclusive(after_object, entry); } } diff --git a/runtime/vm/heap/marker.cc b/runtime/vm/heap/marker.cc index 12056834e06..5558aa8a1cc 100644 --- a/runtime/vm/heap/marker.cc +++ b/runtime/vm/heap/marker.cc @@ -501,11 +501,11 @@ void GCMarker::ProcessWeakTables(PageSpace* page_space) { heap_->GetWeakTable(Heap::kOld, static_cast(sel)); intptr_t size = table->size(); for (intptr_t i = 0; i < size; i++) { - if (table->IsValidEntryAt(i)) { - RawObject* raw_obj = table->ObjectAt(i); + if (table->IsValidEntryAtExclusive(i)) { + RawObject* raw_obj = table->ObjectAtExclusive(i); ASSERT(raw_obj->IsHeapObject()); if (!raw_obj->IsMarked()) { - table->InvalidateAt(i); + table->InvalidateAtExclusive(i); } } } diff --git a/runtime/vm/heap/scavenger.cc b/runtime/vm/heap/scavenger.cc index 497b6ff25e3..b92a8702e4c 100644 --- a/runtime/vm/heap/scavenger.cc +++ b/runtime/vm/heap/scavenger.cc @@ -851,8 +851,8 @@ void Scavenger::ProcessWeakReferences() { WeakTable* replacement_old) { intptr_t size = table->size(); for (intptr_t i = 0; i < size; i++) { - if (table->IsValidEntryAt(i)) { - RawObject* raw_obj = table->ObjectAt(i); + if (table->IsValidEntryAtExclusive(i)) { + RawObject* raw_obj = table->ObjectAtExclusive(i); ASSERT(raw_obj->IsHeapObject()); uword raw_addr = RawObject::ToAddr(raw_obj); uword header = *reinterpret_cast(raw_addr); @@ -862,7 +862,7 @@ void Scavenger::ProcessWeakReferences() { raw_obj = RawObject::FromAddr(new_addr); auto replacement = raw_obj->IsNewObject() ? replacement_new : replacement_old; - replacement->SetValue(raw_obj, table->ValueAt(i)); + replacement->SetValueExclusive(raw_obj, table->ValueAtExclusive(i)); } } } diff --git a/runtime/vm/heap/weak_table.cc b/runtime/vm/heap/weak_table.cc index 4fd7917bea5..95acc864fbc 100644 --- a/runtime/vm/heap/weak_table.cc +++ b/runtime/vm/heap/weak_table.cc @@ -29,11 +29,11 @@ intptr_t WeakTable::SizeFor(intptr_t count, intptr_t size) { return result; } -void WeakTable::SetValue(RawObject* key, intptr_t val) { +void WeakTable::SetValueExclusive(RawObject* key, intptr_t val) { intptr_t mask = size() - 1; intptr_t idx = Hash(key) & mask; intptr_t empty_idx = -1; - RawObject* obj = ObjectAt(idx); + RawObject* obj = ObjectAtExclusive(idx); while (obj != NULL) { if (obj == key) { @@ -44,7 +44,7 @@ void WeakTable::SetValue(RawObject* key, intptr_t val) { empty_idx = idx; // Insert at this location if not found. } idx = (idx + 1) & mask; - obj = ObjectAt(idx); + obj = ObjectAtExclusive(idx); } if (val == 0) { @@ -60,7 +60,7 @@ void WeakTable::SetValue(RawObject* key, intptr_t val) { idx = empty_idx; } - ASSERT(!IsValidEntryAt(idx)); + ASSERT(!IsValidEntryAtExclusive(idx)); // Set the key and value. SetObjectAt(idx, key); SetValueAt(idx, val); @@ -87,7 +87,7 @@ void WeakTable::Forward(ObjectPointerVisitor* visitor) { if (used_ == 0) return; for (intptr_t i = 0; i < size_; i++) { - if (IsValidEntryAt(i)) { + if (IsValidEntryAtExclusive(i)) { visitor->VisitPointer(ObjectPointerAt(i)); } } @@ -107,9 +107,9 @@ void WeakTable::Rehash() { intptr_t mask = new_size - 1; set_used(0); for (intptr_t i = 0; i < old_size; i++) { - if (IsValidEntryAt(i)) { + if (IsValidEntryAtExclusive(i)) { // Find the new hash location for this entry. - RawObject* key = ObjectAt(i); + RawObject* key = ObjectAtExclusive(i); intptr_t idx = Hash(key) & mask; RawObject* obj = reinterpret_cast(new_data[ObjectIndex(idx)]); while (obj != NULL) { @@ -119,7 +119,7 @@ void WeakTable::Rehash() { } new_data[ObjectIndex(idx)] = reinterpret_cast(key); - new_data[ValueIndex(idx)] = ValueAt(i); + new_data[ValueIndex(idx)] = ValueAtExclusive(i); set_used(used() + 1); } } diff --git a/runtime/vm/heap/weak_table.h b/runtime/vm/heap/weak_table.h index c7970cfa2c0..c1487b1d037 100644 --- a/runtime/vm/heap/weak_table.h +++ b/runtime/vm/heap/weak_table.h @@ -8,6 +8,7 @@ #include "vm/globals.h" #include "platform/assert.h" +#include "vm/lockers.h" #include "vm/raw_object.h" namespace dart { @@ -46,64 +47,82 @@ class WeakTable { intptr_t used() const { return used_; } intptr_t count() const { return count_; } - bool IsValidEntryAt(intptr_t i) const { - ASSERT(((ValueAt(i) == 0) && ((ObjectAt(i) == NULL) || - (data_[ObjectIndex(i)] == kDeletedEntry))) || - ((ValueAt(i) != 0) && (ObjectAt(i) != NULL) && - (data_[ObjectIndex(i)] != kDeletedEntry))); + // The following methods can be called concurrently and are guarded by a lock. + + intptr_t GetValue(RawObject* key) { + MutexLocker ml(&mutex_); + return GetValueExclusive(key); + } + + void SetValue(RawObject* key, intptr_t val) { + MutexLocker ml(&mutex_); + return SetValueExclusive(key, val); + } + + // The following "exclusive" methods must only be called from call sites + // which are known to have exclusive access to the weak table. + // + // This is mostly limited to GC related code (e.g. scavenger, marker, ...) + + bool IsValidEntryAtExclusive(intptr_t i) const { + ASSERT((ValueAtExclusive(i) == 0 && + (ObjectAtExclusive(i) == NULL || + data_[ObjectIndex(i)] == kDeletedEntry)) || + (ValueAtExclusive(i) != 0 && ObjectAtExclusive(i) != NULL && + data_[ObjectIndex(i)] != kDeletedEntry)); return (data_[ValueIndex(i)] != 0); } - void InvalidateAt(intptr_t i) { - ASSERT(IsValidEntryAt(i)); + void InvalidateAtExclusive(intptr_t i) { + ASSERT(IsValidEntryAtExclusive(i)); SetValueAt(i, 0); } - RawObject* ObjectAt(intptr_t i) const { + RawObject* ObjectAtExclusive(intptr_t i) const { ASSERT(i >= 0); ASSERT(i < size()); return reinterpret_cast(data_[ObjectIndex(i)]); } - intptr_t ValueAt(intptr_t i) const { + intptr_t ValueAtExclusive(intptr_t i) const { ASSERT(i >= 0); ASSERT(i < size()); return data_[ValueIndex(i)]; } - void SetValue(RawObject* key, intptr_t val); + void SetValueExclusive(RawObject* key, intptr_t val); - intptr_t GetValue(RawObject* key) const { + intptr_t GetValueExclusive(RawObject* key) const { intptr_t mask = size() - 1; intptr_t idx = Hash(key) & mask; - RawObject* obj = ObjectAt(idx); + RawObject* obj = ObjectAtExclusive(idx); while (obj != NULL) { if (obj == key) { - return ValueAt(idx); + return ValueAtExclusive(idx); } idx = (idx + 1) & mask; - obj = ObjectAt(idx); + obj = ObjectAtExclusive(idx); } - ASSERT(ValueAt(idx) == 0); + ASSERT(ValueAtExclusive(idx) == 0); return 0; } // Removes and returns the value associated with |key|. Returns 0 if there is // no value associated with |key|. - intptr_t RemoveValue(RawObject* key) { + intptr_t RemoveValueExclusive(RawObject* key) { intptr_t mask = size() - 1; intptr_t idx = Hash(key) & mask; - RawObject* obj = ObjectAt(idx); + RawObject* obj = ObjectAtExclusive(idx); while (obj != NULL) { if (obj == key) { - intptr_t result = ValueAt(idx); - InvalidateAt(idx); + intptr_t result = ValueAtExclusive(idx); + InvalidateAtExclusive(idx); return result; } idx = (idx + 1) & mask; - obj = ObjectAt(idx); + obj = ObjectAtExclusive(idx); } - ASSERT(ValueAt(idx) == 0); + ASSERT(ValueAtExclusive(idx) == 0); return 0; } @@ -174,6 +193,8 @@ class WeakTable { return reinterpret_cast(key) * 92821; } + Mutex mutex_; + // data_ contains size_ tuples of key/value. intptr_t* data_; // size_ keeps the number of entries in data_. used_ maintains the number of