RFR: 8344593: GenShen: Review of ReduceInitialCardMarks [v2]

[Aleksey Shipilev]

On Wed, 4 Dec 2024 01:29:24 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> Fix documentation comment, and add an assertion check upon slowpath allocation.
>> 
>> I also checked the impact of +/-ReduceInitialCardMarks on GenShen using SPECjbb and didn't see any difference. We've left it enabled by default because less card marking is better in this case.
>
> Y. Srinivas Ramakrishna has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - virtual -> override missed in previous delta.
>    Fix zero build (ReduceInitialCardMarks is defined only in
>    JVMCI/Compiler2)
>  - virtual -> override in derived class ShenandoahBarrierSet.
>  - Refine previous change and future-proof ReduceInitialCardMarks for
>    GenShen.

Yes, good. Let's see if we catch any failure with this assert.

src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp line 93:

> 91: void ShenandoahBarrierSet::on_slowpath_allocation_exit(JavaThread* thread, oop new_obj) {
> 92: #if COMPILER2_OR_JVMCI
> 93:   assert(!(ReduceInitialCardMarks && ShenandoahCardBarrier) || ShenandoahGenerationalHeap::heap()->is_in_young(new_obj),

Not sure why first two are grouped, looks more understandable if written like this? Your call.

Suggestion:

  assert(!ReduceInitialCardMarks || !ShenandoahCardBarrier || ShenandoahGenerationalHeap::heap()->is_in_young(new_obj),

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22507#pullrequestreview-2477889118
PR Review Comment: https://git.openjdk.org/jdk/pull/22507#discussion_r1869089707