RFR: 8260309: Shenandoah: Clean up ShenandoahBarrierSet [v2]
Aleksey Shipilev
shade at openjdk.java.net
Mon Feb 1 09:14:44 UTC 2021
On Fri, 29 Jan 2021 14:45:50 GMT, Roman Kennke <rkennke at openjdk.org> wrote:
>> We collected some cruft in ShenandoahBarrierSet. Time to clean it up.
>>
>> This fixes/removes a number of includes, fixes some comments and it also removes is_a() and is_aligned() which look like leftovers/requirements from earlier incarnations of the superclass BarrierSet. Using the override keyword would be useful for such situations (btw, are we ok to start using override, nullptr, auto etc in Shenandoah, or do we want to keep it C++ for backporting ease?)
>>
>> One thing I was not sure about is the ShenandoahHeap* _heap field. Making it const will likely help the compiler avoid repeated access (e.g. in a number of perf-critical paths like the LRB impl). However, maybe we should get rid of the field altogether and make it explicitely using ShenandoahHeap::heap() and avoid repeated access instead of helping the compiler and hoping for the best?
>>
>> Testing:
>> - [x] hotspot_gc_shenandoah release, fastdebug
>
> Roman Kennke has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
>
> - Restore some changes that have been lost during merge
> - Merge branch 'master' into JDK-8260309
> - 8260309: Shenandoah: Clean up ShenandoahBarrierSet
Looks fine, but I have a minor question.
src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp line 28:
> 26: #define SHARE_GC_SHENANDOAH_SHENANDOAHBARRIERSET_INLINE_HPP
> 27:
> 28: #include "gc/shared/accessBarrierSupport.hpp"
Should it be `accessBarrierSupport.inline.hpp`? Other `*BarrierSet.inline.hpp`-s seem to include that.
-------------
Marked as reviewed by shade (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2202
More information about the shenandoah-dev
mailing list