[vm/shared] Snapshot initial values of shared fields.

BUG=#56016 BUG=#55991 TEST=shared_test in appjit Change-Id: I94ee12355cea95ca1c2698ee77e7ceffe81e27e1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/371944 Commit-Queue: Alexander Aprelev <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
dart-lang · Jun 20, 2024 · f3d7a74 · f3d7a74
1 parent ae3de54
commit f3d7a74
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 50 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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<AcqRelAtomic<ObjectPtr*>*>(&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<ObjectPtr*>(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<ObjectPtr*>(
+        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
@@ -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<ObjectPtr*>()),
         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
@@ -351,7 +351,9 @@ IsolateGroup::IsolateGroup(std::shared_ptr<IsolateGroupSource> 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<ObjectPtr*>(&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
@@ -747,17 +747,30 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
     initial_field_table_ = field_table;
   }
 
+  FieldTable* shared_initial_field_table() const {
+    return shared_initial_field_table_.get();
+  }
+  std::shared_ptr<FieldTable> shared_initial_field_table_shareable() {
+    return shared_initial_field_table_;
+  }
+  void set_shared_initial_field_table(std::shared_ptr<FieldTable> field_table) {
+    shared_initial_field_table_ = field_table;
+  }
+
   FieldTable* shared_field_table() const { return shared_field_table_.get(); }
   std::shared_ptr<FieldTable> shared_field_table_shareable() {
     return shared_field_table_;
   }
-  void set_shared_field_table(std::shared_ptr<FieldTable> 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<IsolateGroup> {
   intptr_t dispatch_table_snapshot_size_ = 0;
   ArrayPtr saved_unlinked_calls_;
   std::shared_ptr<FieldTable> initial_field_table_;
+  std::shared_ptr<FieldTable> shared_initial_field_table_;
   std::shared_ptr<FieldTable> shared_field_table_;
   uint32_t isolate_group_flags_ = 0;