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

Aleksey Shipilev shade at openjdk.org
Mon Jul 4 07:42:49 UTC 2022


On Thu, 30 Jun 2022 19:42:42 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

>> ShenandoahEvacuateUpdateMetadataClosure and ShenandoahEvacuateUpdateRootsClosure are mostly same, can be consolidated.
>> 
>> Also, only uses of ShenandoahEvacuateUpdateMetadataClosure all pass MO_UNORDERED template parameter, so it can be eliminated.
>> 
>> Test:
>> 
>> - [x] hotspot_gc_shenandoah
>
> 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 10 additional commits since the last revision:
> 
>  - Aleksey's comment
>  - Merge branch 'master' into JDK-8287805-evac-updt-closures
>  - v1
>  - Remove unused impl
>  - 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

Looks fine, with minor nits.

src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp line 145:

> 143: 
> 144:   Thread* thr = stable_thread ? _thread : Thread::current();
> 145:   assert(thr == Thread::current(), "Wrong thread");

I think we can move these lines down to the only use in this thread. This would keep `Thread::current()` out of the hotpath.

src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp line 148:

> 146: 
> 147:   T o = RawAccess<>::oop_load(p);
> 148:   if (! CompressedOops::is_null(o)) {

Redundant whitespace?

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

Marked as reviewed by shade (Reviewer).

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



More information about the hotspot-gc-dev mailing list