On Sun, 6 Oct 2024 14:43:09 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
>> [JDK-8240696](https://bugs.openjdk.org/browse/JDK-8240696) added the native >> method for `Reference.clear`. The original patch skipped intrinsification of >> this method, because we thought `Reference.clear` is not on a performance >> sensitive path. However, it shows up prominently on simple benchmarks that >> touch e.g. `ThreadLocal` cleanups. See the bug for an example profile with >> `RRWL` benchmarks. >> >> We need to know the actual oop strongness/weakness before we call into C2 >> Access API, this work models this after existing code for `refersTo0` >> intrinsics. C2 Access also need a support for `AS_NO_KEEPALIVE` for stores. >> >> Additional testing: >> - [x] Linux x86_64 server fastdebug, `all` >> - [x] Linux AArch64 server fastdebug, `all` > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > More precise bit-unmasks One last thing... src/hotspot/share/gc/g1/c2/g1BarrierSetC2.cpp line 342: > 340: } > 341: if ((on_weak || on_phantom) && no_keepalive) { > 342: // Be extra paranoid around this path. Only accept null stores, I think there might be some orthogonal stuff that is unnecessarily mixed up here. When no_keepalive is manually specified, then we shouldn't do the pre-write barrier, regardless of reference strength. Similarly, when the new value is null, we don't need to perform the post write barrier, regardless of reference strength. Roberto added some code in refine_barrier_by_new_val_type that already *should* take care of the latter part. It allows types to flow around a bit, and then checks if the type of the new value is provably null, and then removes the post write barrier. The existing logic for that should be strictly more powerful than the new check you added, I think. Based on the above explanation, I think I'm proposing this block is replaced with this simpler condition: if (no_keepalive) { access.set_barrier_data(access.barrier_data() & ~G1C2BarrierPre); } ------------- Changes requested by eosterlund (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20139#pullrequestreview-2351232319 PR Review Comment: https://git.openjdk.org/jdk/pull/20139#discussion_r1789742277
