From f3d7a7418121afb979550b4e9da01d6e6438b4da Mon Sep 17 00:00:00 2001 From: Alexander Aprelev Date: Thu, 20 Jun 2024 17:08:10 +0000 Subject: [PATCH] [vm/shared] Snapshot initial values of shared fields. BUG=https://github.com/dart-lang/sdk/issues/56016 BUG=https://github.com/dart-lang/sdk/issues/55991 TEST=shared_test in appjit Change-Id: I94ee12355cea95ca1c2698ee77e7ceffe81e27e1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/371944 Commit-Queue: Alexander Aprelev Reviewed-by: Martin Kustermann --- runtime/vm/app_snapshot.cc | 30 ++++++++++-------- runtime/vm/compiler/aot/precompiler.cc | 2 +- runtime/vm/dart.cc | 6 ++++ runtime/vm/field_table.cc | 42 ++++++++++++++++---------- runtime/vm/field_table.h | 14 ++++----- runtime/vm/isolate.cc | 37 +++++++++++++++++------ runtime/vm/isolate.h | 18 +++++++++-- 7 files changed, 99 insertions(+), 50 deletions(-) diff --git a/runtime/vm/app_snapshot.cc b/runtime/vm/app_snapshot.cc index 52b19141200..b6fd3d66821 100644 --- a/runtime/vm/app_snapshot.cc +++ b/runtime/vm/app_snapshot.cc @@ -7112,10 +7112,11 @@ class ProgramSerializationRoots : public SerializationRoots { s->Push(initial_field_table->At(i)); } - FieldTable* shared_field_table = - s->thread()->isolate_group()->shared_field_table(); - for (intptr_t i = 0, n = shared_field_table->NumFieldIds(); i < n; i++) { - s->Push(shared_field_table->At(i)); + FieldTable* shared_initial_field_table = + s->thread()->isolate_group()->shared_initial_field_table(); + for (intptr_t i = 0, n = shared_initial_field_table->NumFieldIds(); i < n; + i++) { + s->Push(shared_initial_field_table->At(i)); } dispatch_table_entries_ = object_store_->dispatch_table_code_entries(); @@ -7150,12 +7151,13 @@ class ProgramSerializationRoots : public SerializationRoots { s->WriteRootRef(initial_field_table->At(i), "some-static-field"); } - FieldTable* shared_field_table = - s->thread()->isolate_group()->shared_field_table(); - intptr_t n_shared = shared_field_table->NumFieldIds(); + FieldTable* shared_initial_field_table = + s->thread()->isolate_group()->shared_initial_field_table(); + intptr_t n_shared = shared_initial_field_table->NumFieldIds(); s->WriteUnsigned(n_shared); for (intptr_t i = 0; i < n_shared; i++) { - s->WriteRootRef(shared_field_table->At(i), "some-shared-static-field"); + s->WriteRootRef(shared_initial_field_table->At(i), + "some-shared-static-field"); } // The dispatch table is serialized only for precompiled snapshots. @@ -7212,12 +7214,14 @@ class ProgramDeserializationRoots : public DeserializationRoots { } { - FieldTable* shared_field_table = - d->thread()->isolate_group()->shared_field_table(); + FieldTable* shared_initial_field_table = + d->thread()->isolate_group()->shared_initial_field_table(); intptr_t n_shared = d->ReadUnsigned(); - shared_field_table->AllocateIndex(n_shared); - for (intptr_t i = 0; i < n_shared; i++) { - shared_field_table->SetAt(i, d->ReadRef()); + if (n_shared > 0) { + shared_initial_field_table->AllocateIndex(n_shared - 1); + for (intptr_t i = 0; i < n_shared; i++) { + shared_initial_field_table->SetAt(i, d->ReadRef()); + } } } diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc index 2ebbaa59a43..908ce7ed5b9 100644 --- a/runtime/vm/compiler/aot/precompiler.cc +++ b/runtime/vm/compiler/aot/precompiler.cc @@ -1329,7 +1329,7 @@ void Precompiler::AddField(const Field& field) { fields_to_retain_.Insert(&Field::ZoneHandle(Z, field.ptr())); if (field.is_static()) { - auto field_table = field.is_shared() ? IG->shared_field_table() + auto field_table = field.is_shared() ? IG->shared_initial_field_table() : IG->initial_field_table(); const Object& value = Object::Handle(Z, field_table->At(field.field_id())); // Should not be in the middle of initialization while precompiling. diff --git a/runtime/vm/dart.cc b/runtime/vm/dart.cc index 23407b42f03..3c4502ba353 100644 --- a/runtime/vm/dart.cc +++ b/runtime/vm/dart.cc @@ -920,6 +920,12 @@ ErrorPtr Dart::InitializeIsolateGroup(Thread* T, Object::VerifyBuiltinVtables(); auto IG = T->isolate_group(); + { + SafepointReadRwLocker reader(T, IG->program_lock()); + IG->set_shared_field_table(T, IG->shared_initial_field_table()->Clone( + /*for_isolate=*/nullptr, + /*for_isolate_group=*/IG)); + } DEBUG_ONLY(IG->heap()->Verify("InitializeIsolate", kForbidMarked)); #if !defined(DART_PRECOMPILED_RUNTIME) diff --git a/runtime/vm/field_table.cc b/runtime/vm/field_table.cc index 25b9449a343..b15dc84bf4e 100644 --- a/runtime/vm/field_table.cc +++ b/runtime/vm/field_table.cc @@ -55,7 +55,6 @@ intptr_t FieldTable::FieldOffsetFor(intptr_t field_id) { bool FieldTable::Register(const Field& field, intptr_t expected_field_id) { DEBUG_ASSERT( IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); - ASSERT(is_shared_ == field.is_shared()); ASSERT(is_ready_to_use_); if (free_head_ < 0) { @@ -119,28 +118,39 @@ void FieldTable::Grow(intptr_t new_capacity) { // Ensure that new_table_ is populated before it is published // via store to table_. reinterpret_cast*>(&table_)->store(new_table); - if (isolate_ != nullptr && isolate_->mutator_thread() != nullptr) { - if (is_shared_) { - isolate_->mutator_thread()->shared_field_table_values_ = table_; - } else { - isolate_->mutator_thread()->field_table_values_ = table_; - } + if (isolate_group_ != nullptr) { + isolate_group_->ForEachIsolate( + [&](Isolate* isolate) { + if (isolate->mutator_thread() != nullptr) { + isolate->mutator_thread()->shared_field_table_values_ = table_; + } + }, + /*at_safepoint=*/false); + } else if (isolate_ != nullptr && isolate_->mutator_thread() != nullptr) { + isolate_->mutator_thread()->field_table_values_ = table_; } } -FieldTable* FieldTable::Clone(Isolate* for_isolate) { +FieldTable* FieldTable::Clone(Isolate* for_isolate, + IsolateGroup* for_isolate_group) { DEBUG_ASSERT( IsolateGroup::Current()->program_lock()->IsCurrentThreadReader()); - FieldTable* clone = new FieldTable(for_isolate); - auto new_table = - static_cast(malloc(capacity_ * sizeof(ObjectPtr))); // NOLINT - memmove(new_table, table_, capacity_ * sizeof(ObjectPtr)); + FieldTable* clone = new FieldTable(for_isolate, for_isolate_group); ASSERT(clone->table_ == nullptr); - clone->table_ = new_table; - clone->capacity_ = capacity_; - clone->top_ = top_; - clone->free_head_ = free_head_; + if (table_ == nullptr) { + ASSERT(capacity_ == 0); + ASSERT(top_ == 0); + ASSERT(free_head_ == -1); + } else { + auto new_table = static_cast( + malloc(capacity_ * sizeof(ObjectPtr))); // NOLINT + memmove(new_table, table_, capacity_ * sizeof(ObjectPtr)); + clone->table_ = new_table; + clone->capacity_ = capacity_; + clone->top_ = top_; + clone->free_head_ = free_head_; + } return clone; } diff --git a/runtime/vm/field_table.h b/runtime/vm/field_table.h index 4013ec69a54..2414038711b 100644 --- a/runtime/vm/field_table.h +++ b/runtime/vm/field_table.h @@ -22,15 +22,15 @@ class FieldInvalidator; class FieldTable { public: - explicit FieldTable(Isolate* isolate, bool is_shared = false) + explicit FieldTable(Isolate* isolate, IsolateGroup* isolate_group = nullptr) : top_(0), capacity_(0), free_head_(-1), table_(nullptr), old_tables_(new MallocGrowableArray()), isolate_(isolate), - is_ready_to_use_(isolate == nullptr), - is_shared_(is_shared) {} + isolate_group_(isolate_group), + is_ready_to_use_(isolate == nullptr) {} ~FieldTable(); @@ -88,7 +88,8 @@ class FieldTable { } } - FieldTable* Clone(Isolate* for_isolate); + FieldTable* Clone(Isolate* for_isolate, + IsolateGroup* for_isolate_group = nullptr); void VisitObjectPointers(ObjectPointerVisitor* visitor); @@ -114,15 +115,12 @@ class FieldTable { // Growing the field table will keep the cached field table on the isolate's // mutator thread up-to-date. Isolate* isolate_; + IsolateGroup* isolate_group_; // Whether this field table is ready to use by e.g. registering new static // fields. bool is_ready_to_use_ = false; - // Is this the shared field table? Need to know what is the isolate's property - // that have to be updated. - bool is_shared_ = false; - DISALLOW_COPY_AND_ASSIGN(FieldTable); }; diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index 3267678410e..0d428ab77e3 100644 --- a/runtime/vm/isolate.cc +++ b/runtime/vm/isolate.cc @@ -351,7 +351,9 @@ IsolateGroup::IsolateGroup(std::shared_ptr source, heap_(nullptr), saved_unlinked_calls_(Array::null()), initial_field_table_(new FieldTable(/*isolate=*/nullptr)), - shared_field_table_(new FieldTable(/*isolate=*/nullptr, /*shared=*/true)), + shared_initial_field_table_(new FieldTable(/*isolate=*/nullptr, + /*isolate_group=*/nullptr)), + shared_field_table_(new FieldTable(/*isolate=*/nullptr, this)), #if !defined(DART_PRECOMPILED_RUNTIME) background_compiler_(new BackgroundCompiler(this)), #endif @@ -784,21 +786,35 @@ void IsolateGroup::ValidateClassTable() { } #endif // DEBUG +void IsolateGroup::RegisterSharedStaticField(const Field& field, + const Object& initial_value) { + const bool need_to_grow_backing_store = + shared_initial_field_table()->Register(field); + const intptr_t field_id = field.field_id(); + shared_initial_field_table()->SetAt(field_id, initial_value.ptr()); + + if (need_to_grow_backing_store) { + // We have to stop other isolates from accessing shared isolate group + // field state, since we'll have to grow the backing store. + GcSafepointOperationScope scope(Thread::Current()); + const bool need_to_grow_other_backing_store = + shared_field_table()->Register(field, field_id); + ASSERT(need_to_grow_other_backing_store); + } else { + const bool need_to_grow_other_backing_store = + shared_field_table()->Register(field, field_id); + ASSERT(!need_to_grow_other_backing_store); + } + shared_field_table()->SetAt(field_id, initial_value.ptr()); +} + void IsolateGroup::RegisterStaticField(const Field& field, const Object& initial_value) { ASSERT(program_lock()->IsCurrentThreadWriter()); ASSERT(field.is_static()); if (field.is_shared()) { - GcSafepointOperationScope scope(Thread::Current()); - if (shared_field_table()->Register(field)) { - for (auto isolate : isolates_) { - isolate->mutator_thread()->shared_field_table_values_ = - shared_field_table()->table(); - } - } - const intptr_t field_id = field.field_id(); - shared_field_table()->SetAt(field_id, initial_value.ptr()); + RegisterSharedStaticField(field, initial_value); return; } const bool need_to_grow_backing_store = @@ -2915,6 +2931,7 @@ void IsolateGroup::VisitSharedPointers(ObjectPointerVisitor* visitor) { } visitor->VisitPointer(reinterpret_cast(&saved_unlinked_calls_)); initial_field_table()->VisitObjectPointers(visitor); + shared_initial_field_table()->VisitObjectPointers(visitor); shared_field_table()->VisitObjectPointers(visitor); // Visit the boxed_field_list_. diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h index 5c4280744f8..2fe03397de2 100644 --- a/runtime/vm/isolate.h +++ b/runtime/vm/isolate.h @@ -747,17 +747,30 @@ class IsolateGroup : public IntrusiveDListEntry { initial_field_table_ = field_table; } + FieldTable* shared_initial_field_table() const { + return shared_initial_field_table_.get(); + } + std::shared_ptr shared_initial_field_table_shareable() { + return shared_initial_field_table_; + } + void set_shared_initial_field_table(std::shared_ptr field_table) { + shared_initial_field_table_ = field_table; + } + FieldTable* shared_field_table() const { return shared_field_table_.get(); } std::shared_ptr shared_field_table_shareable() { return shared_field_table_; } - void set_shared_field_table(std::shared_ptr field_table) { - shared_field_table_ = field_table; + void set_shared_field_table(Thread* T, FieldTable* shared_field_table) { + shared_field_table_.reset(shared_field_table); + T->shared_field_table_values_ = shared_field_table->table(); } MutatorThreadPool* thread_pool() { return thread_pool_.get(); } void RegisterClass(const Class& cls); + void RegisterSharedStaticField(const Field& field, + const Object& initial_value); void RegisterStaticField(const Field& field, const Object& initial_value); void FreeStaticField(const Field& field); @@ -872,6 +885,7 @@ class IsolateGroup : public IntrusiveDListEntry { intptr_t dispatch_table_snapshot_size_ = 0; ArrayPtr saved_unlinked_calls_; std::shared_ptr initial_field_table_; + std::shared_ptr shared_initial_field_table_; std::shared_ptr shared_field_table_; uint32_t isolate_group_flags_ = 0;