RFR: 8227054: ServiceThread needs to know about all OopStorage objects

[coleen.phillimore at oracle.com]

I like this change!

http://cr.openjdk.java.net/~kbarrett/8227054/open.00/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.inline.hpp.udiff.html

Can you remove some #include directives from the GC code since the oop 
storages are coming from oopStorages?

http://cr.openjdk.java.net/~kbarrett/8227054/open.00/src/hotspot/share/prims/resolvedMethodTable.cpp.udiff.html

Does this still need to #include oopStorage.inline.hpp ?

http://cr.openjdk.java.net/~kbarrett/8227054/open.00/src/hotspot/share/prims/resolvedMethodTable.hpp.frames.html

This doesn't seem to need to include oopStorage.hpp.  Might be others too.

http://cr.openjdk.java.net/~kbarrett/8227054/open.00/src/hotspot/share/gc/shared/oopStorages.hpp.html

I thought our compilers were tolerant of a trailing comma in enumerations?

The macros aren't bad though.  It seems like it would be easy to add a 
new OopStorage if we wanted to, but it would be better to use an 
existing vm weak or vm strong oop storage, if we wanted to move more 
oops into an oop storage (say for JFR).

This change looks great apart from trying to remove more #includes.

Thanks!
Coleen


On 7/25/19 6:59 PM, Kim Barrett wrote:
> 8227054: ServiceThread needs to know about all OopStorage objects
> 8227053: ServiceThread cleanup of OopStorage is missing some
>
> Please review this change in how OopStorage objects are managed and
> accessed.  There is a new (all static) class, OopStorages, which
> provides infrastructure for creating all the storage objects, access
> via an enum-based id, and iterating over them.
>
> Various components that previously managed their own storage objects
> now obtain them from OopStorages.  A number of access functions have
> been eliminated as part of that, though some have been retained for
> internal convenience of a component.
>
> The set of OopStorage objects is now declared in one place, using
> x-macros, with collective definitions and usages ultimately driven off
> those macros.  This includes the ServiceThread (which no longer needs
> explicit knowledge of the set, and is no longer missing any) and the
> OopStorage portion of WeakProcessorPhases.  For now, the various GCs
> still have explicit knowledge of the set; that will be addressed in
> followup changes specific to each collector.  (This delay minimizes
> the impact on Leo's in-progress review that changes ParallelGC to use
> WorkGangs.)
>
> This change also includes a couple of utility macros for working with
> x-macros.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8227054
> https://bugs.openjdk.java.net/browse/JDK-8227053
>
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8227054/open.00/
>
> Testing:
> mach5 tier1-3
>