When the build system sends an update for build targets, update the index for the files in those targets #2013
Conversation
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
|
@swift-ci Please test |
ahoppen
force-pushed
the
clean-background-index-up-to-date-checks
branch
from
e1c84fe to
d090fb0
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
We had checks for whether we need to index a file or if it is up-to-date in multiple places of `SemanticIndexManager`. Centralize them in `filesToCover(toIndex:)`, so that it’s easier to reason about them.
Indexing a header via a main file and indexing the main file itself are two different index tasks that update the `indexStoreUpToDateTracker` in different ways. We should thus consider them differently in `inProgressIndexTasks` as well.
…ndex for the files in those targets For each file in the changed targets, we still check whether it has an up-to-date unit based on timestamps. The important thing for this change is that we start indexing files for which we only receive build settings after an update from the build server.
ahoppen
force-pushed
the
clean-background-index-up-to-date-checks
branch
from
d090fb0 to
e5ede0f
Compare
hamishknight
approved these changes
Mar 3, 2025
…nce an in-progress index was requested This fixes the following problem, which caused files to get indexed twice: You open a workspace, which schedules initial indexing. While we are waiting for the build graph generation from the build server, the build servers sends `buildTarget/didChange` notification to SourceKit-LSP (eg. because it has finished loading the build graph). This causes all files in the changed targets to be scheduled for re-indexing since new files might be present in the updated targets. And since we have already started indexing of those files, we assumed that we need to index them again because the contents might have changed. To fix this, keep track of the file’s modification time at the time at which we scheduled indexing for it. If that time hasn’t changed and we have an in-progress indexing operation for it, there is no need to schedule another one.
ahoppen
force-pushed
the
clean-background-index-up-to-date-checks
branch
from
e5ede0f to
15c34bf
Compare
|
@swift-ci Please test |
bnbarham
reviewed
Mar 3, 2025
| .mainFilesContainingFile(uri: uri, crossLanguage: false) | ||
| .sorted(by: { $0.stringValue < $1.stringValue }).first | ||
| guard let mainFile else { | ||
| return nil |
There was a problem hiding this comment.
Could we log in this case (ie. we have a file that isn't known to the project)? Or did you do that and it is that too spammy?
| private func filesToIndex( | ||
| toCover files: some Collection<DocumentURI> & Sendable | ||
| toCover files: some Collection<DocumentURI> & Sendable, | ||
| indexFilesWithUpToDateUnits: Bool | ||
| ) async -> [FileToIndex] { | ||
| let sourceFiles = await orLog("Getting source files in project") { | ||
| Set(try await buildSystemManager.sourceFiles(includeNonBuildableFiles: false).keys) |
There was a problem hiding this comment.
I know this was here before, but this could be fairly large depending on the project - probably not great to reconstruct the set every time this is called.
There was a problem hiding this comment.
Good idea. Caching the set now.
|
@swift-ci Please test |
This way we don’t need to rebuild the set every time we call `filesToIndex(toCover:)`.
ahoppen
force-pushed
the
clean-background-index-up-to-date-checks
branch
from
d37a1e0 to
fe750f6
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
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.
For each file in the changed targets, we still check whether it has an up-to-date unit based on timestamps. The important thing for this change is that we start indexing files for which we only receive build settings after an update from the build server.
While doing this, I found a couple ways in which we could clean up/improve
SemanticIndexManager. These changes should be reviewable commit-by-commit.