[vm] Cleanup reload debugging code.

rmacnak-google · Commit Queue · commit 0ab55451f4ec · 2024-10-07T22:29:49.000Z
TEST=ci Change-Id: I840fc1a614186e0f330f4b2313a6706452af6998 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/388665 Reviewed-by: Alexander Aprelev <aam@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
diff --git a/runtime/vm/heap/become.cc b/runtime/vm/heap/become.cc
@@ -228,12 +228,9 @@ Become::~Become() {
   Thread::Current()->isolate_group()->set_become(nullptr);
 }
 
-void Become::Add(const Object& before,
-                 const Object& after,
-                 const char* whence) {
+void Become::Add(const Object& before, const Object& after) {
   pointers_.Add(before.ptr());
   pointers_.Add(after.ptr());
-  whence_.Add(whence);
 }
 
 void Become::VisitObjectPointers(ObjectPointerVisitor* visitor) {
@@ -296,56 +293,6 @@ void Become::Forward() {
     return;
   }
 
-  {
-    intptr_t conflicts = 0;
-    MallocDirectChainedHashMap<PtrIntTrait> map(pointers_.length() / 2);
-    for (intptr_t i = 0; i < pointers_.length(); i += 2) {
-      ObjectPtr before = pointers_[i];
-      auto* pair = map.Lookup(before);
-      if (pair == nullptr) {
-        map.Insert({before, i + 1});  // 0 value is sentinel.
-      } else {
-        intptr_t j = pair->value - 1;  // 0 value is sentinel.
-        ObjectPtr before1 = pointers_[j];
-        ObjectPtr after1 = pointers_[j + 1];
-        ObjectPtr before2 = pointers_[i];
-        ObjectPtr after2 = pointers_[i + 1];
-        const char* whence1 = whence_[j / 2];
-        const char* whence2 = whence_[i / 2];
-
-        OS::PrintErr("become conflict:\n");
-        OS::PrintErr("whence1 = %s\n", whence1);
-        OS::PrintErr("whence2 = %s\n", whence2);
-
-        OS::PrintErr("before1 cid = %" Pd "\n", before1.GetClassId());
-        OS::PrintErr(" after1 cid = %" Pd "\n", after1.GetClassId());
-        OS::PrintErr("before2 cid = %" Pd "\n", before2.GetClassId());
-        OS::PrintErr(" after2 cid = %" Pd "\n", after2.GetClassId());
-
-        OS::PrintErr("before1 size = %" Pd "\n", before1.untag()->HeapSize());
-        OS::PrintErr(" after1 size = %" Pd "\n", after1.untag()->HeapSize());
-        OS::PrintErr("before2 size = %" Pd "\n", before2.untag()->HeapSize());
-        OS::PrintErr(" after2 size = %" Pd "\n", after2.untag()->HeapSize());
-
-#ifndef PRODUCT
-        ClassTable* class_table = IsolateGroup::Current()->class_table();
-        OS::PrintErr("before1 cls = %s\n",
-                     class_table->UserVisibleNameFor(before1.GetClassId()));
-        OS::PrintErr(" after1 cls = %s\n",
-                     class_table->UserVisibleNameFor(after1.GetClassId()));
-        OS::PrintErr("before2 cls = %s\n",
-                     class_table->UserVisibleNameFor(before2.GetClassId()));
-        OS::PrintErr(" after2 cls = %s\n",
-                     class_table->UserVisibleNameFor(after2.GetClassId()));
-#endif
-        conflicts++;
-      }
-    }
-    if (conflicts != 0) {
-      FATAL("%" Pd " become conflicts", conflicts);
-    }
-  }
-
   Thread* thread = Thread::Current();
   auto heap = thread->isolate_group()->heap();
 
@@ -394,7 +341,6 @@ void Become::Forward() {
   }
 #endif
   pointers_.Clear();
-  whence_.Clear();
 }
 
 void Become::FollowForwardingPointers(Thread* thread) {
diff --git a/runtime/vm/heap/become.h b/runtime/vm/heap/become.h
@@ -98,7 +98,8 @@ class Become {
   Become();
   ~Become();
 
-  void Add(const Object& before, const Object& after, const char* whence);
+  void Add(const Object& before, const Object& after);
+
   void Forward();
   void Exchange() { UNIMPLEMENTED(); }
 
@@ -115,7 +116,6 @@ class Become {
 
  private:
   MallocGrowableArray<ObjectPtr> pointers_;
-  MallocGrowableArray<const char*> whence_;
   DISALLOW_COPY_AND_ASSIGN(Become);
 };
 
diff --git a/runtime/vm/heap/become_test.cc b/runtime/vm/heap/become_test.cc
@@ -22,7 +22,7 @@ void TestBecomeForward(Heap::Space before_space, Heap::Space after_space) {
   EXPECT(before_obj.ptr() != after_obj.ptr());
 
   Become become;
-  become.Add(before_obj, after_obj, "test");
+  become.Add(before_obj, after_obj);
   become.Forward();
 
   EXPECT(before_obj.ptr() == after_obj.ptr());
@@ -64,7 +64,7 @@ ISOLATE_UNIT_TEST_CASE(BecomeForwardPeer) {
   EXPECT_EQ(no_peer, heap->GetPeer(after_obj.ptr()));
 
   Become become;
-  become.Add(before_obj, after_obj, "test");
+  become.Add(before_obj, after_obj);
   become.Forward();
 
   EXPECT(before_obj.ptr() == after_obj.ptr());
@@ -86,7 +86,7 @@ ISOLATE_UNIT_TEST_CASE(BecomeForwardObjectId) {
   EXPECT_EQ(no_id, heap->GetObjectId(after_obj.ptr()));
 
   Become become;
-  become.Add(before_obj, after_obj, "test");
+  become.Add(before_obj, after_obj);
   become.Forward();
 
   EXPECT(before_obj.ptr() == after_obj.ptr());
@@ -112,7 +112,7 @@ ISOLATE_UNIT_TEST_CASE(BecomeForwardMessageId) {
             isolate->forward_table_old()->GetValueExclusive(after_obj.ptr()));
 
   Become become;
-  become.Add(before_obj, after_obj, "test");
+  become.Add(before_obj, after_obj);
   become.Forward();
 
   EXPECT(before_obj.ptr() == after_obj.ptr());
@@ -138,7 +138,7 @@ ISOLATE_UNIT_TEST_CASE(BecomeForwardRememberedObject) {
   EXPECT(before_obj.ptr() != after_obj.ptr());
 
   Become become;
-  become.Add(before_obj, after_obj, "test");
+  become.Add(before_obj, after_obj);
   become.Forward();
 
   EXPECT(before_obj.ptr() == after_obj.ptr());
@@ -167,7 +167,7 @@ ISOLATE_UNIT_TEST_CASE(BecomeForwardRememberedCards) {
   }
 
   Become become;
-  become.Add(old_element, new_element, "test");
+  become.Add(old_element, new_element);
   become.Forward();
 
   EXPECT(old_element.ptr() == new_element.ptr());
diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
@@ -992,7 +992,7 @@ void IsolateGroup::RehashConstants(Become* become) {
         }
 
         if (old_value.ptr() != new_value.ptr()) {
-          become->Add(old_value, new_value, "enum forwarding");
+          become->Add(old_value, new_value);
         }
         if (new_value.IsCanonical()) {
           cls.InsertCanonicalConstant(zone, new_value);
@@ -1013,7 +1013,7 @@ void IsolateGroup::RehashConstants(Become* become) {
         } else {
           new_value = old_value.Canonicalize(thread);
           if (old_value.ptr() != new_value.ptr()) {
-            become->Add(old_value, new_value, "constant forwarding");
+            become->Add(old_value, new_value);
           }
         }
       }
diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc
@@ -357,9 +357,7 @@ void InstanceMorpher::CreateMorphedCopies(Become* become) {
     // instances of any class with the old size.
     Become::MakeDummyObject(before);
 
-    become->Add(
-        before, after,
-        is_canonical ? "instance morphing canonical" : "instance morphing");
+    become->Add(before, after);
   }
 }
 
@@ -1281,7 +1279,7 @@ void ProgramReloadContext::RegisterClass(const Class& new_cls) {
   IG->class_table()->SetAt(old_cls.id(), new_cls.ptr());
   new_cls.CopyCanonicalConstants(old_cls);
   new_cls.CopyDeclarationType(old_cls);
-  AddBecomeMapping(old_cls, new_cls, "class forwarding");
+  AddBecomeMapping(old_cls, new_cls);
   AddClassMapping(new_cls, old_cls);
 }
 
@@ -1627,6 +1625,7 @@ void ProgramReloadContext::RollbackLibraries() {
 }
 
 void ProgramReloadContext::VerifyMaps() {
+#if defined(DEBUG)
   TIMELINE_SCOPE(VerifyMaps);
 
   // Verify that two old classes aren't both mapped to the same new
@@ -1686,6 +1685,7 @@ void ProgramReloadContext::VerifyMaps() {
   }
   library_map.Release();
   reverse_library_map.Release();
+#endif  // defined(DEBUG)
 }
 
 void ProgramReloadContext::CommitBeforeInstanceMorphing() {
@@ -2589,7 +2589,7 @@ void ProgramReloadContext::BuildLibraryMapping() {
       // Replaced class.
       AddLibraryMapping(replacement_or_new, old);
 
-      AddBecomeMapping(old, replacement_or_new, "library forwarding");
+      AddBecomeMapping(old, replacement_or_new);
     }
   }
 }
@@ -2698,13 +2698,12 @@ void ProgramReloadContext::AddStaticFieldMapping(const Field& old_field,
                                                  const Field& new_field) {
   ASSERT(old_field.is_static());
   ASSERT(new_field.is_static());
-  AddBecomeMapping(old_field, new_field, "static field");
+  AddBecomeMapping(old_field, new_field);
 }
 
 void ProgramReloadContext::AddBecomeMapping(const Object& old,
-                                            const Object& neu,
-                                            const char* whence) {
-  become_.Add(old, neu, whence);
+                                            const Object& neu) {
+  become_.Add(old, neu);
 }
 
 void ProgramReloadContext::RestoreClassHierarchyInvariants() {
diff --git a/runtime/vm/isolate_reload.h b/runtime/vm/isolate_reload.h
@@ -379,9 +379,7 @@ class ProgramReloadContext {
   void AddLibraryMapping(const Library& replacement_or_new,
                          const Library& original);
   void AddStaticFieldMapping(const Field& old_field, const Field& new_field);
-  void AddBecomeMapping(const Object& old,
-                        const Object& neu,
-                        const char* whence);
+  void AddBecomeMapping(const Object& old, const Object& neu);
   void RestoreClassHierarchyInvariants();
 
   Become become_;
diff --git a/runtime/vm/object_reload.cc b/runtime/vm/object_reload.cc
@@ -338,7 +338,7 @@ void Class::MigrateImplicitStaticClosures(ProgramReloadContext* irc,
         if (old_closure.IsCanonical()) {
           new_closure.SetCanonical();
         }
-        irc->AddBecomeMapping(old_closure, new_closure, "static tear-off");
+        irc->AddBecomeMapping(old_closure, new_closure);
       }
     }
   }
@@ -758,7 +758,7 @@ void Library::CheckReload(const Library& replacement,
       // The replacement of the old prefix with the new prefix
       // in Isolate::loaded_prefixes_set_ implicitly carried
       // the loaded state over to the new prefix.
-      context->AddBecomeMapping(original_prefix, prefix, "library prefix");
+      context->AddBecomeMapping(original_prefix, prefix);
     }
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -992,7 +992,7 @@ void IsolateGroup::RehashConstants(Become* become) {`
`992`	`992`	`}`
`993`	`993`
`994`	`994`	`if (old_value.ptr() != new_value.ptr()) {`
`995`		`- become->Add(old_value, new_value, "enum forwarding");`
	`995`	`+ become->Add(old_value, new_value);`
`996`	`996`	`}`
`997`	`997`	`if (new_value.IsCanonical()) {`
`998`	`998`	`cls.InsertCanonicalConstant(zone, new_value);`
`@@ -1013,7 +1013,7 @@ void IsolateGroup::RehashConstants(Become* become) {`
`1013`	`1013`	`} else {`
`1014`	`1014`	`new_value = old_value.Canonicalize(thread);`
`1015`	`1015`	`if (old_value.ptr() != new_value.ptr()) {`
`1016`		`- become->Add(old_value, new_value, "constant forwarding");`
	`1016`	`+ become->Add(old_value, new_value);`
`1017`	`1017`	`}`
`1018`	`1018`	`}`
`1019`	`1019`	`}`
Original file line number	Diff line number	Diff line change
`@@ -357,9 +357,7 @@ void InstanceMorpher::CreateMorphedCopies(Become* become) {`
`357`	`357`	`// instances of any class with the old size.`
`358`	`358`	`Become::MakeDummyObject(before);`
`359`	`359`
`360`		`- become->Add(`
`361`		`- before, after,`
`362`		`- is_canonical ? "instance morphing canonical" : "instance morphing");`
	`360`	`+ become->Add(before, after);`
`363`	`361`	`}`
`364`	`362`	`}`
`365`	`363`
`@@ -1281,7 +1279,7 @@ void ProgramReloadContext::RegisterClass(const Class& new_cls) {`
`1281`	`1279`	`IG->class_table()->SetAt(old_cls.id(), new_cls.ptr());`
`1282`	`1280`	`new_cls.CopyCanonicalConstants(old_cls);`
`1283`	`1281`	`new_cls.CopyDeclarationType(old_cls);`
`1284`		`- AddBecomeMapping(old_cls, new_cls, "class forwarding");`
	`1282`	`+ AddBecomeMapping(old_cls, new_cls);`
`1285`	`1283`	`AddClassMapping(new_cls, old_cls);`
`1286`	`1284`	`}`
`1287`	`1285`
`@@ -1627,6 +1625,7 @@ void ProgramReloadContext::RollbackLibraries() {`
`1627`	`1625`	`}`
`1628`	`1626`
`1629`	`1627`	`void ProgramReloadContext::VerifyMaps() {`
	`1628`	`+#if defined(DEBUG)`
`1630`	`1629`	`TIMELINE_SCOPE(VerifyMaps);`
`1631`	`1630`
`1632`	`1631`	`// Verify that two old classes aren't both mapped to the same new`
`@@ -1686,6 +1685,7 @@ void ProgramReloadContext::VerifyMaps() {`
`1686`	`1685`	`}`
`1687`	`1686`	`library_map.Release();`
`1688`	`1687`	`reverse_library_map.Release();`
	`1688`	`+#endif // defined(DEBUG)`
`1689`	`1689`	`}`
`1690`	`1690`
`1691`	`1691`	`void ProgramReloadContext::CommitBeforeInstanceMorphing() {`
`@@ -2589,7 +2589,7 @@ void ProgramReloadContext::BuildLibraryMapping() {`
`2589`	`2589`	`// Replaced class.`
`2590`	`2590`	`AddLibraryMapping(replacement_or_new, old);`
`2591`	`2591`
`2592`		`- AddBecomeMapping(old, replacement_or_new, "library forwarding");`
	`2592`	`+ AddBecomeMapping(old, replacement_or_new);`
`2593`	`2593`	`}`
`2594`	`2594`	`}`
`2595`	`2595`	`}`
`@@ -2698,13 +2698,12 @@ void ProgramReloadContext::AddStaticFieldMapping(const Field& old_field,`
`2698`	`2698`	`const Field& new_field) {`
`2699`	`2699`	`ASSERT(old_field.is_static());`
`2700`	`2700`	`ASSERT(new_field.is_static());`
`2701`		`- AddBecomeMapping(old_field, new_field, "static field");`
	`2701`	`+ AddBecomeMapping(old_field, new_field);`
`2702`	`2702`	`}`
`2703`	`2703`
`2704`	`2704`	`void ProgramReloadContext::AddBecomeMapping(const Object& old,`
`2705`		`- const Object& neu,`
`2706`		`- const char* whence) {`
`2707`		`- become_.Add(old, neu, whence);`
	`2705`	`+ const Object& neu) {`
	`2706`	`+ become_.Add(old, neu);`
`2708`	`2707`	`}`
`2709`	`2708`
`2710`	`2709`	`void ProgramReloadContext::RestoreClassHierarchyInvariants() {`
Original file line number	Diff line number	Diff line change
`@@ -338,7 +338,7 @@ void Class::MigrateImplicitStaticClosures(ProgramReloadContext* irc,`
`338`	`338`	`if (old_closure.IsCanonical()) {`
`339`	`339`	`new_closure.SetCanonical();`
`340`	`340`	`}`
`341`		`- irc->AddBecomeMapping(old_closure, new_closure, "static tear-off");`
	`341`	`+ irc->AddBecomeMapping(old_closure, new_closure);`
`342`	`342`	`}`
`343`	`343`	`}`
`344`	`344`	`}`
`@@ -758,7 +758,7 @@ void Library::CheckReload(const Library& replacement,`
`758`	`758`	`// The replacement of the old prefix with the new prefix`
`759`	`759`	`// in Isolate::loaded_prefixes_set_ implicitly carried`
`760`	`760`	`// the loaded state over to the new prefix.`
`761`		`- context->AddBecomeMapping(original_prefix, prefix, "library prefix");`
	`761`	`+ context->AddBecomeMapping(original_prefix, prefix);`
`762`	`762`	`}`
`763`	`763`	`}`
`764`	`764`	`}`