On Wed, 30 Apr 2025 22:26:30 GMT, Chen Liang <li...@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.giulie...@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
