RFR: 8261954: Dependencies: Improve iteration over class hierarchy under context class
Coleen Phillimore
coleenp at openjdk.java.net
Thu Feb 18 22:44:40 UTC 2021
On Thu, 18 Feb 2021 17:05:08 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:
> Simplify `ClassHierarchyWalker::find_witness_anywhere()` which iterates over class hierarchy under context class searching for witnesses.
>
> Current implementation traverses the hierarchy in a breadth-first manner and keeps a stack-allocated array to keep a worklist.
> But all the subclasses are already part of a singly linked list formed by `Klass::subklass()`/`next_sibling()`/`superklass()`.
>
> Proposed refactoring gets rid of the explicit worklist and switches to the traversal over the linked list (encapsulated into `ClassHierarchyIterator`). It performs depth-first pre-order hierarchy traversal.
>
> (There are some other minor refactorings applied in `ClassHierarchyWalker` along the way.)
>
> Testing:
> - [x] hs-tier1 - hs-tier8
> - [x] additional verification that CHA decisions aren't affected
This seems like a nice change. Some questions.
src/hotspot/share/oops/instanceKlass.hpp line 1486:
> 1484: }
> 1485: };
> 1486:
The _subklass and _next_sibling fields and implementation are in Klass (klass.hpp/cpp) but I always wonder why they are not in InstanceKlass (instanceKlass.hpp/cpp). If this new class is in instanceKlass.hpp, these fields should be in the same. If you agree, we should file an RFE and I will move them. If the fields truly belong in klass.hpp, then this should be there too. ie. they should match.
src/hotspot/share/code/dependencies.cpp line 1345:
> 1343: } else if (nof_impls == 1) { // unique implementor
> 1344: assert(context_type != context_type->implementor(), "not unique");
> 1345: context_type = InstanceKlass::cast(context_type->implementor());
There's no reason that implementor() should return Klass* rather than InstanceKlass* ? We can clean that up also later to reduce casts. (I thought I already tried to do this once).
-------------
Marked as reviewed by coleenp (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2630
More information about the hotspot-runtime-dev
mailing list