Speed up FileSystemResourceManager.resourceForLocation() - #2964
Merged
fedejeanne merged 2 commits intoSep 24, 2026
Merged
Conversation
Check the cheap segment counts before the more expensive IPath.isPrefixOf() and stop iterating over the projects as soon as a project location equal to the given location has been matched, since no other project can provide a better match. Add tests covering project locations, nested projects in both iteration orders, filtered resources in nested projects, hidden projects and project names being plain string prefixes of each other. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Both changed plug-ins require service-version updates before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Optimizes filesystem location lookup while preserving existing behavior.
Changes:
- Reorders segment checks and exits after an exact match.
- Adds regression coverage for location-resolution scenarios.
| File | Review |
|---|---|
resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/IWorkspaceRootTest.java |
Adds regression tests; requires a test plug-in Bundle-Version update. |
resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/localstore/FileSystemResourceManager.java |
Optimizes project matching; requires an implementation bundle Bundle-Version update. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Let it match the current version of 'org.eclipse.core.resources', according to /org.eclipse.platform.docs/VersionNumbering.md (Line 77): "In particular, a source/test/documentation plug-in that has changes in the current stream should evolve its version number in sync with the plug-in(s) it is providing source/test/documentation for."
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.
resourceForLocation(IPath, boolean)loops over all projects. This change only reorders side-effect-free checks inside that loop and stops the loop once no better match is possible. It returns exactly what it returned before.What changed
IPath.isPrefixOf():segmentsToRemove <= resultProjectPathSegmentswas already checked before, just afterisPrefixOf().segmentsToRemove > location.segmentCount()is new, but in that caseisPrefixOf()returnsfalseanyway.locationhas been accepted. No other project can match more segments, and the loop only replaces a match with a strictly longer one.No caching, no API change.
Tests
Existing coverage:
FileSystemResourceManagerTest.testContainerFor/testFileForIWorkspaceRootTest.testGetContainerForLocation/testGetFileForLocation/testBug476585FilteredResourceTest.test343914New tests in
IWorkspaceRootTestcover:p/p2)The new tests pass against both the old and the new implementation.
🤖 Generated with Claude Code