On Thu, 7 Aug 2025 17:19:29 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
>> JDK-8351996, PR #24043, updated java.lang.ClassValue, but accidentally >> missed a piece of cache refresh code that is meaningful when the cache is >> speculatively invalidated by `remove` calls. >> >> This caused a significant regression in the >> [PageRank](https://github.com/renaissance-benchmarks/renaissance/blob/master/benchmarks/apache-spark/src/main/scala/org/renaissance/apache/spark/PageRank.scala) >> benchmark, primarily because Scala array creation like `Array.fill` uses >> `scala.reflect.ClassTag` which uses `ClassValue`, making `ClassValue.get` a >> hot code path for every array creation. > > src/java.base/share/classes/java/lang/ClassValue.java line 479: > >> 477: put(classValue.identity, updated); >> 478: } >> 479: // Add to the cache, to enable the fast path, next time. > > A question: do we only care about `if (updated != entry)` path for this > cache-add then? Should this code be moved into that block? I think whenever we hit `readAccess`, it means we are missing this item in the cache and are already in this slow path. I pondered this may happen due to cache being too full too, in which case we have `updated == entry` in the backing map, so leaving it out of the if block should be correct. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26679#discussion_r2261142029
