[DRAFT] Introduce faster (batched) LocalFile.childInfos() version#2779
Open
iloveeclipse wants to merge 2 commits into
Open
[DRAFT] Introduce faster (batched) LocalFile.childInfos() version#2779iloveeclipse wants to merge 2 commits into
iloveeclipse wants to merge 2 commits into
Conversation
Contributor
|
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. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a faster directory-children enumeration path for LocalFile by adding JNI-backed directory listing APIs on Unix and wiring LocalFile.childInfos(...) / childNames(...) to use the new native handler entrypoints (intended to significantly speed up NFS access on Linux).
Changes:
- Add
NativeHandler/LocalFileNativesManagerAPIs for listing directory entry names and for listing + fetchingIFileInfoin one pass. - Implement Unix-native
listDir(...)andlistDirAndGetFileInfos(...)JNI methods and extendStructStatto carry per-entryerrno,name, and symlink target. - Override
LocalFile.childInfos(...)to use the new batched native-backed listing path.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/unix/UnixFileNatives.java | Adds Java wrappers for new JNI directory listing methods. |
| resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/unix/UnixFileHandler.java | Exposes the new listing methods via the Unix native handler. |
| resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/unix/StructStat.java | Adds fields to carry name/errno/symlink target and adapts toFileInfo(). |
| resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/NativeHandler.java | Adds default (non-native) implementations for the new listing APIs. |
| resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFileNativesManager.java | Adds manager-level entrypoints to call the new listing APIs. |
| resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java | Overrides childInfos(...) and routes both childInfos/childNames via the manager. |
| resources/bundles/org.eclipse.core.filesystem/natives/unix/unixfile.h | Updates native signatures and declares new JNI functions. |
| resources/bundles/org.eclipse.core.filesystem/natives/unix/unixfile.c | Implements listDir / listDirAndGetFileInfos and updates stat calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+436
to
+440
| /* Add the directory entry name to the names array */ | ||
| nameString = (*env)->NewStringUTF(env, entry->d_name); | ||
| if (nameString == NULL) { | ||
| goto cleanup; | ||
| } |
Comment on lines
+578
to
+580
| } else if (statErrno == 0) { | ||
| statErrno = errno; | ||
| } |
Comment on lines
+40
to
+43
| if (errno != 0 && errno != ENOENT) { | ||
| info.setError(IFileInfo.IO_ERROR); | ||
| return info; | ||
| } |
Comment on lines
+611
to
+613
| nameString = (*env)->NewStringUTF(env, entry->d_name); | ||
| if (nameString == NULL) { | ||
| (*env)->DeleteLocalRef(env, statObject); |
Comment on lines
+582
to
+585
| if (linkPathLen >= 0) { | ||
| linkString = (*env)->NewStringUTF(env, linkPath); | ||
| } else { | ||
| linkString = (*env)->NewStringUTF(env, ""); |
The code is supposed to speed up NFS file access on Linux by an order of magnitude by fetching directory children data (IFileInfo) in one shot from JNI (via array of StructStat). LocalFile.childInfos(int, IProgressMonitor) overrides now the FileStore.childInfos(int, IProgressMonitor) on Linux with an optimized version. No idea how to build binary for Mac without Mac.
iloveeclipse
force-pushed
the
faster_refresh
branch
from
c33e9e2 to
d5fc213
Compare
Comment on lines
+575
to
+580
| linkPathLen = readlinkat(directoryFd, entry->d_name, linkPath, PATH_MAX); | ||
| if (linkPathLen >= 0) { | ||
| linkPath[linkPathLen] = '\0'; | ||
| } else if (statErrno == 0) { | ||
| statErrno = errno; | ||
| } |
| info = stat.toFileInfo(); | ||
| } else { | ||
| if (lstat(name, stat) == 0) { // return information about the link itself if the file is a symbolic link | ||
| if ((stat.st_mode & UnixFileFlags.S_IFMT) == UnixFileFlags.S_IFLNK) { // it a link! |
Comment on lines
39
to
+43
| FileInfo info = new FileInfo(); | ||
| info.setExists(true); | ||
| if (errno != 0 && errno != ENOENT) { | ||
| info.setError(IFileInfo.IO_ERROR); | ||
| return info; | ||
| } |
Contributor
Test Results 51 files - 3 51 suites - 3 1h 1m 30s ⏱️ + 7m 45s For more details on these failures and errors, see this check. Results for commit e913952. ± Comparison against base commit c45a7a7. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The code is supposed to speed up NFS file access on Linux by an order of magnitude by fetching directory children data (IFileInfo) in one shot from JNI (via array of StructStat).
LocalFile.childInfos(int, IProgressMonitor) overrides now the FileStore.childInfos(int, IProgressMonitor) on Linux with an optimized version.
No idea how to build binary for Mac without Mac.