On Tue, 26 Nov 2024 15:04:51 GMT, Per Minborg <[email protected]> wrote:
> Going forward, converting older JDK code to use the relatively new FFM API
> requires system calls that can provide `errno` and the likes to explicitly
> allocate a MemorySegment to capture potential error states. This can lead to
> negative performance implications if not designed carefully and also
> introduces unnecessary code complexity.
>
> Hence, this PR proposes to add a _JDK internal_ method handle adapter that
> can be used to handle system calls with `errno`, `GetLastError`, and
> `WSAGetLastError`.
>
> It currently relies on a thread-local cache of MemorySegments to allide
> allocations. If, in the future, a more efficient thread-associated allocation
> scheme becomes available, we could easily migrate to that one.
>
> Tested and passed tiers 1-3.
Since the `returnFilter(…)` methods are only ever invoked reflectively by
method handle combinators, they can use `throws Throwable`, which avoids
possible double‑wrapping of `Error`s (`StackOverflowError` or
`OutOfMemoryError`) and `RuntimeException`s:
src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java line 171:
> 169: throw new InternalError(t);
> 170: }
> 171: }
Suggestion:
private static int returnFilter(MethodHandle errorHandle, int result)
throws Throwable {
if (result >= 0) {
return result;
}
return -(int) errorHandle.invoke();
}
src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java line 183:
> 181: throw new InternalError(t);
> 182: }
> 183: }
Suggestion:
private static long returnFilter(MethodHandle errorHandle, long result)
throws Throwable {
if (result >= 0) {
return result;
}
return -(int) errorHandle.invoke();
}
src/java.base/share/classes/jdk/internal/foreign/ErrnoUtil.java line 122:
> 120: }
> 121:
> 122: // Used reflectively via RETURN_FILTER_MH
Suggestion:
// Used reflectively via INT_RETURN_FILTER_MH
src/java.base/share/classes/jdk/internal/foreign/ErrnoUtil.java line 127:
> 125: }
> 126:
> 127: // Used reflectively via RETURN_FILTER_MH
Suggestion:
// Used reflectively via LONG_RETURN_FILTER_MH
src/java.base/share/classes/jdk/internal/foreign/ResultErrno.java line 30:
> 28: *
> 29: * @param result the result returned from the native call
> 30: * @param errno the errno (if result <= 0) or 0
Suggestion:
* @param errno the errno (if result < 0) or 0
-------------
PR Review: https://git.openjdk.org/jdk/pull/22391#pullrequestreview-2465893687
PR Review Comment: https://git.openjdk.org/jdk/pull/22391#discussion_r1861206877
PR Review Comment: https://git.openjdk.org/jdk/pull/22391#discussion_r1861207179
PR Review Comment: https://git.openjdk.org/jdk/pull/22391#discussion_r1860280069
PR Review Comment: https://git.openjdk.org/jdk/pull/22391#discussion_r1860280510
PR Review Comment: https://git.openjdk.org/jdk/pull/22391#discussion_r1859646876
