RFR: 8356126: Duplication handling and optimization of CaptureCallState [v5]
Shaojin Wen
swen at openjdk.org
Tue May 6 09:38:15 UTC 2025
On Mon, 5 May 2025 16:44:02 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 incrementally with one additional commit since the last revision:
>
> Review remarks
src/java.base/share/classes/jdk/internal/foreign/abi/CapturableState.java line 59:
> 57: MemoryLayout[] stateLayouts = new MemoryLayout[supported.size()];
> 58: @SuppressWarnings({"unchecked", "rawtypes"})
> 59: Map.Entry<String, CapturableState>[] entries = new Map.Entry[supported.size()];
Suggestion:
CapturableState[] supported;
if (OperatingSystem.isWindows()) {
supported = new CapturableState[] {
new CapturableState("GetLastError", JAVA_INT, 1 << 0),
new CapturableState("WSAGetLastError", JAVA_INT, 1 << 1),
new CapturableState("errno", JAVA_INT, 1 << 2)
};
} else {
supported = new CapturableState[] {new CapturableState("errno", JAVA_INT, 1 << 2)};
}
MemoryLayout[] stateLayouts = new MemoryLayout[supported.length];
@SuppressWarnings({"unchecked", "rawtypes"})
Map.Entry<String, CapturableState>[] entries = new Map.Entry[supported.length];
Here it might be better to replace List with an array
src/java.base/share/classes/jdk/internal/foreign/abi/CapturableState.java line 84:
> 82: if (ret == null) {
> 83: throw new IllegalArgumentException(
> 84: "Unknown name: " + name +", must be one of: "
Suggestion:
"Unknown name: " + name + ", must be one of: "
space
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25025#discussion_r2075096871
PR Review Comment: https://git.openjdk.org/jdk/pull/25025#discussion_r2075097771
More information about the core-libs-dev
mailing list