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

Reply via email to