Skip to content

Address review comments from #1322 and #1329 #1349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/SemanticIndex/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
add_library(SemanticIndex STATIC
CheckedIndex.swift
CompilerCommandLineOption.swift
IndexStatusManager.swift
IndexTaskDescription.swift
PreparationTaskDescription.swift
SemanticIndexManager.swift
TestHooks.swift
UpdateIndexStoreTaskDescription.swift
UpToDateTracker.swift
)
set_target_properties(SemanticIndex PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_Swift_MODULE_DIRECTORY})
Expand Down
10 changes: 5 additions & 5 deletions Sources/SemanticIndex/PreparationTaskDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public struct PreparationTaskDescription: IndexTaskDescription {
/// The build system manager that is used to get the toolchain and build settings for the files to index.
private let buildSystemManager: BuildSystemManager

private let preparationUpToDateStatus: IndexUpToDateStatusManager<ConfiguredTarget>
private let preparationUpToDateTracker: UpToDateTracker<ConfiguredTarget>

/// See `SemanticIndexManager.indexProcessDidProduceResult`
private let indexProcessDidProduceResult: @Sendable (IndexProcessResult) -> Void
Expand All @@ -59,13 +59,13 @@ public struct PreparationTaskDescription: IndexTaskDescription {
init(
targetsToPrepare: [ConfiguredTarget],
buildSystemManager: BuildSystemManager,
preparationUpToDateStatus: IndexUpToDateStatusManager<ConfiguredTarget>,
preparationUpToDateTracker: UpToDateTracker<ConfiguredTarget>,
indexProcessDidProduceResult: @escaping @Sendable (IndexProcessResult) -> Void,
testHooks: IndexTestHooks
) {
self.targetsToPrepare = targetsToPrepare
self.buildSystemManager = buildSystemManager
self.preparationUpToDateStatus = preparationUpToDateStatus
self.preparationUpToDateTracker = preparationUpToDateTracker
self.indexProcessDidProduceResult = indexProcessDidProduceResult
self.testHooks = testHooks
}
Expand All @@ -76,7 +76,7 @@ public struct PreparationTaskDescription: IndexTaskDescription {
// The last 2 digits should be sufficient to differentiate between multiple concurrently running preparation operations
await withLoggingSubsystemAndScope(subsystem: indexLoggingSubsystem, scope: "preparation-\(id % 100)") {
let targetsToPrepare = await targetsToPrepare.asyncFilter {
await !preparationUpToDateStatus.isUpToDate($0)
await !preparationUpToDateTracker.isUpToDate($0)
}.sorted(by: {
($0.targetID, $0.runDestinationID) < ($1.targetID, $1.runDestinationID)
})
Expand Down Expand Up @@ -114,7 +114,7 @@ public struct PreparationTaskDescription: IndexTaskDescription {
}
await testHooks.preparationTaskDidFinish?(self)
if !Task.isCancelled {
await preparationUpToDateStatus.markUpToDate(targetsToPrepare, updateOperationStartDate: startDate)
await preparationUpToDateTracker.markUpToDate(targetsToPrepare, updateOperationStartDate: startDate)
}
}
}
Expand Down
20 changes: 10 additions & 10 deletions Sources/SemanticIndex/SemanticIndexManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ public final actor SemanticIndexManager {
/// ...). `nil` if no build graph is currently being generated.
private var generateBuildGraphTask: Task<Void, Never>?

private let preparationUpToDateStatus = IndexUpToDateStatusManager<ConfiguredTarget>()
private let preparationUpToDateTracker = UpToDateTracker<ConfiguredTarget>()

private let indexStoreUpToDateStatus = IndexUpToDateStatusManager<DocumentURI>()
private let indexStoreUpToDateTracker = UpToDateTracker<DocumentURI>()

/// The preparation tasks that have been started and are either scheduled in the task scheduler or currently
/// executing.
Expand Down Expand Up @@ -273,23 +273,23 @@ public final actor SemanticIndexManager {
// We only re-index the files that were changed and don't re-index any of their dependencies. See the
// `Documentation/Files_To_Reindex.md` file.
let changedFiles = events.map(\.uri)
await indexStoreUpToDateStatus.markOutOfDate(changedFiles)
await indexStoreUpToDateTracker.markOutOfDate(changedFiles)

// Note that configured targets are the right abstraction layer here (instead of a non-configured target) because a
// build system might have targets that include different source files. Hence a source file might be in target T
// configured for macOS but not in target T configured for iOS.
let targets = await changedFiles.asyncMap { await buildSystemManager.configuredTargets(for: $0) }.flatMap { $0 }
if let dependentTargets = await buildSystemManager.targets(dependingOn: targets) {
await preparationUpToDateStatus.markOutOfDate(dependentTargets)
await preparationUpToDateTracker.markOutOfDate(dependentTargets)
} else {
await preparationUpToDateStatus.markAllOutOfDate()
await preparationUpToDateTracker.markAllKnownOutOfDate()
// `markAllOutOfDate` only marks targets out-of-date that have been indexed before. Also mark all targets with
// in-progress preparation out of date. So we don't get into the following situation, which would result in an
// incorrect up-to-date status of a target
// - Target preparation starts for the first time
// - Files changed
// - Target preparation finishes.
await preparationUpToDateStatus.markOutOfDate(inProgressPreparationTasks.keys)
await preparationUpToDateTracker.markOutOfDate(inProgressPreparationTasks.keys)
}

await scheduleBackgroundIndex(files: changedFiles)
Expand Down Expand Up @@ -367,7 +367,7 @@ public final actor SemanticIndexManager {
// schedule two preparations of the same target in quick succession, only the first one actually performs a prepare
// and the second one will be a no-op once it runs.
let targetsToPrepare = await targets.asyncFilter {
await !preparationUpToDateStatus.isUpToDate($0)
await !preparationUpToDateTracker.isUpToDate($0)
}

guard !targetsToPrepare.isEmpty else {
Expand All @@ -378,7 +378,7 @@ public final actor SemanticIndexManager {
PreparationTaskDescription(
targetsToPrepare: targetsToPrepare,
buildSystemManager: self.buildSystemManager,
preparationUpToDateStatus: preparationUpToDateStatus,
preparationUpToDateTracker: preparationUpToDateTracker,
indexProcessDidProduceResult: indexProcessDidProduceResult,
testHooks: testHooks
)
Expand Down Expand Up @@ -425,7 +425,7 @@ public final actor SemanticIndexManager {
filesToIndex: filesAndTargets,
buildSystemManager: self.buildSystemManager,
index: index,
indexStoreUpToDateStatus: indexStoreUpToDateStatus,
indexStoreUpToDateTracker: indexStoreUpToDateTracker,
indexProcessDidProduceResult: indexProcessDidProduceResult,
testHooks: testHooks
)
Expand Down Expand Up @@ -470,7 +470,7 @@ public final actor SemanticIndexManager {
// schedule two indexing jobs for the same file in quick succession, only the first one actually updates the index
// store and the second one will be a no-op once it runs.
let outOfDateFiles = await filesToIndex(toCover: files).asyncFilter {
if await indexStoreUpToDateStatus.isUpToDate($0.sourceFile) {
if await indexStoreUpToDateTracker.isUpToDate($0.sourceFile) {
return false
}
guard let language = await buildSystemManager.defaultLanguage(for: $0.mainFile),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import Foundation
import SKCore

/// Keeps track of whether an item (a target or file to index) is up-to-date.
actor IndexUpToDateStatusManager<Item: Hashable> {
actor UpToDateTracker<Item: Hashable> {
private enum Status {
/// The item is up-to-date.
case upToDate
Expand Down Expand Up @@ -57,7 +57,7 @@ actor IndexUpToDateStatusManager<Item: Hashable> {
}
}

func markAllOutOfDate() {
func markAllKnownOutOfDate() {
markOutOfDate(status.keys)
}

Expand Down
10 changes: 5 additions & 5 deletions Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
/// The build system manager that is used to get the toolchain and build settings for the files to index.
private let buildSystemManager: BuildSystemManager

private let indexStoreUpToDateStatus: IndexUpToDateStatusManager<DocumentURI>
private let indexStoreUpToDateTracker: UpToDateTracker<DocumentURI>

/// A reference to the underlying index store. Used to check if the index is already up-to-date for a file, in which
/// case we don't need to index it again.
Expand Down Expand Up @@ -138,14 +138,14 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
filesToIndex: [FileAndTarget],
buildSystemManager: BuildSystemManager,
index: UncheckedIndex,
indexStoreUpToDateStatus: IndexUpToDateStatusManager<DocumentURI>,
indexStoreUpToDateTracker: UpToDateTracker<DocumentURI>,
indexProcessDidProduceResult: @escaping @Sendable (IndexProcessResult) -> Void,
testHooks: IndexTestHooks
) {
self.filesToIndex = filesToIndex
self.buildSystemManager = buildSystemManager
self.index = index
self.indexStoreUpToDateStatus = indexStoreUpToDateStatus
self.indexStoreUpToDateTracker = indexStoreUpToDateTracker
self.indexProcessDidProduceResult = indexProcessDidProduceResult
self.testHooks = testHooks
}
Expand Down Expand Up @@ -202,7 +202,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
}

private func updateIndexStore(forSingleFile file: FileToIndex, in target: ConfiguredTarget) async {
guard await !indexStoreUpToDateStatus.isUpToDate(file.sourceFile) else {
guard await !indexStoreUpToDateTracker.isUpToDate(file.sourceFile) else {
// If we know that the file is up-to-date without having ot hit the index, do that because it's fastest.
return
}
Expand Down Expand Up @@ -264,7 +264,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
"Not updating index store for \(file) because it is a language that is not supported by background indexing"
)
}
await indexStoreUpToDateStatus.markUpToDate([file.sourceFile], updateOperationStartDate: startDate)
await indexStoreUpToDateTracker.markUpToDate([file.sourceFile], updateOperationStartDate: startDate)
}

private func updateIndexStore(
Expand Down
2 changes: 1 addition & 1 deletion Sources/SourceKitLSP/SourceKitLSPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,7 @@ extension SourceKitLSPServer {
nonisolated func indexTaskDidProduceResult(_ result: IndexProcessResult) {
self.sendNotificationToClient(
LogMessageNotification(
type: result.failed ? .info : .warning,
type: result.failed ? .warning : .info,
message: """
\(result.taskDescription) finished in \(result.duration)
\(result.command)
Expand Down
4 changes: 2 additions & 2 deletions Sources/sourcekit-lsp/SourceKitLSP.swift
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ struct SourceKitLSP: AsyncParsableCommand {

@Flag(
help: """
Show which index tasks are currently running in the indexing work done progress. \
This produces a multi-line work done progress, which might render incorrectly depending in the editor.
When reporting index progress, show the currently running index tasks in addition to the task's count. \
This produces a multi-line work done progress, which might render incorrectly, depending on the editor.
"""
)
var experimentalShowActivePreparationTasksInProgress = false
Expand Down