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

[Kelvin Nilsen]

On Fri, 1 Sep 2023 20:12:58 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

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

I've made your suggested changes and am sending through the test pipeline again.

I agree that it would be good to eventually audit/refactor to have more consistent method names and conventions.

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

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