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

[Y. Srinivas Ramakrishna]

On Wed, 4 Dec 2024 09:48:20 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> 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.
>
> 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),

The deMorganization of the first disjunct does make it easier to read as you state. My reason to write it in its first form was because I tend to think of `(not A) or B` as `A implies B` (and written in the first form because C lacks an `implies` operator). I'll rewrite as you suggest before I push this.

Thanks for the review!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22507#discussion_r1870023768