Repository navigation
Coalesce sidebar git metadata probes #5402
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,76 @@ typealias Tab = Workspace | |
|
|
||
| private let tabManagerLogger = Logger(subsystem: "com.cmuxterm.app", category: "TabManager") | ||
|
|
||
| protocol WorkspaceGitMetadataReading: Sendable { | ||
| func workspaceMetadata(for directory: String) async -> GitWorkspaceMetadata | ||
| } | ||
|
|
||
| extension GitMetadataService: WorkspaceGitMetadataReading {} | ||
|
|
||
| private struct WorkspaceGitMetadataProbeWaiter { | ||
| let id: UUID | ||
| let continuation: CheckedContinuation<Bool, Never> | ||
| } | ||
|
|
||
| actor WorkspaceGitMetadataProbeLimiter { | ||
| static let shared = WorkspaceGitMetadataProbeLimiter(limit: 2) | ||
|
|
||
| private let limit: Int | ||
| private var activeCount = 0 | ||
| private var waiters: [WorkspaceGitMetadataProbeWaiter] = [] | ||
| private var cancelledWaiterIds: Set<UUID> = [] | ||
|
|
||
| init(limit: Int) { | ||
| self.limit = max(1, limit) | ||
| } | ||
|
|
||
| func acquire() async -> Bool { | ||
| let id = UUID() | ||
| guard !Task.isCancelled else { return false } | ||
| if activeCount < limit { | ||
| activeCount += 1 | ||
| return true | ||
| } | ||
|
|
||
| return await withTaskCancellationHandler { | ||
| await withCheckedContinuation { continuation in | ||
| if cancelledWaiterIds.remove(id) != nil { | ||
| continuation.resume(returning: false) | ||
| } else { | ||
| waiters.append(WorkspaceGitMetadataProbeWaiter(id: id, continuation: continuation)) | ||
| } | ||
| } | ||
| } onCancel: { | ||
| Task { | ||
| await self.cancelWaiter(id: id) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func release() { | ||
| guard activeCount > 0 else { return } | ||
| while !waiters.isEmpty { | ||
| let waiter = waiters.removeFirst() | ||
| if cancelledWaiterIds.remove(waiter.id) != nil { | ||
| waiter.continuation.resume(returning: false) | ||
| continue | ||
| } | ||
| waiter.continuation.resume(returning: true) | ||
| return | ||
| } | ||
| activeCount -= 1 | ||
| } | ||
|
|
||
| private func cancelWaiter(id: UUID) { | ||
| if let index = waiters.firstIndex(where: { $0.id == id }) { | ||
| let waiter = waiters.remove(at: index) | ||
| waiter.continuation.resume(returning: false) | ||
| } else { | ||
| cancelledWaiterIds.insert(id) | ||
| } | ||
| } | ||
|
Comment on lines
+80
to
+87
Contributor
There was a problem hiding this comment.
There is a race window where Rule Used: Flag new blocking or timing-based synchronization ... (source) |
||
| } | ||
|
|
||
| enum NewWorkspacePlacement: String, CaseIterable, Identifiable { | ||
| case top | ||
| case afterCurrent | ||
|
|
@@ -980,6 +1050,11 @@ class TabManager: ObservableObject { | |
| let panelId: UUID | ||
| } | ||
|
|
||
| private struct WorkspaceGitSnapshotProbeRequest: Sendable { | ||
| let probeKey: WorkspaceGitProbeKey | ||
| let isLastAttempt: Bool | ||
| } | ||
|
|
||
| private enum WorkspaceGitProbeState: Equatable { | ||
| case idle | ||
| case inFlight(rerunPending: Bool) | ||
|
|
@@ -1139,6 +1214,9 @@ class TabManager: ObservableObject { | |
| private var workspaceGitMetadataWatcherSourceDirectoryByKey: [WorkspaceGitProbeKey: String] = [:] | ||
| private var workspaceGitMetadataWatcherDescriptorRequestsByKey: [WorkspaceGitProbeKey: WorkspaceGitMetadataWatcherDescriptorRequest] = [:] | ||
| private var workspaceGitMetadataWatcherDescriptorGeneration: UInt64 = 0 | ||
| private var workspaceGitSnapshotRequestsByDirectory: [String: [WorkspaceGitSnapshotProbeRequest]] = [:] | ||
| private var workspaceGitSnapshotTasksByDirectory: [String: Task<Void, Never>] = [:] | ||
| private var workspaceGitSnapshotDirectoryByProbeKey: [WorkspaceGitProbeKey: String] = [:] | ||
| private var workspaceGitMetadataFallbackTask: Task<Void, Never>? | ||
| private var lastSidebarGitMetadataWatchEnabled = SidebarWorkspaceDetailDefaults.watchGitStatusValue(defaults: .standard) | ||
| private var lastSidebarPullRequestPollingEnabled = SidebarWorkspaceDetailDefaults.pullRequestPollingEnabled(defaults: .standard) | ||
|
|
@@ -1200,6 +1278,7 @@ class TabManager: ObservableObject { | |
| // slugs) off the main actor. Stateless; the reads are pure functions of the | ||
| // directory argument. | ||
| private let gitMetadataService: GitMetadataService | ||
| private let workspaceGitMetadataReader: any WorkspaceGitMetadataReading | ||
|
|
||
| // Resolves GitHub PR badges (slug resolution, REST fetch, candidate | ||
| // matching). Stateless; the repo cache stays here in | ||
|
|
@@ -1217,10 +1296,12 @@ class TabManager: ObservableObject { | |
| autoWelcomeIfNeeded: Bool = true, | ||
| commandRunner: any CommandRunning = CommandRunner(), | ||
| gitMetadataService: GitMetadataService = GitMetadataService(), | ||
| workspaceGitMetadataReader: (any WorkspaceGitMetadataReading)? = nil, | ||
| gitPollClock: any GitPollClock = SystemGitPollClock() | ||
| ) { | ||
| self.commandRunner = commandRunner | ||
| self.gitMetadataService = gitMetadataService | ||
| self.workspaceGitMetadataReader = workspaceGitMetadataReader ?? gitMetadataService | ||
| self.gitPollClock = gitPollClock | ||
| #if DEBUG | ||
| self.pullRequestProbeService = PullRequestProbeService( | ||
|
|
@@ -1301,6 +1382,9 @@ class TabManager: ObservableObject { | |
| for task in workspaceGitProbeTasksByKey.values { | ||
| task.cancel() | ||
| } | ||
| for task in workspaceGitSnapshotTasksByDirectory.values { | ||
| task.cancel() | ||
| } | ||
| workspacePullRequestRefreshTask?.cancel() | ||
| } | ||
|
|
||
|
|
@@ -1423,6 +1507,7 @@ class TabManager: ObservableObject { | |
| task.cancel() | ||
| } | ||
| workspaceGitProbeTasksByKey.removeAll() | ||
| cancelAllWorkspaceGitSnapshotTasks() | ||
| workspaceGitTrackedDirectoryByKey.removeAll() | ||
| workspaceGitCleanIndexSignatureByKey.removeAll() | ||
| workspaceGitCleanIndexContentSignatureByKey.removeAll() | ||
|
|
@@ -2715,27 +2800,116 @@ class TabManager: ObservableObject { | |
| return | ||
| } | ||
|
|
||
| enqueueWorkspaceGitMetadataSnapshotRequest( | ||
| probeKey: probeKey, | ||
| expectedDirectory: expectedDirectory, | ||
| isLastAttempt: isLastAttempt | ||
| ) | ||
| } | ||
|
|
||
| private func enqueueWorkspaceGitMetadataSnapshotRequest( | ||
| probeKey: WorkspaceGitProbeKey, | ||
| expectedDirectory: String, | ||
| isLastAttempt: Bool | ||
| ) { | ||
| let request = WorkspaceGitSnapshotProbeRequest( | ||
| probeKey: probeKey, | ||
| isLastAttempt: isLastAttempt | ||
| ) | ||
| if let currentDirectory = workspaceGitSnapshotDirectoryByProbeKey[probeKey], | ||
| currentDirectory != expectedDirectory { | ||
| removeWorkspaceGitSnapshotRequest(for: probeKey) | ||
| } | ||
| workspaceGitSnapshotDirectoryByProbeKey[probeKey] = expectedDirectory | ||
| if var requests = workspaceGitSnapshotRequestsByDirectory[expectedDirectory], | ||
| let existingRequestIndex = requests.firstIndex(where: { $0.probeKey == probeKey }) { | ||
| requests[existingRequestIndex] = request | ||
| workspaceGitSnapshotRequestsByDirectory[expectedDirectory] = requests | ||
| } else { | ||
| workspaceGitSnapshotRequestsByDirectory[expectedDirectory, default: []].append(request) | ||
| } | ||
| guard workspaceGitSnapshotTasksByDirectory[expectedDirectory] == nil else { | ||
| #if DEBUG | ||
| cmuxDebugLog( | ||
| "workspace.gitProbe.joinSnapshot dir=\(expectedDirectory) " + | ||
| "queued=\(workspaceGitSnapshotRequestsByDirectory[expectedDirectory]?.count ?? 0)" | ||
| ) | ||
| #endif | ||
| return | ||
| } | ||
|
|
||
| let reader = workspaceGitMetadataReader | ||
| workspaceGitSnapshotTasksByDirectory[expectedDirectory] = Task.detached(priority: .utility) { [weak self] in | ||
| let didAcquirePermit = await WorkspaceGitMetadataProbeLimiter.shared.acquire() | ||
| guard didAcquirePermit else { return } | ||
| defer { | ||
| Task { | ||
| await WorkspaceGitMetadataProbeLimiter.shared.release() | ||
| } | ||
| } | ||
|
|
||
| guard !Task.isCancelled else { return } | ||
| let snapshot = await Self.initialWorkspaceGitMetadataSnapshot( | ||
| for: expectedDirectory, | ||
| reader: reader | ||
| ) | ||
| guard !Task.isCancelled else { return } | ||
| await MainActor.run { [weak self] in | ||
| guard !Task.isCancelled else { return } | ||
| self?.applyWorkspaceGitMetadataSnapshotBatch( | ||
| snapshot, | ||
| expectedDirectory: expectedDirectory | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private func applyWorkspaceGitMetadataSnapshotBatch( | ||
| _ snapshot: InitialWorkspaceGitMetadataSnapshot, | ||
| expectedDirectory: String | ||
| ) { | ||
| workspaceGitSnapshotTasksByDirectory.removeValue(forKey: expectedDirectory) | ||
| let requests = workspaceGitSnapshotRequestsByDirectory.removeValue(forKey: expectedDirectory) ?? [] | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| for request in requests { | ||
| workspaceGitSnapshotDirectoryByProbeKey.removeValue(forKey: request.probeKey) | ||
| applyWorkspaceGitMetadataSnapshot( | ||
| snapshot, | ||
| probeKey: request.probeKey, | ||
| expectedDirectory: expectedDirectory, | ||
| isLastAttempt: request.isLastAttempt | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| private func removeWorkspaceGitSnapshotRequest(for key: WorkspaceGitProbeKey) { | ||
| guard let directory = workspaceGitSnapshotDirectoryByProbeKey.removeValue(forKey: key), | ||
| var requests = workspaceGitSnapshotRequestsByDirectory[directory] else { | ||
| return | ||
| } | ||
| requests.removeAll { $0.probeKey == key } | ||
| if requests.isEmpty { | ||
| workspaceGitSnapshotRequestsByDirectory.removeValue(forKey: directory) | ||
| workspaceGitSnapshotTasksByDirectory.removeValue(forKey: directory)?.cancel() | ||
| } else { | ||
| workspaceGitSnapshotRequestsByDirectory[directory] = requests | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| private func cancelAllWorkspaceGitSnapshotTasks() { | ||
| for task in workspaceGitSnapshotTasksByDirectory.values { | ||
| task.cancel() | ||
| } | ||
| workspaceGitSnapshotTasksByDirectory.removeAll() | ||
| workspaceGitSnapshotRequestsByDirectory.removeAll() | ||
| workspaceGitSnapshotDirectoryByProbeKey.removeAll() | ||
| } | ||
|
|
||
| private func cancelWorkspaceGitProbeTask(for key: WorkspaceGitProbeKey) { | ||
| workspaceGitProbeTasksByKey.removeValue(forKey: key)?.cancel() | ||
| } | ||
|
|
||
| private func clearWorkspaceGitProbe(_ key: WorkspaceGitProbeKey) { | ||
| removeWorkspaceGitSnapshotRequest(for: key) | ||
| workspaceGitProbeStateByKey.removeValue(forKey: key) | ||
| workspaceGitCleanIndexSignatureByKey.removeValue(forKey: key) | ||
| workspaceGitCleanIndexContentSignatureByKey.removeValue(forKey: key) | ||
|
|
@@ -3002,10 +3176,11 @@ class TabManager: ObservableObject { | |
| } | ||
| } | ||
|
|
||
| private nonisolated static func initialWorkspaceGitMetadataSnapshot( | ||
| for directory: String, | ||
| reader: any WorkspaceGitMetadataReading | ||
| ) async -> InitialWorkspaceGitMetadataSnapshot { | ||
| let metadata = await reader.workspaceMetadata(for: directory) | ||
| guard metadata.isRepository else { | ||
| return InitialWorkspaceGitMetadataSnapshot( | ||
| isRepository: false, | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WorkspaceGitMetadataReadingwas correctly made injectable (and the test stubs it out), butWorkspaceGitMetadataProbeLimiter.sharedis a module-level singleton referenced directly inside theTask.detachedbody. This means the limiter's 2-permit semantics cannot be varied or isolated in tests — all tests share its live state. If two test cases that exercise the probe path run concurrently, they compete for the same 2 permits. Passing the limiter throughTabManager's initializer (with the shared instance as the default) would mirror the pattern already applied toworkspaceGitMetadataReaderand eliminate the shared mutable singleton.Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!