On Wed, 16 Nov 2022 15:47:37 GMT, Richard Reingruber <[email protected]> wrote:
>> 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.
Thanks @reinrich for the review! I have pushed a comment for the continuation
oops where they are handled as requested.
> 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`?
The compiler should be able to do that already. We devirtualize calls into oop
closures, and the closure is stack allocated. So the compiler should be able to
do that if it finds that it is a good idea. I'd prefer to leave that to the
compiler.
> 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?
No we don't convert the oops for Shenandoah. Instead, Shenandoah's closures
know how to deal with both oop* and narrowOop* on the heap, and will get passed
the appropriate type of pointer. So it doesn't use the bitmap. I have tested
that it works with Shenandoah as well.
-------------
PR: https://git.openjdk.org/jdk/pull/11111
