Fix bug with CHA dependencies by recording a set of classes for registering code.

runtime/vm/cha.cc

Index: runtime/vm/cha.cc
===================================================================
--- runtime/vm/cha.cc	(revision 39070)
+++ runtime/vm/cha.cc	(working copy)
@@ -12,92 +12,93 @@
 
 namespace dart {
 
-bool CHA::HasSubclasses(intptr_t cid) {
-  Isolate* isolate = Isolate::Current();
-  const bool has_subclasses = HasSubclassesSafe(cid);
-  if (!has_subclasses) {
-    isolate->set_cha_used(true);
+void CHA::AddToCHAClasses(Class* cls) {

[Vyacheslav Egorov (Google), 2014/08/13 11:40:28]

Accept an normal Handle here?

[Florian Schneider, 2014/08/13 12:08:57]

Done.

+  for (intptr_t i = 0; i < cha_classes_.length(); i++) {
+    if (cha_classes_[i]->raw() == cls->raw()) {
+      return;
+    }
   }
-  return has_subclasses;
+  cha_classes_.Add(cls);

[Vyacheslav Egorov (Google), 2014/08/13 11:40:27]

Rewrap into ZoneHandle here.

[Florian Schneider, 2014/08/13 12:08:57]

Done.

 }
 
 
-bool CHA::HasSubclassesSafe(intptr_t cid) {
-  ASSERT(cid >= kInstanceCid);
-  Isolate* isolate = Isolate::Current();
-  const ClassTable& class_table = *isolate->class_table();
-  const Class& cls = Class::Handle(isolate, class_table.At(cid));
+// Return true if the class is private to our internal libraries (not extendable
+// or implementable by users).

[Vyacheslav Egorov (Google), 2014/08/13 11:40:28]

The key point is that it can't be extended later after startup, not that it
can't be extended by users.

+// (TODO): Allow more libraries.

[Vyacheslav Egorov (Google), 2014/08/13 11:40:27]

TODO(fschneider) ? :)

strange TODO syntax.

[Florian Schneider, 2014/08/13 12:08:58]

Done.

+static bool IsKnownPrivateClass(const Class& type_class) {
+  if (!Library::IsPrivate(String::Handle(type_class.Name()))) return false;
+  const Library& library = Library::Handle(type_class.library());
+  if (library.raw() == Library::CoreLibrary()) return true;
+  if (library.raw() == Library::CollectionLibrary()) return true;
+  if (library.raw() == Library::TypedDataLibrary()) return true;
+  if (library.raw() == Library::MathLibrary()) return true;
+  return false;
+}
+
+
+bool CHA::HasSubclasses(const Class& cls) {
   ASSERT(!cls.IsNull());
+  ASSERT(cls.id() >= kInstanceCid);
+  // Can't track dependencies for classes on the VM heap since those are
+  // read-only.
+  // TODO(fschneider): Enable tracking of CHA dependent code for VM heap
+  // classes.
+  if (cls.InVMHeap()) return true;
+
   if (cls.IsObjectClass()) {
     // Class Object has subclasses, although we do not keep track of them.
     return true;
   }
-  const GrowableObjectArray& cls_direct_subclasses =
-      GrowableObjectArray::Handle(isolate, cls.direct_subclasses());
-  return
-      !cls_direct_subclasses.IsNull() && (cls_direct_subclasses.Length() > 0);
+  const GrowableObjectArray& direct_subclasses =
+      GrowableObjectArray::Handle(isolate_, cls.direct_subclasses());
+  bool result =
+      !direct_subclasses.IsNull() && (direct_subclasses.Length() > 0);
+  if (!result && !IsKnownPrivateClass(cls)) {
+    AddToCHAClasses(&Class::ZoneHandle(cls.raw()));

[Vyacheslav Egorov (Google), 2014/08/13 11:40:28]

I think you should move IsKnownPrivateClass into the AddToCHAClasses and just do
an early return from there.

[Florian Schneider, 2014/08/13 12:08:58]

Done.

+  }
+  return result;
 }
 
 
-// Returns true if the given array of cids contains the given cid.
-static bool ContainsCid(ZoneGrowableArray<intptr_t>* cids, intptr_t cid) {
-  for (intptr_t i = 0; i < cids->length(); i++) {
-    if ((*cids)[i] == cid) {
-      return true;
-    }
-  }
-  return false;
+bool CHA::HasSubclasses(intptr_t cid) {
+  const ClassTable& class_table = *isolate_->class_table();
+  Class& cls = Class::Handle(isolate_, class_table.At(cid));
+  return HasSubclasses(cls);
 }
 
 
-// Recursively collect direct and indirect subclass ids of cls.
-static void CollectSubclassIds(ZoneGrowableArray<intptr_t>* cids,
-                               const Class& cls) {
-  const GrowableObjectArray& cls_direct_subclasses =
-      GrowableObjectArray::Handle(cls.direct_subclasses());
-  if (cls_direct_subclasses.IsNull()) {
-    return;
+bool CHA::IsImplemented(const Class& cls) {
+  // Can't track dependencies for classes on the VM heap since those are
+  // read-only.
+  // TODO(fschneider): Enable tracking of CHA dependent code for VM heap
+  // classes.
+  if (cls.InVMHeap()) return true;
+
+  bool result = cls.is_implemented();
+  if (!result && !IsKnownPrivateClass(cls)) {
+    AddToCHAClasses(&Class::ZoneHandle(cls.raw()));
   }
-  Class& direct_subclass = Class::Handle();
-  for (intptr_t i = 0; i < cls_direct_subclasses.Length(); i++) {
-    direct_subclass ^= cls_direct_subclasses.At(i);
-    intptr_t direct_subclass_id = direct_subclass.id();
-    if (!ContainsCid(cids, direct_subclass_id)) {
-      cids->Add(direct_subclass_id);
-      CollectSubclassIds(cids, direct_subclass);
-    }
-  }
+  return result;
 }
 
 
-ZoneGrowableArray<intptr_t>* CHA::GetSubclassIdsOf(intptr_t cid) {
-  ASSERT(cid > kInstanceCid);
-  Isolate* isolate = Isolate::Current();
-  const ClassTable& class_table = *isolate->class_table();
-  const Class& cls = Class::Handle(isolate, class_table.At(cid));
-  ASSERT(!cls.IsNull());
-  ZoneGrowableArray<intptr_t>* ids = new ZoneGrowableArray<intptr_t>();
-  CollectSubclassIds(ids, cls);
-  isolate->set_cha_used(true);
-  return ids;
-}
-
-
 bool CHA::HasOverride(const Class& cls, const String& function_name) {
-  Isolate* isolate = Isolate::Current();
   const GrowableObjectArray& cls_direct_subclasses =
-      GrowableObjectArray::Handle(isolate, cls.direct_subclasses());
+      GrowableObjectArray::Handle(isolate_, cls.direct_subclasses());
   // Subclasses of Object are not tracked by CHA. Safely assume that overrides
   // exist.
   if (cls.IsObjectClass()) {
     return true;
   }
 
+  bool is_known_private_class = IsKnownPrivateClass(cls);
   if (cls_direct_subclasses.IsNull()) {
-    isolate->set_cha_used(true);
+    if (!is_known_private_class) {
+      AddToCHAClasses(&Class::ZoneHandle(cls.raw()));
+    }
     return false;
   }
-  Class& direct_subclass = Class::Handle(isolate);
+  Class& direct_subclass = Class::Handle(isolate_);
   for (intptr_t i = 0; i < cls_direct_subclasses.Length(); i++) {
     direct_subclass ^= cls_direct_subclasses.At(i);
     // Unfinalized classes are treated as non-existent for CHA purposes,
@@ -111,44 +112,10 @@
       return true;
     }
   }
-  isolate->set_cha_used(true);
+  if (!is_known_private_class) {
+    AddToCHAClasses(&Class::ZoneHandle(cls.raw()));
+  }
   return false;
 }
 
-
-ZoneGrowableArray<Function*>* CHA::GetNamedInstanceFunctionsOf(
-    const ZoneGrowableArray<intptr_t>& cids,
-    const String& function_name) {
-  Isolate* isolate = Isolate::Current();
-  ASSERT(!function_name.IsNull());
-  const ClassTable& class_table = *isolate->class_table();
-  ZoneGrowableArray<Function*>* functions = new ZoneGrowableArray<Function*>();
-  Class& cls = Class::Handle(isolate);
-  Function& cls_function = Function::Handle(isolate);
-  for (intptr_t i = 0; i < cids.length(); i++) {
-    const intptr_t cid = cids[i];
-    ASSERT(cid > kInstanceCid);
-    cls = class_table.At(cid);
-    cls_function = cls.LookupDynamicFunction(function_name);
-    if (!cls_function.IsNull()) {
-      functions->Add(&Function::ZoneHandle(isolate, cls_function.raw()));
-    }
-  }
-  isolate->set_cha_used(true);
-  return functions;
-}
-
-
-ZoneGrowableArray<Function*>* CHA::GetOverridesOf(const Function& function) {
-  ASSERT(!function.IsNull());
-  ASSERT(function.IsDynamicFunction());
-  Isolate* isolate = Isolate::Current();
-  const Class& function_owner = Class::Handle(isolate, function.Owner());
-  const String& function_name = String::Handle(isolate, function.name());
-  ZoneGrowableArray<intptr_t>* cids = new ZoneGrowableArray<intptr_t>();
-  CollectSubclassIds(cids, function_owner);
-  isolate->set_cha_used(true);
-  return GetNamedInstanceFunctionsOf(*cids, function_name);
-}
-
 }  // namespace dart