Skip to content

Commit

Permalink
[vm/concurrency] Lock weak table to prepare for concurrent access.
Browse files Browse the repository at this point in the history 
Right now there is only one mutator working on a heap and it can
therefore access the heap's weak maps without locking. The GC is the
only other user of the weak maps and it accesses it within a safepoint
operation scope.

Once we move the heap to the isolate group there can be concurrent
accesses to the weak maps. As a preparation step this CL adds locking
around the non-GC API.

Issue dart-lang/sdk#36097

Change-Id: I84acce24612b12a7393154cab816f0eff9c7589a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116201
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
  • Loading branch information
@mkustermann
mkustermann authored and commit-bot@chromium.org committed Sep 10, 2019
1 parent 8329968 commit 6717bfe
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 37 deletions.
4 changes: 2 additions & 2 deletions runtime/vm/heap/heap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -885,10 +885,10 @@ void Heap::ForwardWeakEntries(RawObject* before_object,
for (int sel = 0; sel < Heap::kNumWeakSelectors; sel++) {
const auto selector = static_cast<Heap::WeakSelector>(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);
}
}

Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/heap/marker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,11 +501,11 @@ void GCMarker::ProcessWeakTables(PageSpace* page_space) {
heap_->GetWeakTable(Heap::kOld, static_cast<Heap::WeakSelector>(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);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/heap/scavenger.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<uword*>(raw_addr);
Expand All @@ -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));
}
}
}
Expand Down
16 changes: 8 additions & 8 deletions runtime/vm/heap/weak_table.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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);
Expand All @@ -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));
}
}
Expand All @@ -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<RawObject*>(new_data[ObjectIndex(idx)]);
while (obj != NULL) {
Expand All @@ -119,7 +119,7 @@ void WeakTable::Rehash() {
}

new_data[ObjectIndex(idx)] = reinterpret_cast<intptr_t>(key);
new_data[ValueIndex(idx)] = ValueAt(i);
new_data[ValueIndex(idx)] = ValueAtExclusive(i);
set_used(used() + 1);
}
}
Expand Down
63 changes: 42 additions & 21 deletions runtime/vm/heap/weak_table.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "vm/globals.h"

#include "platform/assert.h"
#include "vm/lockers.h"
#include "vm/raw_object.h"

namespace dart {
Expand Down Expand Up @@ -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<RawObject*>(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;
}

Expand Down Expand Up @@ -174,6 +193,8 @@ class WeakTable {
return reinterpret_cast<uintptr_t>(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
Expand Down

0 comments on commit 6717bfe

Please sign in to comment.