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