RFR: 8356126: Duplication handling and optimization of CaptureCallState [v8]

Jorn Vernee jvernee at openjdk.org
Tue May 6 21:49:19 UTC 2025


On Tue, 6 May 2025 20:07:56 GMT, Chen Liang <liach at 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.wensj at 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


More information about the core-libs-dev mailing list