RFR: 8358535: Changes in ClassValue (JDK-8351996) caused a 1-9% regression in Renaissance-PageRank

[Chen Liang]

On Thu, 7 Aug 2025 17:19:29 GMT, Aleksey Shipilev <shade at 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