On Tue, 13 May 2025 12:47:12 GMT, Viktor Klang <vkl...@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.we...@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

Reply via email to