RFR: 8355223: Improve documentation on @IntrinsicCandidate [v6]

[John R Rose]

On Wed, 30 Apr 2025 22:26:30 GMT, Chen Liang <liach at openjdk.org> wrote:

>> In offline discussion, we noted that the documentation on this annotation does not recommend minimizing the intrinsified section and moving whatever can be done in Java to Java; thus I prepared this documentation update, to shrink a "TLDR" essay to something concise for readers, such as pointing to that list at `vmIntrinsics.hpp` instead of "a list".
>
> 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 eight additional commits since the last revision:
> 
>  - Move intrinsic to be a subsection; just one most common function of the annotation
>  - Merge branch 'master' of https://github.com/openjdk/jdk into doc/intrinsic-candidate
>  - Merge branch 'master' of https://github.com/openjdk/jdk into doc/intrinsic-candidate
>  - Update src/java.base/share/classes/jdk/internal/vm/annotation/IntrinsicCandidate.java
>    
>    Co-authored-by: Raffaello Giulietti <raffaello.giulietti at oracle.com>
>  - Shorter first sentence
>  - Updates, thanks to John
>  - Refine validation and defensive copying
>  - 8355223: Improve documentation on @IntrinsicCandidate

Marked as reviewed by jrose (Reviewer).

src/java.base/share/classes/jdk/internal/vm/annotation/IntrinsicCandidate.java line 38:

> 36:  *
> 37:  * <h2 id="intrinsification">Intrinsification</h2>
> 38:  * The most frequently special treatment is intrinsification, which replaces a

> The most frequently special treatment is intrinsification, which

Better:

> Most frequently, the special treatment of an intrinsic is <em>intrinsification</em>, which

src/java.base/share/classes/jdk/internal/vm/annotation/IntrinsicCandidate.java line 47:

> 45:  * intrinsics necessary.
> 46:  * <p>
> 47:  * Intrinsification may never happen, or happen at any moment during execution.

s/or happen/or may happen/ (easier to parse)

src/java.base/share/classes/jdk/internal/vm/annotation/IntrinsicCandidate.java line 53:

> 51:  * intrinsic implementors must ensure that non-bytecode execution has the same
> 52:  * results as execution of the actual Java code in all application contexts
> 53:  * (given the assumptions on the arguments hold).

s/given the/given that the/

src/java.base/share/classes/jdk/internal/vm/annotation/IntrinsicCandidate.java line 57:

> 55:  * A candidate method should contain a minimal piece of Java code that should be
> 56:  * replaced by an intrinsic wholesale.  The backing intrinsic is (in the common
> 57:  * case) <strong>unsafe</strong> - they do not perform checks, but have

s/they do not perform/it may omit safety/
s/but have/but instead makes/
s/their/its/
s/that can ensure type safety/that type safety is ensured elsewhere/

src/java.base/share/classes/jdk/internal/vm/annotation/IntrinsicCandidate.java line 67:

> 65:  * accessing a field or method on an object which does not possess that field or
> 66:  * method; accessing an element of an array not actually present in the array;
> 67:  * and manipulating managed references in a way that prevents the GC from

? s/managed references/object references/  ("manage" is mentioned a moment later)

src/java.base/share/classes/jdk/internal/vm/annotation/IntrinsicCandidate.java line 90:

> 88:  * intrinsic.)  For example, the documentation can simply say that the result is
> 89:  * undefined if a race happens.  However, race conditions must not lead to
> 90:  * program failures or type safety breaches, as listed above.

Maybe add a teaching paragraph:

> Reasoning about such race conditions is difficult, but it is a necessary skill when working with intrinsics that can observe racing shared variables.  One example of a tolerable race is a repeated read of a shared reference.  This only works if the algorithm takes no action based on the first read, other than deciding to perform the second read; it must "forget what it saw" in the first read.  This is why the array-mismatch intrinsics can sometimes report a tentative search hit (maybe using vectorized code), which can then be confirmed (by scalar code) as the caller makes a fresh and independent observation. 

(This is done when the array mismatch logic performs NaN-folding.  I just noticed that the NaN-folding code in ArraysSupport is slightly incorrect with respect to races!)

-------------

PR Review: https://git.openjdk.org/jdk/pull/24777#pullrequestreview-2847502871
PR Review Comment: https://git.openjdk.org/jdk/pull/24777#discussion_r2093599254
PR Review Comment: https://git.openjdk.org/jdk/pull/24777#discussion_r2093605455
PR Review Comment: https://git.openjdk.org/jdk/pull/24777#discussion_r2093605395
PR Review Comment: https://git.openjdk.org/jdk/pull/24777#discussion_r2093605356
PR Review Comment: https://git.openjdk.org/jdk/pull/24777#discussion_r2093605323
PR Review Comment: https://git.openjdk.org/jdk/pull/24777#discussion_r2093632716