8360025: (se) Convert kqueue Selector Implementation to use FFM APIs #25546

DarraghClarke · 2025-05-30T12:00:28Z

This PR aims to Panamize the Java Kqueue implementation, This is based on the work that was previously shared in #22307 , The main change since then is that this branch takes advantage of the changes made in #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.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

JDK-8360025: (se) Convert kqueue Selector Implementation to use FFM APIs (Enhancement - P4)

Reviewers

Per Minborg (@minborg - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25546/head:pull/25546
$ git checkout pull/25546

Update a local copy of the PR:
$ git checkout pull/25546
$ git pull https://git.openjdk.org/jdk.git pull/25546/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25546

View PR using the GUI difftool:
$ git pr show -t 25546

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25546.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-05-30T12:01:09Z

👋 Welcome back dclarke! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-05-30T12:01:29Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-05-30T12:01:59Z

@DarraghClarke The following labels will be automatically applied to this pull request:

core-libs
nio

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

liach · 2025-05-31T02:07:40Z

src/java.base/share/classes/jdk/internal/ffi/util/FFMUtils.java

+ * questions.
+ */
+
+package jdk.internal.ffi.util;


If this is some general ffm utils for using ffm, you can put it in jdk.internal.foreign.

These methods are only intended to be used for "panamization" and are not for general use. So, this is the reason we propose to have them here. But maybe some of them might be of general interest?

src/java.base/macosx/classes/sun/nio/ch/KQueue.java

src/java.base/macosx/classes/jdk/internal/ffi/generated/kqueue/kqueue_h.java

src/java.base/macosx/classes/sun/nio/ch/KQueue.java

src/java.base/macosx/classes/jdk/internal/ffi/generated/kqueue/kqueue_h.java

src/java.base/macosx/classes/sun/nio/ch/KQueue.java

minborg · 2025-06-19T08:08:10Z

src/java.base/macosx/classes/sun/nio/ch/KQueue.java

    // filters
-    static final int EVFILT_READ  = -1;
-    static final int EVFILT_WRITE = -2;
+    static final int EVFILT_READ  = kqueue_h.EVFILT_READ();


Could we remove these constants and use kqueue_h constants directly?

No, because those are private, accessible only through the public static getters.

minborg · 2025-06-19T08:19:55Z

src/java.base/macosx/classes/jdk/internal/ffi/generated/BindingUtils.java

+    public static final ValueLayout.OfDouble C_DOUBLE = ValueLayout.JAVA_DOUBLE;
+    public static final AddressLayout C_POINTER = ValueLayout.ADDRESS
+            .withTargetLayout(MemoryLayout.sequenceLayout(Long.MAX_VALUE, JAVA_BYTE));
+    public static final ValueLayout.OfLong C_LONG = ValueLayout.JAVA_LONG;


This is int on Windows. So, we could perhaps use Linker.nativeLinker().canonicalLayouts().get("long") here? "size_t" and "wchar_t" are other canonical layouts that may differ across platforms.

I think this is a good idea and would allow us to make BindingUtils shared,
Though I wonder if it's best to move BindingUtils to be shared, or if its worth merging it into FFMUtils. I'd favour the second option

minborg · 2025-06-19T08:30:57Z

Some of the files are missing a blank line at the end.

AlanBateman · 2025-06-25T10:21:01Z

src/java.base/macosx/classes/sun/nio/ch/KQueuePoller.java

    private final int filter;
    private final int maxEvents;
-    private final long address;
+    private final MemorySegment pollArrayRegions;


This is a poll array so I think very confusing to see "Regions" in the name.

AlanBateman · 2025-06-25T10:25:57Z

src/java.base/macosx/classes/sun/nio/ch/KQueue.java

+                result = kqueue_h.kevent(
+                        kqfd, keventMS, 1, NULL,
+                        0, NULL);
+            } while ((result == KQUEUE_ERROR_VALUE));


The only case where the syscall should be restarted is EINTR.

