RFR: 8287805: Shenandoah: consolidate evacuate-update-root closures [v2]

[Zhengyu Gu]

On Wed, 22 Jun 2022 15:26:09 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Zhengyu Gu has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
>> 
>>  - Review feedback
>>  - Merge branch 'master' into JDK-8287805-evac-updt-closures
>>  - 8287805: Shenandoah: consolidate evacuate-update-root closures
>>  - Merge branch 'master' into consolidate_evac_root_closures
>>  - v0
>>  - v0
>
> src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp line 99:
> 
>> 97:     inline ShenandoahEvacuateUpdateRootClosureBase();
>> 98:     inline virtual void do_oop(oop* p);
>> 99:     inline virtual void do_oop(narrowOop* p);
> 
> Do we have to make these functions `virtual`? I supposed we do templated closures exactly to avoid any sort of virtual method dispatching. Maybe I am missing something here...

Fixed

> src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp line 144:
> 
>> 142: template <bool atomic>
>> 143: void ShenandoahEvacuateUpdateRootClosureBase<atomic>::do_oop(oop* p) {
>> 144:   do_oop_work(p, Thread::current());
> 
> Not fond of calling `Thread::current()` on this hotpath. From the code, it looks like we only need it when actually calling `_heap->evacuate_object()`, can we move it there? 
> 
> Related: is `ShenandoahContextEvacuateUpdateRootsClosure::_thread` used now? Can we use it on some paths too?

My mistake, fixed.

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

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