Replace native UnixFileHandler by Java NIO to fetch/put file-info - #2925
Replace native UnixFileHandler by Java NIO to fetch/put file-info#2925HannesWell wants to merge 2 commits into
Conversation
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
|
Will try next week. |
There was a problem hiding this comment.
🟡 Changes recommended
The test bundle no longer compiles, the native-disable switch is ignored, and macOS immutable-file support is lost.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Replaces Unix JNI file-info operations with Java NIO while retaining optimized Linux directory listing.
Changes:
- Delegates POSIX metadata operations to
PosixHandler. - Removes Unix native code and obsolete platform fragments.
- Retains and versions the Linux x86_64 fast-listing fragment.
File summaries
| File | Description |
|---|---|
resources/tests/org.eclipse.core.tests.filesystem.feature/feature.xml |
Removes macOS fragment. |
resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/unix/UnixFileNatives.java |
Removes Unix JNI bridge. |
resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/unix/UnixFileHandler.java |
Removes Unix handler. |
resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/unix/UnixFileFlags.java |
Removes native constants. |
resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/unix/StructStat.java |
Removes Unix stat mapping. |
resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFileNativesManager.java |
Selects NIO or fast Linux handling. |
resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/linux/LinuxFileNatives.java |
Retains only directory-listing JNI. |
resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/linux/LinuxFileHandler.java |
Delegates metadata operations to NIO. |
resources/bundles/org.eclipse.core.filesystem/natives/unix/unixfile.h |
Removes Unix JNI declarations. |
resources/bundles/org.eclipse.core.filesystem/natives/unix/unixfile.c |
Removes Unix JNI implementation. |
resources/bundles/org.eclipse.core.filesystem/natives/unix/README.TXT |
Removes obsolete build guidance. |
resources/bundles/org.eclipse.core.filesystem/natives/unix/macosx/Makefile |
Removes macOS native build. |
resources/bundles/org.eclipse.core.filesystem/natives/unix/linux/Makefile |
Removes legacy Linux build. |
resources/bundles/org.eclipse.core.filesystem/natives/unix/fastlinux/fastlinuxfile.h |
Removes unused metadata JNI declarations. |
resources/bundles/org.eclipse.core.filesystem/natives/unix/fastlinux/fastlinuxfile.c |
Removes unused metadata JNI functions. |
resources/bundles/org.eclipse.core.filesystem/META-INF/MANIFEST.MF |
Stops exporting removed Unix package. |
resources/bundles/org.eclipse.core.filesystem.macosx/pom.xml |
Removes fragment build definition. |
resources/bundles/org.eclipse.core.filesystem.macosx/META-INF/MANIFEST.MF |
Removes fragment manifest. |
resources/bundles/org.eclipse.core.filesystem.macosx/fragment.properties |
Removes fragment metadata. |
resources/bundles/org.eclipse.core.filesystem.macosx/build.properties |
Removes packaging configuration. |
resources/bundles/org.eclipse.core.filesystem.macosx/BUILD_INFO.txt |
Removes native build record. |
resources/bundles/org.eclipse.core.filesystem.macosx/about.html |
Removes fragment legal page. |
resources/bundles/org.eclipse.core.filesystem.macosx/.settings/org.eclipse.core.runtime.prefs |
Removes project settings. |
resources/bundles/org.eclipse.core.filesystem.macosx/.settings/org.eclipse.core.resources.prefs |
Removes project encoding setting. |
resources/bundles/org.eclipse.core.filesystem.macosx/.project |
Removes Eclipse project metadata. |
resources/bundles/org.eclipse.core.filesystem.linux.x86_64/pom.xml |
Increments fragment version. |
resources/bundles/org.eclipse.core.filesystem.linux.x86_64/META-INF/MANIFEST.MF |
Increments OSGi version. |
resources/bundles/org.eclipse.core.filesystem.linux.ppc64le/pom.xml |
Removes fragment build definition. |
resources/bundles/org.eclipse.core.filesystem.linux.ppc64le/META-INF/MANIFEST.MF |
Removes fragment manifest. |
resources/bundles/org.eclipse.core.filesystem.linux.ppc64le/fragment.properties |
Removes fragment metadata. |
resources/bundles/org.eclipse.core.filesystem.linux.ppc64le/build.properties |
Removes packaging configuration. |
resources/bundles/org.eclipse.core.filesystem.linux.ppc64le/about.html |
Removes fragment legal page. |
resources/bundles/org.eclipse.core.filesystem.linux.ppc64le/.settings/org.eclipse.core.runtime.prefs |
Removes project settings. |
resources/bundles/org.eclipse.core.filesystem.linux.ppc64le/.settings/org.eclipse.core.resources.prefs |
Removes project encoding setting. |
resources/bundles/org.eclipse.core.filesystem.linux.ppc64le/.project |
Removes Eclipse project metadata. |
resources/bundles/org.eclipse.core.filesystem.linux.loongarch64/pom.xml |
Removes fragment build definition. |
resources/bundles/org.eclipse.core.filesystem.linux.loongarch64/META-INF/MANIFEST.MF |
Removes fragment manifest. |
resources/bundles/org.eclipse.core.filesystem.linux.loongarch64/fragment.properties |
Removes fragment metadata. |
resources/bundles/org.eclipse.core.filesystem.linux.loongarch64/build.properties |
Removes packaging configuration. |
resources/bundles/org.eclipse.core.filesystem.linux.loongarch64/about.html |
Removes fragment legal page. |
resources/bundles/org.eclipse.core.filesystem.linux.loongarch64/.settings/org.eclipse.core.runtime.prefs |
Removes project settings. |
resources/bundles/org.eclipse.core.filesystem.linux.loongarch64/.settings/org.eclipse.core.resources.prefs |
Removes project encoding setting. |
resources/bundles/org.eclipse.core.filesystem.linux.loongarch64/.project |
Removes Eclipse project metadata. |
resources/bundles/org.eclipse.core.filesystem.linux.aarch64/pom.xml |
Removes fragment build definition. |
resources/bundles/org.eclipse.core.filesystem.linux.aarch64/META-INF/MANIFEST.MF |
Removes fragment manifest. |
resources/bundles/org.eclipse.core.filesystem.linux.aarch64/fragment.properties |
Removes fragment metadata. |
resources/bundles/org.eclipse.core.filesystem.linux.aarch64/build.properties |
Removes packaging configuration. |
resources/bundles/org.eclipse.core.filesystem.linux.aarch64/about.html |
Removes fragment legal page. |
resources/bundles/org.eclipse.core.filesystem.linux.aarch64/.settings/org.eclipse.core.runtime.prefs |
Removes project settings. |
resources/bundles/org.eclipse.core.filesystem.linux.aarch64/.settings/org.eclipse.core.resources.prefs |
Removes project encoding setting. |
resources/bundles/org.eclipse.core.filesystem.linux.aarch64/.project |
Removes Eclipse project metadata. |
Review details
Suppressed comments (2)
resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFileNativesManager.java:52
- Changing this signature leaves
BenchFileStorecallingsetUsingNative(natives, useFastLinuxNatives), so the resources test bundle no longer compiles. Those benchmarks also still expect the removed generic Unix-native mode to be selectable; update or remove those cases together with this API change.
resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFileNativesManager.java:54 - Removing the Unix-handler fallback drops macOS support for the public
EFS.ATTRIBUTE_IMMUTABLEcapability: the old handler fetched and changedUF_IMMUTABLE/SF_IMMUTABLE, whilePosixHandlerneither advertises nor processes that attribute. This also makes the immutable assertions inBug_329836silently skip. Preserve a macOS implementation for this attribute or provide an equivalent NIO-based implementation before removing the fragment.
- Files reviewed: 51/56 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Seeing the readlink native code removed, I believe this would be a problem for this: #2798, because NIO can't recognize broken links if I remember it right. |
Thank you, no rush from my side.
Yes, in my experience (but only with smaller cases), AI agents can generate native code quite reliable. But that's for the future. This PR mostly just removes the native handler that seems not to be beneficial anymore. |
In my experiment, AI agent proposed to use different, better portable and modern Unix API (don't ask me which). jextract would probably just blindly convert JNI to FFM. But I guess AI should be able to optimize jextact generated code too. |
Test Results 51 files - 3 51 suites - 3 54m 25s ⏱️ - 4m 1s For more details on these failures, see this check. Results for commit 8665ca1. ± Comparison against base commit 31b3f6a. ♻️ This comment has been updated with latest results. |
With Java-25 the java.nio.file APIs have similar performance like the native C implementations invoked by the UnixFileHandler. In order to reduce native code and native fragments, remove the UnixFileHandler in favor of the org.eclipse.core.internal.filesystem.local.PosixHandler. The native LinuxFileHandler remains partially because the native implementation to list directory contents is still significantly faster. But its methods to fetch/put file-infos delegate to PosixHandler and the native code for that is removed. Fixes eclipse-platform#302
ec81f80 to
21fd71e
Compare
In general jextract just generates the FFM binding code for functions, structs and constants, i.e. function descriptors and memory layouts from given C header files. It avoids generating and maintaining these bindings and generates the corresponding java methods for these native functions. But the APIs are still low level and it can make sense to wrap them in 'hand-written' higher level methods.
Yes, that's at least my goal for 4.42 to replace JNI and JNA for everything than SWT (and maybe already some parts of SWT). And until now I believe that's doable. |
With Java-25 the
java.nio.fileAPIs have similar performance like the native C implementations invoked by theUnixFileHandler.In order to reduce native code and native fragments, remove the
UnixFileHandlerin favor of theorg.eclipse.core.internal.filesystem.local.PosixHandler.The native
LinuxFileHandler(and consequently theorg.eclipse.core.filesystem.linux.x86_64fragment) remains partially because the native implementation to list directory contents is still significantly faster. But its methods to fetch/put file-infos delegate toPosixHandlerand the native code for that is removed.When this is submitted, the removed fragments should be removed from the
org.eclipse.platformfeature too:Fixes #302
@iloveeclipse could you please review this? It would also be great if you could check if my clean-up of the native code was complete and not too much, since I don't have the native build pipelines set up at the moment.
At the same time I wouldn't update the native binaries since I hope that we can replace it either with FFM bindings or even some standard Java API.
Furthermore it would be great if somebody on macOS could test this change too.