On Tue, 14 Oct 2025 10:53:51 GMT, Per Minborg <[email protected]> wrote:

>> This PR was originally part of #25546, though that PR has been split in 2, 
>> the first chunk was https://github.com/openjdk/jdk/pull/27590.
>> 
>> This PR aims to convert KQueue to use FFM apis, the first PR in this area 
>> added all the jextract generated code needed, while this PR modifies some of 
>> the jextract code and then uses it with Kqueue.
>> 
>> A brief rundown of the changes:
>> - The files `errno_h$shared.java` , `kqueue_h$shared.java`, 
>> `timespec_h$shared.java` and `timespec_h.java` have all been deleted. This 
>> is because they all contained shared elements that could be moved into the 
>> Utility file `FFMUtils.java`
>> - `Kqueue.c` has been deleted, and all native methods in the other `KQueue` 
>> files have been replaced with references to the generated files kevent and 
>> kqueue. This is the bulk of the changes
>> - Both the `Kqueue()` and `Kevent()` methods in `kqueue_h.java`  were 
>> modified to use adapted method handles that will return the errno value
>
> src/java.base/macosx/classes/jdk/internal/ffi/generated/kqueue/kqueue_h.java 
> line 45:
> 
>> 43:     }
>> 44: 
>> 45:     static final SymbolLookup SYMBOL_LOOKUP = SymbolLookup.loaderLookup()
> 
> Can we remove this static field?

You're right, i'll remove it

> src/java.base/macosx/classes/jdk/internal/ffi/generated/kqueue/kqueue_h.java 
> line 152:
> 
>> 150: 
>> 151:         public static final MemorySegment ADDR = 
>> FFMUtils.findOrThrow("kevent");
>> 152:         public static final MethodHandle HANDLE = 
>> Linker.nativeLinker().downcallHandle(ADDR, DESC,
> 
> I would suspect we do not use `ADDR` and `HANDLE` so maybe we do not have to 
> store them?

The only place that uses them is `ADAPTED` directly below them, so it could get 
moved into that instead of being stored, though it might be a slight hit to 
readability.

edit: I see theres also getters for them, though I don't think those are used 
anywhere

> src/java.base/macosx/classes/jdk/internal/ffi/util/FFMUtils.java line 39:
> 
>> 37: public final class FFMUtils {
>> 38: 
>> 39:     public static final ValueLayout.OfBoolean C_BOOL =
> 
> This class is specific for macOS but yet, it appears it would work for any 
> platform (given its use of canonical layouts)?

Yeah I think with minimal changes (maybe none?) this could be moved to shared, 
though I don't know if we want to keep this current PR as minimal in scope as 
possible. I played it safe with just putting it in Mac for now but I'm happy 
for it to move

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/27796#discussion_r2429140759
PR Review Comment: https://git.openjdk.org/jdk/pull/27796#discussion_r2429142232
PR Review Comment: https://git.openjdk.org/jdk/pull/27796#discussion_r2429157101

Reply via email to