RFR: 8351996: Behavioral updates for ClassValue::remove [v13]

Chen Liang liach at openjdk.org
Tue May 13 13:34:41 UTC 2025


On Tue, 13 May 2025 12:47:12 GMT, Viktor Klang <vklang at openjdk.org> wrote:

>> Chen Liang 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 22 additional commits since the last revision:
>> 
>>  - Jaikiran comments, improve thread reference tracking, more documents
>>  - Merge branch 'master' of https://github.com/openjdk/jdk into fix/classvalue-compute-remove
>>  - Update src/java.base/share/classes/java/lang/ClassValue.java
>>    
>>    Co-authored-by: Shaojin Wen <shaojin.wensj at alibaba-inc.com>
>>  - Merge branch 'master' of https://github.com/openjdk/jdk into fix/classvalue-compute-remove
>>  - Reviews from viktor
>>  - Rewrite impl to follow the new simplified spec
>>  - Merge branch 'master' of https://github.com/openjdk/jdk into fix/classvalue-compute-remove
>>  - Try to simplify the model - use the finish of computeValue
>>    
>>  - Test updates - remove repetition, test case for no stale installation
>>  - Fix incorrect promise removal when unnecessary and add regression test
>>  - ... and 12 more: https://git.openjdk.org/jdk/compare/f1e84a54...e24a1fd7
>
> src/java.base/share/classes/java/lang/ClassValue.java line 99:
> 
>> 97:      * that computed the value to associate.  A new invocation to {@code
>> 98:      * computeValue}, which that {@code remove} call happens-before, happens to
>> 99:      * re-establish this happens-before relationship.</li>
> 
> Suggestion:
> 
>      * computeValue}, which that {@code remove} call happens-before,
>      * re-establishes this happens-before relationship.</li>

I think `can re-establish` or `will re-establish` is better just because this happens after the outcome. What do you think?

> src/java.base/share/classes/java/lang/ClassValue.java line 348:
> 
>> 346: 
>> 347:         // Arguments are nullable, intentionally to allow initial tokens
>> 348:         static boolean areCompatible(RemovalToken current, RemovalToken original) {
> 
> Are "equivalent"?

Not really: if you have an original token and a current token created during the computeValue of the original token, they are compatible but not really "equivalent" (i.e. the threadId case)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24043#discussion_r2086833475
PR Review Comment: https://git.openjdk.org/jdk/pull/24043#discussion_r2086837740


More information about the core-libs-dev mailing list