On Thu, 28 Aug 2025 08:51:14 GMT, Darragh Clarke <dcla...@openjdk.org> wrote:
>> This PR aims to Panamize the Java Kqueue implementation, This is based on >> the work that was previously shared in >> https://github.com/openjdk/jdk/pull/22307 , The main change since then is >> that this branch takes advantage of the changes made in >> https://github.com/openjdk/jdk/pull/25043 to allow for better performance >> during errno handling. >> >> These changes feature a lot of Jextract generated files, though alterations >> have been made in relation to Errno handling and performance improvements. >> >> I will update this description soon to include performance metrics on a few >> microbenchmarks, though currently it's roughly 2% to 3% slower with the >> changes, which is somewhat expected, though there are still a few ideas of >> possible performance improvements that could be tried. Any suggestions or >> comments in that area are more than welcome however. > > Darragh Clarke 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: > > - fixed copyright header > - merged master into branch > - moved repeating code into own method > - implementing feedback, adding missing errno checks, cleanup > - feedback > - general cleanup > - small refactoring > - Performance > - implementing feedback > - removed unrelated change > - ... and 2 more: https://git.openjdk.org/jdk/compare/6924a084...cc5f558a src/java.base/macosx/classes/jdk/internal/ffi/generated/ErrnoUtils.java line 26: > 24: */ > 25: > 26: package jdk.internal.ffi.generated; Using jdk.internal.ffi.generated.** for jextract generated code is good. I'm not sure this is the right place for ErrnoUtils. src/java.base/macosx/classes/jdk/internal/ffi/generated/ErrnoUtils.java line 42: > 40: private static final long ERRNO_STRING_HOLDER_ARRAY_SIZE = 256L; > 41: > 42: public static IOException IOExceptionWithErrnoString(int errno, > String message) { The naming is a bit confusing here, minimally need a method description so we can quickly understand the relationship between "message" and strerror(errno). src/java.base/macosx/classes/sun/nio/ch/KQueue.java line 48: > 46: final class KQueue { > 47: > 48: private static final BufferStack POOL = > BufferStack.of(timespec.layout()); I think we'll need to rename this to BUFFER_STACK or something better as "POLL" is a bit confusing. src/java.base/macosx/classes/sun/nio/ch/KQueue.java line 91: > 89: */ > 90: static MemorySegment getEvent(MemorySegment memoryHandle, int i) { > 91: return kevent.asSlice(memoryHandle, i); Previous work eliminated allocating from the performance critical usages, might have to check the implications of asSlice here. src/java.base/macosx/classes/sun/nio/ch/KQueue.java line 111: > 109: // -- Native methods -- > 110: > 111: static public int register(int kqfd, int fd, int filter, int flags) { No need to make this public. src/java.base/macosx/classes/sun/nio/ch/KQueue.java line 113: > 111: static public int register(int kqfd, int fd, int filter, int flags) { > 112: int result; > 113: try (Arena arena = Arena.ofConfined()) { The register method is very performance critical (the equivalents on Linux might be called >1M/sec) so need to see the impact of creating a confined arena and allocating. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25546#discussion_r2307400147 PR Review Comment: https://git.openjdk.org/jdk/pull/25546#discussion_r2307403262 PR Review Comment: https://git.openjdk.org/jdk/pull/25546#discussion_r2307408362 PR Review Comment: https://git.openjdk.org/jdk/pull/25546#discussion_r2307411960 PR Review Comment: https://git.openjdk.org/jdk/pull/25546#discussion_r2307414172 PR Review Comment: https://git.openjdk.org/jdk/pull/25546#discussion_r2307419308
