On Fri, 9 May 2025 15:59:38 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Currently there is intrinsic support by the interpreter, C1, C2, and graal, >> which are always used. With this change we can later remove all the >> per-platform interpreter intrinsic implementations, and might also remove the >> C1 intrinsic implementation. >> >> Testing: >> (1) mach5 tier1-6 normal (so using all the existing intrinsics). >> (2) mach5 tier1-6 with interpreter and C1 Reference::get intrinsics disabled. > > Kim Barrett 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 10 additional > commits since the last revision: > > - Merge branch 'master' into native-reference-get > - use new waitForRefProc, some tidying > - Merge branch 'master' into native-reference-get > - remove timeout by using waitForReferenceProcessing > - make ill-timed gc in non-concurrent case less likely > - fix test package use > - add package decl to test > - parameterized return type of native get0 > - test native method > - native Reference.get helper Overall, looks good. src/java.base/share/classes/java/lang/ref/Reference.java line 366: > 364: > 365: /* Implementation of unintrinsified get(). Making get() native may > lead > 366: * C2 to sometimes prefer the native implementation over the > intrinsic. Can you share more details on this? ------------- Marked as reviewed by vlivanov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24315#pullrequestreview-2855249526 PR Review Comment: https://git.openjdk.org/jdk/pull/24315#discussion_r2098645006