AlanBateman · 2025-06-25T10:26:29Z

src/java.base/macosx/classes/sun/nio/ch/KQueue.java

+                if (result == -errno_h.EINTR()) {
+                    return IOStatus.INTERRUPTED;
+                } else {
+                    throw new IOException("kqueue_poll failed");


The syscall is "kqueue" rather than "kqueue_poll".

mcimadamore · 2025-07-31T09:49:02Z

src/java.base/share/classes/jdk/internal/ffi/util/FFMUtils.java

+    static final SymbolLookup SYMBOL_LOOKUP = SymbolLookup.loaderLookup()
+            .or(Linker.nativeLinker().defaultLookup());
+
+    public static void traceDowncall(String name, Object... args) {


The tracing downcal support is also something that might be moved directly to the Linker -- either as a Linker option, or as a global flag, similar to Xcheck:jni (not in this PR)

mcimadamore · 2025-07-31T09:51:02Z

src/java.base/share/classes/jdk/internal/ffi/util/FFMUtils.java

+        System.out.printf("%s(%s)\n", name, traceArgs);
+    }
+
+    public static MemorySegment findOrThrow(String symbol) {


Hopefully this can go away once we update jextract to a new version that doesn't require this (not in this PR):
openjdk/jextract#280

DarraghClarke · 2025-08-19T13:20:17Z

I made a few changes mostly based on comments but I'll give a little patch notes:

I'd missed some errno checks for kqueue
Moved the FFMUtils class from shared to just be in maxosx, I also changed how this gets the valueLayouts for C values to match how newer versions of jextract generate this
I changed how the util method that converts errno codes to their respective error strings

I ran this against the selector tests on all OSs and everything seems green after repeated tests

ExE-Boss · 2025-08-19T17:22:02Z

src/java.base/macosx/classes/sun/nio/ch/KQueuePoller.java

+        int res = kqueue_h.kqueue();
+        if (res < 0) {
+            throw ErrnoUtils.IOExceptionWithErrnoString(-res,
+                    "kqueue failed");
+        }
+        this.kqfd = res;


Maybe this duplicated check should probably be moved into a common method in KQueue, like the old KQueue.create() method, but implemented in Java:

static int create() throws IOException { int res = kqueue_h.kqueue(); if (res < 0) { throw ErrnoUtils.IOExceptionWithErrnoString(-res, "kqueue failed"); } return res; }

minborg · 2025-08-27T08:44:19Z

src/java.base/macosx/classes/sun/nio/ch/KQueuePoller.java

        while (i < n) {
-            long keventAddress = KQueue.getEvent(address, i);
-            int fdVal = KQueue.getDescriptor(keventAddress);
+            MemorySegment eventMS = KQueue.getEvent(pollArray, i);


Nit: Can we call these variables eventSegment? My reptile part of the brain first read "event in milliseconds".

Yes, I think rename this to kevent or keventSegment as it's a kevent struct (which is why the Unsafe usage named it keventAddres).

minborg

Some of the methods in FFMUtils are general and should be available on all platforms. I think we can fix this when we submit the next panamization PR.

AlanBateman · 2025-08-28T13:15:05Z

src/java.base/macosx/classes/sun/nio/ch/KQueuePort.java


    // address of the poll array passed to kqueue_wait
-    private final long address;
+    private final MemorySegment pollArrayRegions;


The "Regions" suffix is confusing here. This is the contiguous poll array when invoking kqueue so I don't think "Regions" should be in the name.

AlanBateman · 2025-08-28T13:15:34Z

src/java.base/macosx/classes/sun/nio/ch/KQueueSelectorImpl.java

    @Override
    protected int doSelect(Consumer<SelectionKey> action, long timeout)
-        throws IOException
+            throws IOException


I assume you didn't mean to change this (processEvents is another).

AlanBateman · 2025-08-28T13:20:23Z

src/java.base/macosx/classes/jdk/internal/ffi/generated/ErrnoUtils.java

+ * questions.
+ */
+
+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.

AlanBateman · 2025-08-28T13:21:27Z

src/java.base/macosx/classes/jdk/internal/ffi/generated/ErrnoUtils.java

+
+    private static final long ERRNO_STRING_HOLDER_ARRAY_SIZE = 256L;
+
+    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).

AlanBateman · 2025-08-28T13:23:14Z

src/java.base/macosx/classes/sun/nio/ch/KQueue.java

+final class KQueue {

-    private static final Unsafe unsafe = Unsafe.getUnsafe();
+    private static final BufferStack POOL = BufferStack.of(timespec.layout());


I think we'll need to rename this to BUFFER_STACK or something better as "POOL" is a bit confusing.

AlanBateman · 2025-08-28T13:24:30Z

src/java.base/macosx/classes/sun/nio/ch/KQueue.java

-    static long getEvent(long address, int i) {
-        return address + (SIZEOF_KQUEUEEVENT*i);
+    static MemorySegment getEvent(MemorySegment memoryHandle, int i) {
+        return kevent.asSlice(memoryHandle, i);


Previous work eliminated allocating from the performance critical usages, might have to check the implications of asSlice here.

We could use indices/offsets directly in the methods that receive the slices from this method.

getDescriptor(MemorySegment keventArray, int index) and getFilter(MemorySegment keventArray, int index) would work for the use-sites.

The other thing here is that a fd is an int rather than a long. The re-implemented KQueue should pick ident or fd (the common usage). Right now register takes an int fd, where the modified getDescriptor is actually returning the 64-bit ident.

So on my local branch I've address all the comments except the two in this thread,

So for using the indices directly what would that look like? would we remove getEvent and instead pass the MemorySegment and whatever indices we want to access directly to getDescriptor/getFilter?

As for fd/ident, I think that's interesting, the reason for getDescriptor using long ident is because that's what jextract generated for it (from uintptr_t specifically), I see that in mainline today it's just the simpler call of return unsafe.getInt(address + OFFSET_IDENT); I could try just replicating the existing code instead of using the generated method?

I could try just replicating the existing code instead of using the generated method?

We always use the ident as an fd so don't use the full width. It doesn't matter if KQueue uses int fd or long ident in the API that it exposes but I'd prefer not to have a mix of both, at least not with good naming to avoid mis-use. If you want a preference then expose just the narrow fd form as this would work best the use-sites.

AlanBateman · 2025-08-28T13:25:14Z

src/java.base/macosx/classes/sun/nio/ch/KQueue.java

-    private static native int flagsOffset();
-
-    static native int create() throws IOException;
+    static public int register(int kqfd, int fd, int filter, int flags) {


No need to make this public.

AlanBateman · 2025-08-28T13:27:04Z

src/java.base/macosx/classes/sun/nio/ch/KQueue.java

-    static native int create() throws IOException;
+    static public int register(int kqfd, int fd, int filter, int flags) {
+        int result;
+        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. I assumed it would use BufferStack buf maybe there is a reason it is not used here?

AlanBateman · 2025-08-28T13:31:10Z

src/java.base/macosx/classes/sun/nio/ch/KQueue.java

+            kevent.flags(keventMS, (short) flags);
+            // rest default to zero
+
+            // this do-while replaces restartable


Future maintainers may ask what "replaces restartable" means, I think better to say that it calls kqueue if the syscall is interrupted (EINTR).

AlanBateman · 2025-08-28T13:58:35Z

@DarraghClarke @minborg Would it be possible to provide a brief summary on what modifications have been done to the jextract-generated classes? The use of FFMUtils jumps out. Ideally they would be checked in without modification as it makes it easy to re-generate. It might be that this never happens but there is something a bit uncomfortable about checking in modified sources.

DarraghClarke · 2025-08-28T15:02:57Z

Thanks for the thorough review, I'll address all these in the next commit.
As for the changes to generated code, I'll draft up a list of what changed and why, though FFMUtils in particular came about as trying to move reusable utility methods into a shared place to cut down on code duplication going forward

minborg · 2025-09-02T07:18:54Z

@DarraghClarke @minborg Would it be possible to provide a brief summary on what modifications have been done to the jextract-generated classes? The use of FFMUtils jumps out. Ideally they would be checked in without modification as it makes it easy to re-generate. It might be that this never happens but there is something a bit uncomfortable about checking in modified sources.

I think we could document the changes where we document how jextract is run. Ideally, we spoke about sed but maybe a more informal description can be made?

AlanBateman · 2025-09-02T07:36:01Z

I think we could document the changes where we document how jextract is run. Ideally, we spoke about sed but maybe a more informal description can be made?

That might be okay. In the distant past the in-tree copy of the zlib code needed a few patches. The list of changes, and reasoning, went into a readme file to help future sync ups. It should be very rare that we need to re-run jextract but if we do then it's important to document the patches.

DarraghClarke · 2025-09-03T12:08:49Z

So I double checked the changes made there and wrote this up, I can make it more precise or change wording if its going to be attached to a file. Package info currently contains the script used to generate the code

Common changes:

utility methods and constant values got moved into FFMUtils, this is to cut down on duplicated code as more FFM code gets added
kqueue_h and kevent both get updated to reference FFMUtils
@SuppressWarnings("restricted") is added above the class declarations

kqueue_h specific changes:

Errno handling had to be added, this involved adding the ADAPTED method handles as well as editing the HANDLE methodhandle to add Linker.Option.captureCallState(ERRNO_NAME));
Removing some unused generated objects such as static final Arena LIBRARY_ARENA = Arena.ofAuto();
added private static final String ERRNO_NAME = "errno”;

AlanBateman · 2025-09-08T06:36:13Z

So I double checked the changes made there and wrote this up, I can make it more precise or change wording if its going to be attached to a file. Package info currently contains the script used to generate the code

Common changes:

utility methods and constant values got moved into FFMUtils, this is to cut down on duplicated code as more FFM code gets added

kqueue_h and kevent both get updated to reference FFMUtils

@SuppressWarnings("restricted") is added above the class declarations

kqueue_h specific changes:

Errno handling had to be added, this involved adding the ADAPTED method handles as well as editing the HANDLE methodhandle to add Linker.Option.captureCallState(ERRNO_NAME));

Removing some unused generated objects such as static final Arena LIBRARY_ARENA = Arena.ofAuto();

added private static final String ERRNO_NAME = "errno”;

It might also help if the integration were split into two. A first commit with the jextract-generated files, then a second that has the changes. That would capture in the git log the changes that were made, and would also make things much easier to review too as it would be obvious what has been changed.

bridgekeeper · 2025-10-06T16:50:39Z

@DarraghClarke This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

DarraghClarke · 2025-10-14T10:04:59Z

Closed as this ticket is being addressed in 2 other PRs, the first of which #27590 is already integrated, and the second will be opened shortly and will be linked from here also

DarraghClarke added 3 commits May 28, 2025 14:55

panamization of kqueue

112fcbc

merge master into branch

5467408

removed unrelated change

4fd27e9

openjdk bot added nio nio-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels May 30, 2025

liach reviewed May 31, 2025

View reviewed changes

minborg reviewed Jun 2, 2025

View reviewed changes

src/java.base/macosx/classes/sun/nio/ch/KQueue.java Outdated Show resolved Hide resolved

minborg reviewed Jun 2, 2025

View reviewed changes

src/java.base/macosx/classes/jdk/internal/ffi/generated/kqueue/kqueue_h.java Outdated Show resolved Hide resolved

minborg reviewed Jun 2, 2025

View reviewed changes

src/java.base/macosx/classes/jdk/internal/ffi/generated/kqueue/kqueue_h.java Outdated Show resolved Hide resolved

minborg reviewed Jun 2, 2025

View reviewed changes

src/java.base/macosx/classes/sun/nio/ch/KQueue.java Outdated Show resolved Hide resolved

minborg reviewed Jun 2, 2025

View reviewed changes

src/java.base/macosx/classes/jdk/internal/ffi/generated/kqueue/kqueue_h.java Outdated Show resolved Hide resolved

minborg reviewed Jun 2, 2025

View reviewed changes

src/java.base/macosx/classes/sun/nio/ch/KQueue.java Outdated Show resolved Hide resolved

DarraghClarke added 3 commits June 11, 2025 15:15

implementing feedback

0410db7

Performance

109a656

small refactoring

0e39ef2

minborg reviewed Jun 19, 2025

View reviewed changes

general cleanup

8d9a47a

AlanBateman reviewed Jun 25, 2025

View reviewed changes

feedback

4878d55

DarraghClarke marked this pull request as ready for review July 8, 2025 11:04

DarraghClarke changed the title ~~WIP: Panamize KQueue selector implementation~~ JDK-8360025: (se) Convert kqueue Selector Implementation to use FFM APIs Jul 8, 2025

openjdk bot changed the title ~~JDK-8360025: (se) Convert kqueue Selector Implementation to use FFM APIs~~ 8360025: (se) Convert kqueue Selector Implementation to use FFM APIs Jul 8, 2025

openjdk bot added the rfr Pull request is ready for review label Jul 8, 2025

mcimadamore reviewed Jul 31, 2025

View reviewed changes

implementing feedback, adding missing errno checks, cleanup

e220788

ExE-Boss reviewed Aug 19, 2025

View reviewed changes

moved repeating code into own method

9de0789

minborg reviewed Aug 27, 2025

View reviewed changes

DarraghClarke added 2 commits August 27, 2025 13:48

merged master into branch

c66200c

fixed copyright header

cc5f558

minborg approved these changes Aug 28, 2025

View reviewed changes

AlanBateman reviewed Aug 28, 2025

View reviewed changes

DarraghClarke mentioned this pull request Oct 1, 2025

8368877: Generate Jextract bindings for Kqueue #27590

Closed

3 tasks

DarraghClarke closed this Oct 14, 2025

DarraghClarke mentioned this pull request Oct 14, 2025

8360025: (se) Convert kqueue Selector Implementation to use FFM APIs #27796

Open

3 tasks


		private static final long ERRNO_STRING_HOLDER_ARRAY_SIZE = 256L;

		public static IOException IOExceptionWithErrnoString(int errno, String message) {

8360025: (se) Convert kqueue Selector Implementation to use FFM APIs #25546

8360025: (se) Convert kqueue Selector Implementation to use FFM APIs #25546

Uh oh!

Conversation

DarraghClarke commented May 30, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented May 30, 2025

Uh oh!

openjdk bot commented May 30, 2025

Uh oh!

openjdk bot commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

minborg Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

minborg commented Jun 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcimadamore Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcimadamore Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DarraghClarke commented Aug 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlanBateman Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

minborg left a comment

Choose a reason for hiding this comment

Uh oh!

AlanBateman Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlanBateman Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlanBateman Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

DarraghClarke commented May 30, 2025 •

edited by openjdk bot

Loading

openjdk bot commented May 30, 2025 •

edited

Loading

minborg Jun 19, 2025 •

edited

Loading

mcimadamore Jul 31, 2025 •

edited

Loading

mcimadamore Jul 31, 2025 •

edited

Loading

AlanBateman Aug 28, 2025 •

edited

Loading

AlanBateman Aug 28, 2025 •

edited

Loading

AlanBateman Aug 28, 2025 •

edited

Loading

AlanBateman Aug 28, 2025 •

edited

Loading

DarraghClarke Sep 3, 2025 •

edited

Loading

AlanBateman Sep 3, 2025 •

edited

Loading

AlanBateman Aug 28, 2025 •

edited

Loading

DarraghClarke commented Sep 3, 2025 •

edited

Loading