On Tue, 6 May 2025 20:07:56 GMT, Chen Liang <li...@openjdk.org> wrote:
>> Credit to @lukellmann that the duplication arg handling in #24742 avoided >> throwing exceptions but produced a wrong option. This patch fixes that and >> removed stream usages in CaptureCallState to speed up bootstrap. >> >> Also, the previous patch affected the toString display of the option; I >> added a unit test to ensure the option prints names that is user-friendly. >> >> Another thing I noted is `CapturableState` uses `OperatingSystem`; using >> `valueOf` brings a performance overhead due to setups with reflection, so I >> made this lazy. (The enum is thread safe, so we allow racy access to the >> cache field) >> >> Testing: jdk/lang/foreign, tier 1-3 in progress. > > 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 12 additional commits since > the last revision: > > - Simplify to utility class > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/ffm-opt-capture-states > - Update > src/java.base/share/classes/jdk/internal/foreign/abi/CapturableState.java > > Co-authored-by: Shaojin Wen <shaojin.we...@alibaba-inc.com> > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/ffm-opt-capture-states > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/ffm-opt-capture-states > - rollback OS > - Review remarks > - No env to test > > - Unchecked > > - Further revamp, remove unnecessary instances on irrelevant platforms > > - ... and 2 more: https://git.openjdk.org/jdk/compare/479659a2...0f495539 Changes look good src/java.base/share/classes/jdk/internal/foreign/abi/CapturableState.java line 62: > 60: "errno", 1 << 2 > 61: ); > 62: } Pre-existing, but while you're here, could you add a comment stating that the mask values have to be kept in sync with the value in `DowncallLinker::capture_state` in `src/hotspot/share/prims/downcallLinker.cpp`? (See https://github.com/openjdk/jdk/blob/master/src/hotspot/share/prims/downcallLinker.cpp#L36) ------------- Marked as reviewed by jvernee (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/25025#pullrequestreview-2819760597 PR Review Comment: https://git.openjdk.org/jdk/pull/25025#discussion_r2076367601
