Skip to content

Commit fb8e85f

Browse files
committed
Use a Confinement Queue to Protect IncrementalCompilationState
This class is serendipitously used only from the main queue for the Driver's usual use cases. But we have no reason to expect clients of libSwiftDriver to manipulate this class in the same manner. Provide a serial confinement queue and protect the mutable members of IncrementalCompilationState with it. In the future, we ought to drop the queue and use actors. rdar://75744005
1 parent eb918de commit fb8e85f

File tree

1 file changed

+59
-25
lines changed

1 file changed

+59
-25
lines changed

Sources/SwiftDriver/IncrementalCompilation/IncrementalCompilationState.swift

Lines changed: 59 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,25 @@ import TSCBasic
1313
import Foundation
1414
import SwiftOptions
1515

16-
public class IncrementalCompilationState {
16+
/// An instance of `IncrementalCompilationState` encapsulates the data necessary
17+
/// to make incremental build scheduling decisions.
18+
///
19+
/// The primary form of interaction with the incremental compilation state is
20+
/// using it as an oracle to discover the jobs to execute as the incremental
21+
/// build progresses. After a job completes, call
22+
/// `IncrementalCompilationState.collectJobsDiscoveredToBeNeededAfterFinishing(job:)`
23+
/// to both update the incremental state and recieve an array of jobs that
24+
/// need to be executed in response.
25+
///
26+
/// Jobs become "unstuck" as their inputs become available, or may be discovered
27+
/// by this class as fresh dependency information is integrated.
28+
///
29+
/// Threading Considerations
30+
/// ========================
31+
///
32+
/// The public API surface of this class is thread safe, but not re-entrant.
33+
/// FIXME: This should be an actor.
34+
public final class IncrementalCompilationState {
1735

1836
/// The oracle for deciding what depends on what. Applies to this whole module.
1937
@_spi(Testing) public let moduleDependencyGraph: ModuleDependencyGraph
@@ -25,15 +43,24 @@ public class IncrementalCompilationState {
2543
/// Already batched, and in order of input files.
2644
public let mandatoryJobsInOrder: [Job]
2745

46+
/// Jobs to run *after* the last compile, for instance, link-editing.
47+
public let jobsAfterCompiles: [Job]
48+
49+
/// A high-priority confinement queue that must be used to protect the incremental compilation state.
50+
private let confinementQueue: DispatchQueue = DispatchQueue(label: "com.apple.swift-driver.incremental-compilation-state", qos: .userInteractive)
51+
2852
/// Sadly, has to be `var` for formBatchedJobs
53+
///
54+
/// After initialization, mutating accesses to the driver must be protected by
55+
/// the confinement queue.
2956
private var driver: Driver
3057

3158
/// Keyed by primary input. As required compilations are discovered after the first wave, these shrink.
59+
///
60+
/// This state is modified during the incremental build. All accesses must
61+
/// be protected by the confinement queue.
3262
private var skippedCompileGroups = [TypedVirtualPath: CompileJobGroup]()
3363

34-
/// Jobs to run *after* the last compile, for instance, link-editing.
35-
public let jobsAfterCompiles: [Job]
36-
3764
// MARK: - Creating IncrementalCompilationState if possible
3865
/// Return nil if not compiling incrementally
3966
init?(
@@ -124,10 +151,6 @@ extension IncrementalCompilationState {
124151
/// for the first wave to execute.
125152
let mandatoryJobsInOrder: [Job]
126153
}
127-
128-
129-
130-
131154
}
132155

133156
extension Driver {
@@ -199,18 +222,20 @@ extension IncrementalCompilationState {
199222
/// Careful: job may not be primary.
200223
public func collectJobsDiscoveredToBeNeededAfterFinishing(
201224
job finishedJob: Job) throws -> [Job]? {
202-
// Find and deal with inputs that now need to be compiled
203-
let invalidatedInputs = collectInputsInvalidatedByRunning(finishedJob)
204-
assert(Set(invalidatedInputs).isDisjoint(with: finishedJob.primaryInputs),
205-
"Primaries should not overlap secondaries.")
206-
207-
if let reporter = self.reporter {
208-
for input in invalidatedInputs {
209-
reporter.report(
210-
"Queuing because of dependencies discovered later:", input)
225+
return self.confinementQueue.sync {
226+
// Find and deal with inputs that now need to be compiled
227+
let invalidatedInputs = collectInputsInvalidatedByRunning(finishedJob)
228+
assert(Set(invalidatedInputs).isDisjoint(with: finishedJob.primaryInputs),
229+
"Primaries should not overlap secondaries.")
230+
231+
if let reporter = self.reporter {
232+
for input in invalidatedInputs {
233+
reporter.report(
234+
"Queuing because of dependencies discovered later:", input)
235+
}
211236
}
237+
return try getJobs(for: invalidatedInputs)
212238
}
213-
return try getJobs(for: invalidatedInputs)
214239
}
215240

216241
/// Needed for API compatibility, `result` will be ignored
@@ -222,6 +247,7 @@ extension IncrementalCompilationState {
222247

223248
/// After `job` finished find out which inputs must compiled that were not known to need compilation before
224249
private func collectInputsInvalidatedByRunning(_ job: Job)-> Set<TypedVirtualPath> {
250+
dispatchPrecondition(condition: .onQueue(self.confinementQueue))
225251
guard job.kind == .compile else {
226252
return Set<TypedVirtualPath>()
227253
}
@@ -231,8 +257,10 @@ extension IncrementalCompilationState {
231257
.subtracting(job.primaryInputs) // have already compiled these
232258
}
233259

234-
private func collectInputsInvalidated(byCompiling input: TypedVirtualPath)
235-
-> TransitivelyInvalidatedInputSet {
260+
private func collectInputsInvalidated(
261+
byCompiling input: TypedVirtualPath
262+
) -> TransitivelyInvalidatedInputSet {
263+
dispatchPrecondition(condition: .onQueue(self.confinementQueue))
236264
if let found = moduleDependencyGraph.collectInputsRequiringCompilation(byCompiling: input) {
237265
return found
238266
}
@@ -242,8 +270,10 @@ extension IncrementalCompilationState {
242270
}
243271

244272
/// Find the jobs that now must be run that were not originally known to be needed.
245-
private func getJobs(for invalidatedInputs: Set<TypedVirtualPath>
273+
private func getJobs(
274+
for invalidatedInputs: Set<TypedVirtualPath>
246275
) throws -> [Job] {
276+
dispatchPrecondition(condition: .onQueue(self.confinementQueue))
247277
let unbatched = invalidatedInputs.flatMap { input -> [Job] in
248278
if let group = skippedCompileGroups.removeValue(forKey: input) {
249279
let primaryInputs = group.compileJob.primaryInputs
@@ -264,12 +294,16 @@ extension IncrementalCompilationState {
264294
// MARK: - After the build
265295
extension IncrementalCompilationState {
266296
var skippedCompilationInputs: Set<TypedVirtualPath> {
267-
Set(skippedCompileGroups.keys)
297+
return self.confinementQueue.sync {
298+
Set(skippedCompileGroups.keys)
299+
}
268300
}
269301
public var skippedJobs: [Job] {
270-
skippedCompileGroups.values
271-
.sorted {$0.primaryInput.file.name < $1.primaryInput.file.name}
272-
.flatMap {$0.allJobs()}
302+
return self.confinementQueue.sync {
303+
skippedCompileGroups.values
304+
.sorted {$0.primaryInput.file.name < $1.primaryInput.file.name}
305+
.flatMap {$0.allJobs()}
306+
}
273307
}
274308
}
275309

0 commit comments

Comments
 (0)