On Thu, 31 Jul 2025 09:45:10 GMT, Maurizio Cimadamore <mcimadam...@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. > > src/java.base/share/classes/jdk/internal/ffi/util/FFMUtils.java line 51: > >> 49: public static final AddressLayout C_POINTER = ValueLayout.ADDRESS >> 50: >> .withTargetLayout(MemoryLayout.sequenceLayout(Long.MAX_VALUE, JAVA_BYTE)); >> 51: public static final ValueLayout.OfLong C_LONG = (ValueLayout.OfLong) >> Linker.nativeLinker().canonicalLayouts().get("long"); > > This seems problematic. On Windows, the C type `long` has a `JAVA_INT` > layout, because it only uses 32 bits. The reason jextract re-generates all > these constants is that they can, in general, vary by platform, so my general > feeling is that we can't just have a shared set of layout constants at these > level -- the constants will need to be added in the platform-specific > directories. If you want to make this layer more portable, then the `C_LONG` > constant should be dropped, and clients should use either `C_INT` or > `C_LONG_LONG` (which is what most portable C APIs end up doing anyway). > > Another possible way to have a more robust shared layer is to use definitions > in stdint.h -- e.g. not `C_INT`, but `C_INT32_T` (but then you might have > issues, as the jextract-generated files you are importing depend on names > like `C_INT`). IMHO the more honest approach is to move these constants closer to the _h generated files that use them, as that's the way jextract intends them to be used. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25546#discussion_r2244894284
