8261954: Dependencies: Improve iteration over class hierarchy under context class

Reviewed-by: kvn, coleenp, eosterlund

src/hotspot/share/code/dependencies.cpp

@@ -1122,17 +1122,18 @@ class ClassHierarchyWalker {
     } else if (!k->is_instance_klass()) {
       return false; // no methods to find in an array type
     } else {
+      InstanceKlass* ik = InstanceKlass::cast(k);
       // Search class hierarchy first, skipping private implementations
       // as they never override any inherited methods
-      Method* m = InstanceKlass::cast(k)->find_instance_method(_name, _signature, Klass::PrivateLookupMode::skip);
+      Method* m = ik->find_instance_method(_name, _signature, Klass::PrivateLookupMode::skip);
-      if (!Dependencies::is_concrete_method(m, k)) {
+      if (!Dependencies::is_concrete_method(m, ik)) {
         // Check for re-abstraction of method
-        if (!k->is_interface() && m != NULL && m->is_abstract()) {
+        if (!ik->is_interface() && m != NULL && m->is_abstract()) {
           // Found a matching abstract method 'm' in the class hierarchy.
           // This is fine iff 'k' is an abstract class and all concrete subtypes
           // of 'k' override 'm' and are participates of the current search.
           ClassHierarchyWalker wf(_participants, _num_participants);
-          Klass* w = wf.find_witness_subtype(k);
+          Klass* w = wf.find_witness_subtype(ik);
           if (w != NULL) {
             Method* wm = InstanceKlass::cast(w)->find_instance_method(_name, _signature, Klass::PrivateLookupMode::skip);
             if (!Dependencies::is_concrete_method(wm, w)) {
@@ -1145,10 +1146,10 @@ class ClassHierarchyWalker {
           }
         }
         // Check interface defaults also, if any exist.
-        Array<Method*>* default_methods = InstanceKlass::cast(k)->default_methods();
+        Array<Method*>* default_methods = ik->default_methods();
         if (default_methods == NULL)
             return false;
-        m = InstanceKlass::cast(k)->find_method(default_methods, _name, _signature);
+        m = ik->find_method(default_methods, _name, _signature);
         if (!Dependencies::is_concrete_method(m, NULL))
             return false;
       }
@@ -1188,16 +1189,17 @@ class ClassHierarchyWalker {
 
  private:
   // the actual search method:
-  Klass* find_witness_anywhere(Klass* context_type,
+  Klass* find_witness_anywhere(InstanceKlass* context_type,
-                                 bool participants_hide_witnesses,
+                               bool participants_hide_witnesses);
-                                 bool top_level_call = true);
   // the spot-checking version:
   Klass* find_witness_in(KlassDepChange& changes,
-                         Klass* context_type,
+                         InstanceKlass* context_type,
-                           bool participants_hide_witnesses);
+                         bool participants_hide_witnesses);
  public:
-  Klass* find_witness_subtype(Klass* context_type, KlassDepChange* changes = NULL) {
+  Klass* find_witness_subtype(Klass* k, KlassDepChange* changes = NULL) {
     assert(doing_subtype_search(), "must set up a subtype search");
+    assert(k->is_instance_klass(), "required");
+    InstanceKlass* context_type = InstanceKlass::cast(k);
     // When looking for unexpected concrete types,
     // do not look beneath expected ones.
     const bool participants_hide_witnesses = true;
@@ -1209,8 +1211,10 @@ class ClassHierarchyWalker {
       return find_witness_anywhere(context_type, participants_hide_witnesses);
     }
   }
-  Klass* find_witness_definer(Klass* context_type, KlassDepChange* changes = NULL) {
+  Klass* find_witness_definer(Klass* k, KlassDepChange* changes = NULL) {
     assert(!doing_subtype_search(), "must set up a method definer search");
+    assert(k->is_instance_klass(), "required");
+    InstanceKlass* context_type = InstanceKlass::cast(k);
     // When looking for unexpected concrete methods,
     // look beneath expected ones, to see if there are overrides.
     const bool participants_hide_witnesses = true;
@@ -1271,7 +1275,7 @@ static bool count_find_witness_calls() {
 
 
 Klass* ClassHierarchyWalker::find_witness_in(KlassDepChange& changes,
-                                             Klass* context_type,
+                                             InstanceKlass* context_type,
                                              bool participants_hide_witnesses) {
   assert(changes.involves_context(context_type), "irrelevant dependency");
   Klass* new_type = changes.new_type();
@@ -1284,7 +1288,7 @@ Klass* ClassHierarchyWalker::find_witness_in(KlassDepChange& changes,
   // Must not move the class hierarchy during this check:
   assert_locked_or_safepoint(Compile_lock);
 
-  int nof_impls = InstanceKlass::cast(context_type)->nof_implementors();
+  int nof_impls = context_type->nof_implementors();
   if (nof_impls > 1) {
     // Avoid this case: *I.m > { A.m, C }; B.m > C
     // %%% Until this is fixed more systematically, bail out.
@@ -1315,15 +1319,8 @@ Klass* ClassHierarchyWalker::find_witness_in(KlassDepChange& changes,
   return NULL;
 }
 
-
 // Walk hierarchy under a context type, looking for unexpected types.
-// Do not report participant types, and recursively walk beneath
+Klass* ClassHierarchyWalker::find_witness_anywhere(InstanceKlass* context_type, bool participants_hide_witnesses) {
-// them only if participants_hide_witnesses is false.
-// If top_level_call is false, skip testing the context type,
-// because the caller has already considered it.
-Klass* ClassHierarchyWalker::find_witness_anywhere(Klass* context_type,
-                                                   bool participants_hide_witnesses,
-                                                   bool top_level_call) {
   // Current thread must be in VM (not native mode, as in CI):
   assert(must_be_in_vm(), "raw oops here");
   // Must not move the class hierarchy during this check:
@@ -1332,106 +1329,50 @@ Klass* ClassHierarchyWalker::find_witness_anywhere(Klass* context_type,
   bool do_counts = count_find_witness_calls();
 
   // Check the root of the sub-hierarchy first.
-  if (top_level_call) {
+  if (do_counts) {
-    if (do_counts) {
+    NOT_PRODUCT(deps_find_witness_calls++);
-      NOT_PRODUCT(deps_find_witness_calls++);
-      NOT_PRODUCT(deps_find_witness_steps++);
-    }
-    if (is_participant(context_type)) {
-      if (participants_hide_witnesses)  return NULL;
-      // else fall through to search loop...
-    } else if (is_witness(context_type) && !ignore_witness(context_type)) {
-      // The context is an abstract class or interface, to start with.
-      return context_type;
-    }
   }
 
-  // Now we must check each implementor and each subclass.
+  // (Note: Interfaces do not have subclasses.)
-  // Use a short worklist to avoid blowing the stack.
-  // Each worklist entry is a *chain* of subklass siblings to process.
-  const int CHAINMAX = 100;  // >= 1 + InstanceKlass::implementors_limit
-  Klass* chains[CHAINMAX];
-  int    chaini = 0;  // index into worklist
-  Klass* chain;       // scratch variable
-#define ADD_SUBCLASS_CHAIN(k)                     {  \
-    assert(chaini < CHAINMAX, "oob");                \
-    chain = k->subklass();                           \
-    if (chain != NULL)  chains[chaini++] = chain;    }
-
-  // Look for non-abstract subclasses.
-  // (Note:  Interfaces do not have subclasses.)
-  ADD_SUBCLASS_CHAIN(context_type);
-
   // If it is an interface, search its direct implementors.
-  // (Their subclasses are additional indirect implementors.
+  // (Their subclasses are additional indirect implementors. See InstanceKlass::add_implementor().)
-  // See InstanceKlass::add_implementor.)
+  if (context_type->is_interface()) {
-  // (Note:  nof_implementors is always zero for non-interfaces.)
+    int nof_impls = context_type->nof_implementors();
-  if (top_level_call) {
+    if (nof_impls == 0) {
-    int nof_impls = InstanceKlass::cast(context_type)->nof_implementors();
+      return NULL; // no implementors
-    if (nof_impls > 1) {
+    } else if (nof_impls == 1) { // unique implementor
+      assert(context_type != context_type->implementor(), "not unique");
+      context_type = InstanceKlass::cast(context_type->implementor());
+    } else { // nof_impls >= 2
       // Avoid this case: *I.m > { A.m, C }; B.m > C
       // Here, I.m has 2 concrete implementations, but m appears unique
       // as A.m, because the search misses B.m when checking C.
       // The inherited method B.m was getting missed by the walker
       // when interface 'I' was the starting point.
       // %%% Until this is fixed more systematically, bail out.
-      // (Old CHA had the same limitation.)
       return context_type;
     }
-    if (nof_impls > 0) {
-      Klass* impl = InstanceKlass::cast(context_type)->implementor();
-      assert(impl != NULL, "just checking");
-      // If impl is the same as the context_type, then more than one
-      // implementor has seen. No exact info in this case.
-      if (impl == context_type) {
-        return context_type;  // report an inexact witness to this sad affair
-      }
-      if (do_counts)
-        { NOT_PRODUCT(deps_find_witness_steps++); }
-      if (is_participant(impl)) {
-        if (!participants_hide_witnesses) {
-          ADD_SUBCLASS_CHAIN(impl);
-        }
-      } else if (is_witness(impl) && !ignore_witness(impl)) {
-        return impl;
-      } else {
-        ADD_SUBCLASS_CHAIN(impl);
-      }
-    }
   }
 
-  // Recursively process each non-trivial sibling chain.
+  assert(!context_type->is_interface(), "not allowed");
-  while (chaini > 0) {
+
-    Klass* chain = chains[--chaini];
+  for (ClassHierarchyIterator iter(context_type); !iter.done(); iter.next()) {
-    for (Klass* sub = chain; sub != NULL; sub = sub->next_sibling()) {
+    Klass* sub = iter.klass();
-      if (do_counts) { NOT_PRODUCT(deps_find_witness_steps++); }
+
-      if (is_participant(sub)) {
+    if (do_counts) { NOT_PRODUCT(deps_find_witness_steps++); }
-        if (participants_hide_witnesses)  continue;
+
-        // else fall through to process this guy's subclasses
+    // Do not report participant types.
-      } else if (is_witness(sub) && !ignore_witness(sub)) {
+    if (is_participant(sub)) {
-        return sub;
+      // Walk beneath a participant only when it doesn't hide witnesses.
-      }
+      if (participants_hide_witnesses) {
-      if (chaini < (VerifyDependencies? 2: CHAINMAX)) {
+        iter.skip_subclasses();
-        // Fast path.  (Partially disabled if VerifyDependencies.)
-        ADD_SUBCLASS_CHAIN(sub);
-      } else {
-        // Worklist overflow.  Do a recursive call.  Should be rare.
-        // The recursive call will have its own worklist, of course.
-        // (Note that sub has already been tested, so that there is
-        // no need for the recursive call to re-test.  That's handy,
-        // since the recursive call sees sub as the context_type.)
-        if (do_counts) { NOT_PRODUCT(deps_find_witness_recursions++); }
-        Klass* witness = find_witness_anywhere(sub,
-                                               participants_hide_witnesses,
-                                               /*top_level_call=*/ false);
-        if (witness != NULL)  return witness;
       }
+    } else if (is_witness(sub) && !ignore_witness(sub)) {
+      return sub; // found a witness
     }
   }
-
   // No witness found.  The dependency remains unbroken.
   return NULL;
-#undef ADD_SUBCLASS_CHAIN
 }
 
 

src/hotspot/share/oops/instanceKlass.cpp

@@ -4284,3 +4284,24 @@ void InstanceKlass::log_to_classlist(const ClassFileStream* stream) const {
   }
 #endif // INCLUDE_CDS
 }
+
+// Make a step iterating over the class hierarchy under the root class.
+// Skips subclasses if requested.
+void ClassHierarchyIterator::next() {
+  assert(_current != NULL, "required");
+  if (_visit_subclasses && _current->subklass() != NULL) {
+    _current = _current->subklass();
+    return; // visit next subclass
+  }
+  _visit_subclasses = true; // reset
+  while (_current->next_sibling() == NULL && _current != _root) {
+    _current = _current->superklass(); // backtrack; no more sibling subclasses left
+  }
+  if (_current == _root) {
+    // Iteration is over (back at root after backtracking). Invalidate the iterator.
+    _current = NULL;
+    return;
+  }
+  _current = _current->next_sibling();
+  return; // visit next sibling subclass
+}

src/hotspot/share/oops/instanceKlass.hpp

@@ -1430,4 +1430,41 @@ class InnerClassesIterator : public StackObj {
   }
 };
 
+// Iterator over class hierarchy under a particular class. Implements depth-first pre-order traversal.
+// Usage:
+//  for (ClassHierarchyIterator iter(root_klass); !iter.done(); iter.next()) {
+//    Klass* k = iter.klass();
+//    ...
+//  }
+class ClassHierarchyIterator : public StackObj {
+ private:
+  InstanceKlass* _root;
+  Klass*         _current;
+  bool           _visit_subclasses;
+
+ public:
+  ClassHierarchyIterator(InstanceKlass* root) : _root(root), _current(root), _visit_subclasses(true) {
+    assert(!root->is_interface(), "no subclasses");
+    assert(_root == _current, "required"); // initial state
+  }
+
+  bool done() {
+    return (_current == NULL);
+  }
+
+  // Make a step iterating over the class hierarchy under the root class.
+  // Skips subclasses if requested.
+  void next();
+
+  Klass* klass() {
+    assert(!done(), "sanity");
+    return _current;
+  }
+
+  // Skip subclasses of the current class.
+  void skip_subclasses() {
+    _visit_subclasses = false;
+  }
+};
+
 #endif // SHARE_OOPS_INSTANCEKLASS_HPP