On Tue, 13 Jun 2023 19:42:44 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
>> Man Cao 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 two additional commits >> since the last revision: >> >> - Merge branch 'master' into JDK8309688 >> - 8309688: Data race on java.io.ClassCache.strongReferent > > I think this data race is benign, because `ClassValue` gives us the required > memory semantics. Introducing additional performance hogs seems weird in this > case, see below. > > Since TSAN complains about the unsynchronized conflicting accesses in > `getStrong()` and `clearStrong()`, maybe the better solution would be marking > `strong` as `volatile`? It would still be awkward, but we would "only" pay a > price of volatile load on a hot path. Thank you @shipilev! Good point about the cost of extra AtomicReference instance, and slowing down the fast path with a CAS. Agree that making `strongReferent` `volatile` is better and simpler. For `AtomicReferenceFieldUpdater` or `VarHandle`, they are more complicated and I don't see much benefit over a simple `volatile` in this case. ------------- PR Comment: https://git.openjdk.org/jdk/pull/14386#issuecomment-1590038988
