RFR: 8288129: Shenandoah: Skynet test crashed with iu + aggressive

[Erik Österlund]

On Tue, 30 Aug 2022 15:42:31 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>>> The neat thing with using the right Access API stores instead of an ad hoc barrier, is that it works for both shenandoah IU mode, and generational ZGC, while the ad hoc barrier does not.
>> 
>> @fisk are you suggesting that changing the Access API from `set_parent_raw` to `set_parent_access` is the right approach? This introduces gc policy specific code which does not look clean.
>> It appears that the combination of `requires_barriers()` and `do_barriers()` API is suppose to encapsulate the gc policy specific requirements. `requires_barriers()` would handle any gc specific requirement for using barriers and `do_barriers()` would then take the necessary action. This makes sense as it keeps the code clean. wdyt?
>
>> > The neat thing with using the right Access API stores instead of an ad hoc barrier, is that it works for both shenandoah IU mode, and generational ZGC, while the ad hoc barrier does not.
>> 
>> @fisk are you suggesting that changing the Access API from `set_parent_raw` to `set_parent_access` is the right approach? This introduces gc policy specific code which does not look clean. It appears that the combination of `requires_barriers()` and `do_barriers()` API is suppose to encapsulate the gc policy specific requirements. `requires_barriers()` would handle any gc specific requirement for using barriers and `do_barriers()` would then take the necessary action. This makes sense as it keeps the code clean. wdyt?
> 
> That is precisely what I am suggesting. The situation is the opposite from what you think. The Access API *is* the public interface to the GC, that will do the right thing for all GCs, while the requires_barriers and do_barriers stuff is less general, as I just discussed.

> @fisk you are right, I see now that `set_parent_access<IS_DEST_UNINITIALIZED>` is a no-op for satb barriers. So we can just change `set_parent_raw` to `set_parent_access<IS_DEST_UNINITIALIZED>` for all gc policies, right?
> 
> btw `set_parent_access<decorators>` is missing in the mainline, so I would have to add this API (and the corresponding API oop::obj_field_put_access) as well. Does that make sense?

That sounds great! :)

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

PR: https://git.openjdk.org/jdk/pull/9982