RFR: 8296875: Generational ZGC: Refactor loom code [v4]

[Patricio Chilano Mateo]

On Thu, 17 Nov 2022 09:24:04 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> The current loom code makes some assumptions about GC that will not work with generational ZGC. We should make this code more GC agnostic, and provide a better interface for talking to the GC.
>> 
>> In particular,
>> 1) All GCs have a way of encoding oops inside of the heap differently to oops outside of the heap. For non-ZGC collectors, that is compressed oops. For ZGC, that is colored pointers. With generational ZGC, pointers on-heap will be colored and pointers off-heap will be "colorless". So we need to generalize encoding and decoding of oops in the heap, for loom.
>> 
>> 2) The cont_oop is located on a stack. In order to access it we need to start_processing on that thread, if it isn't the current thread. This happened to work so far for ZGC, because the stale pointers had enough colors. But with generational ZGC, these on-stack oops will be colorless, so we have to be more accurate here and ensure processing really has started on any thread that cont_oop is used on. To make life a bit easier, I'm moving the oop processing responsibility for these oops to the thread instead. Currently there is no more than one of these, so doing it lazily per frame seems a bit overkill.
>> 
>> 3) Refactoring the stack chunk allocation code
>> 
>> Tested with tier1-5 and manually running Skynet. No regressions detected. We have also been running with this (yet a slightly different backend) in the generational ZGC repo for a while now.
>
> Erik Österlund has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix Richard comments

I went through the changes and all looks good to me. Only minor comments.

Thanks,
Patricio

src/hotspot/share/gc/shared/memAllocator.cpp line 381:

> 379: }
> 380: 
> 381: oop MemAllocator::try_allocate_in_existing_tlab() {

try_allocate_in_existing_tlab() is now unused in memAllocator.hpp.

src/hotspot/share/gc/shared/memAllocator.hpp line 98:

> 96:   virtual oop initialize(HeapWord* mem) const;
> 97: 
> 98:   using MemAllocator::allocate;

Do we need these declarations? I thought this would be needed if allocate() would not be public on the base class or to avoid hiding it if here we define a method with the same name but different signature.

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1393:

> 1391:       // Guaranteed to be in young gen / newly allocated memory
> 1392:       assert(!chunk->requires_barriers(), "Unfamiliar GC requires barriers on TLAB allocation");
> 1393:       _barriers = false;

Do we need to explicitly set _barriers to false? It's already initialized to be false (same above for the UseZGC case). That would also allow to simplify the code a bit I think to be just an if statement that calls requires_barriers() for the "ZGC_ONLY(!UseZGC &&) (SHENANDOAHGC_ONLY(UseShenandoahGC ||) allocator.took_slow_path())" case, and then ZGC and the fast path could use just separate asserts outside conditionals.

-------------

Marked as reviewed by pchilanomate (Reviewer).

PR: https://git.openjdk.org/jdk/pull/11111