RFR: 8315044: GenShen: Verifier detects clean card should be dirty [v4]

[Kelvin Nilsen]

On Fri, 1 Sep 2023 00:34:58 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> Kelvin Nilsen 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:
>> 
>>  - Replace is_generational with ShenandoahCardBarrier test
>>  - Merge remote-tracking branch 'origin/master' into ref-processor-updates-remembered-set
>>  - Merge remote-tracking branch 'origin/master' into ref-processor-updates-remembered-set
>>  - Also update card table when moving discovered References to global list
>>  - Fix whitespace
>>  - Mark card as dirty if discovered reference list has interesting pointer
>
> src/hotspot/share/gc/shenandoah/shenandoahReferenceProcessor.cpp line 387:
> 
>> 385:       T* addr = reinterpret_cast<T*>(java_lang_ref_Reference::discovered_addr_raw(reference));
>> 386:       if (heap->is_in_old(addr) && heap->is_in_young(discovered_head)) {
>> 387:         heap->mark_card_as_dirty(addr);
> 
> Can you instead call the existing `card_mark_barrier` method?
> At that time, could also look at the existing uses of that method and refactor them to use the `ShenandoahCardBarrier` test like you did above, and elide the generational check in the body of the method itself, replacing it with an assertion (if you want, that the heap is generational).
> 
> (PS: I was also thinking that in the fullness of time it might be worth doing an audit of the uses of `cas` and `set_oop` and making sure they would call the card-marking code instead of exposing the functionality at the call-sites like done here. Callers that want to avoid card-marking would need to call the respective `_raw` versions then. We should makr sure to conform to naming conventions for those methods that are consistent with those used in other parts of jvm/gc code. This can be done separately and might affect legacy Shenandoah code as well. Just leaving this comment here so as not to lose the thought, but I doubt we want to do that locally here in this PR.)

Made this change and am testing result.

I agree that audit/refactoring in the future would be a good idea.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/314#discussion_r1313473253