On Sun, 4 May 2025 22:51:21 GMT, Luca Kellermann <d...@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. > > src/java.base/share/classes/jdk/internal/foreign/abi/CapturableState.java > line 63: > >> 61: stateLayouts[i++] = supported.layout; >> 62: } >> 63: LAYOUT = MemoryLayout.structLayout(stateLayouts); > > Does it matter that the order is not deterministic here? This question can > also be asked for the exception in `forName` and result of `displayString`. After all, this should not be an enum when the entries differ by platform. We need to convert this to records. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25025#discussion_r2072740841
