On Thu, 15 Jun 2023 08:23:58 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 five additional commits >> since the last revision: >> >> - Merge branch 'master' into JDK8309688 >> - Removed volatile and comment about the benign race >> - Switch to using volatile instead of AtomicReference >> - Merge branch 'master' into JDK8309688 >> - 8309688: Data race on java.io.ClassCache.strongReferent > > src/java.base/share/classes/java/io/ClassCache.java line 93: > >> 91: // multiple threads calls get() with the same Class >> parameter. >> 92: // Fixing this race could introduce noticeable performance >> penalty. >> 93: // See the review thread for JDK-8309688 for details. > > Feels like this comment belongs near `strongReferent` declaration. I suggest: > > > // This field is deliberately accessed without sychronization. ClassValue > // provides synchronization when CacheRef is published. However, when > // a thread reads this field, while another thread is clearing the field, it > // would formally constitute a data race. But that data race is benign, and > // fixing it could introduce noticeable performance penalty, see JDK-8309688. > private T strongReferent; Thank you. Updated. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14386#discussion_r1231381928
