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