On Mon, 5 May 2025 16:44:02 GMT, Chen Liang <[email protected]> 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 incrementally with one additional
> commit since the last revision:
>
> Review remarks
src/java.base/share/classes/jdk/internal/foreign/abi/CapturableState.java line
81:
> 79:
> 80: public static CapturableState forName(String name) {
> 81: var ret = LOOKUP.get(name);
As we usually have just one element (and for Windows three elements) it might
be both less memory consuming and faster to just search a `@Stable` array
linearly rather than having a Map.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25025#discussion_r2075308234
