On Mon, 14 Nov 2022 16:07:34 GMT, Erik Österlund <[email protected]> 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:
>
> Indentation fix
Hi @fisk, I've skimmed the changes. They look good to me. I do have a few
comments/questions also.
src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp line 876:
> 874:
> 875: OopMap* map = new OopMap(((int)ContinuationEntry::size() + wordSize) /
> VMRegImpl::stack_slot_size, 0 /* arg_slots*/);
> 876: ContinuationEntry::setup_oopmap(map);
I'd suggest to add a comment where the oops are handled.
src/hotspot/share/gc/shared/barrierSetStackChunk.cpp line 68:
> 66:
> 67: virtual void do_oop(oop* p) override {
> 68: if (UseCompressedOops) {
Wouldn't it be better to hoist the check for `UseCompressedOops`?
src/hotspot/share/gc/shenandoah/shenandoahBarrierSetStackChunk.cpp line 30:
> 28:
> 29: void ShenandoahBarrierSetStackChunk::encode_gc_mode(stackChunkOop chunk,
> OopIterator* oop_iterator) {
> 30: // Nothing to do
Shenandoah allows `UseCompressedOops` enabled, doesn't it? Isn't it necessary
then to do the encoding as in the super class?
-------------
PR: https://git.openjdk.org/jdk/pull/11111
