Skip to content

Commit 7782771

Browse files
committed
Refactor TaskStore/Task to control mutability when interning stored strings
1 parent 3922a7d commit 7782771

File tree

8 files changed

+299
-191
lines changed

8 files changed

+299
-191
lines changed

Sources/SWBCore/EnvironmentBindings.swift

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,8 @@ public import SWBUtil
1616
///
1717
/// We almost never inspect the environment, so we make a space/size tradeoff by compactly storing the environment.
1818
public struct EnvironmentBindings: Sendable {
19-
private enum BindingsStorage {
20-
case direct([(String, String)])
21-
case interned([(StringArena.Handle, StringArena.Handle)], StringArena)
22-
}
23-
2419
/// The bindings are stored as one contiguous string
25-
private var bindingsStorage: BindingsStorage
26-
27-
public var bindings: [(String, String)] {
28-
switch bindingsStorage {
29-
case .direct(let bindings):
30-
return bindings
31-
case .interned(let internedBindings, let arena):
32-
return internedBindings.map { (arena.lookup(handle: $0.0), arena.lookup(handle: $0.1)) }
33-
}
34-
}
20+
public let bindings: [(String, String)]
3521

3622
public var bindingsDictionary: [String: String] {
3723
var dict = [String: String]()
@@ -43,15 +29,15 @@ public struct EnvironmentBindings: Sendable {
4329

4430
/// Create an empty binding map.
4531
public init() {
46-
self.bindingsStorage = .direct([])
32+
self.bindings = []
4733
}
4834

4935
public init(_ bindings: [(String, String)]) {
50-
self.bindingsStorage = .direct(bindings)
36+
self.bindings = bindings
5137
}
5238

5339
public init(_ bindings: [String: String]) {
54-
self.bindingsStorage = .direct(bindings.map { ($0, $1) }.sorted(by: { $0.0 < $1.0 }))
40+
self.bindings = bindings.map { ($0, $1) }.sorted(by: { $0.0 < $1.0 })
5541
}
5642

5743
/// Add a signature of the bindings into the given context.
@@ -64,13 +50,6 @@ public struct EnvironmentBindings: Sendable {
6450
ctx.add(number: 0)
6551
}
6652
}
67-
68-
public mutating func intern(in arena: StringArena) {
69-
guard case .direct(let bindings) = bindingsStorage else {
70-
return
71-
}
72-
self.bindingsStorage = .interned(bindings.map { (arena.intern($0.0), arena.intern($0.1)) }, arena)
73-
}
7453
}
7554

7655
extension EnvironmentBindings: Equatable {
@@ -90,6 +69,6 @@ extension EnvironmentBindings: Serializable {
9069

9170
public init(from deserializer: any Deserializer) throws {
9271
try deserializer.beginAggregate(2)
93-
self.bindingsStorage = .direct(Array(zip(try deserializer.deserialize(), try deserializer.deserialize())))
72+
self.bindings = Array(zip(try deserializer.deserialize(), try deserializer.deserialize()))
9473
}
9574
}

Sources/SWBTaskExecution/BuildDescription.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ package final class BuildDescription: Serializable, Sendable, Encodable, Cacheab
132132
}
133133

134134
// rdar://107734664 (Consider passing the task store as a parameter to build description methods rather than as a property of BuildDescription)
135-
package let taskStore: TaskStore
135+
package let taskStore: FrozenTaskStore
136136

137137
/// The set of all (non-virtual) paths produced by tasks in the build description.
138138
private let allOutputPaths: Set<Path>
@@ -188,7 +188,7 @@ package final class BuildDescription: Serializable, Sendable, Encodable, Cacheab
188188
package let targetsBuildInParallel: Bool
189189

190190
/// Load a build description from the given path.
191-
fileprivate init(inDir dir: Path, signature: BuildDescriptionSignature, taskStore: TaskStore, allOutputPaths: Set<Path>, rootPathsPerTarget: [ConfiguredTarget: [Path]], moduleCachePathPerTarget: [ConfiguredTarget: Path], settingsPerTarget: [ConfiguredTarget: Settings], enableStaleFileRemoval: Bool = true, taskActionMap: [String: TaskAction.Type], targetTaskCounts: [ConfiguredTarget: Int], moduleSessionFilePath: Path?, diagnostics: [ConfiguredTarget?: [Diagnostic]], fs: any FSProxy, invalidationPaths: [Path], recursiveSearchPathResults: [RecursiveSearchPathResolver.CachedResult], copiedPathMap: [String: String], targetDependencies: [TargetDependencyRelationship], definingTargetsByModuleName: [String: OrderedSet<ConfiguredTarget>], capturedBuildInfo: CapturedBuildInfo?, bypassActualTasks: Bool, targetsBuildInParallel: Bool) throws {
191+
fileprivate init(inDir dir: Path, signature: BuildDescriptionSignature, taskStore: FrozenTaskStore, allOutputPaths: Set<Path>, rootPathsPerTarget: [ConfiguredTarget: [Path]], moduleCachePathPerTarget: [ConfiguredTarget: Path], settingsPerTarget: [ConfiguredTarget: Settings], enableStaleFileRemoval: Bool = true, taskActionMap: [String: TaskAction.Type], targetTaskCounts: [ConfiguredTarget: Int], moduleSessionFilePath: Path?, diagnostics: [ConfiguredTarget?: [Diagnostic]], fs: any FSProxy, invalidationPaths: [Path], recursiveSearchPathResults: [RecursiveSearchPathResolver.CachedResult], copiedPathMap: [String: String], targetDependencies: [TargetDependencyRelationship], definingTargetsByModuleName: [String: OrderedSet<ConfiguredTarget>], capturedBuildInfo: CapturedBuildInfo?, bypassActualTasks: Bool, targetsBuildInParallel: Bool) throws {
192192
self.dir = dir
193193
self.signature = signature
194194
self.taskStore = taskStore
@@ -650,9 +650,11 @@ package final class BuildDescriptionBuilder {
650650
throw StubError.error("unable to record manifest to build description delegate: \(error)")
651651
}
652652

653+
let frozenTaskStore = taskStore.freeze()
654+
653655
// Compute the count of tasks by target, which we use to know when a task is complete.
654656
var targetTaskCounts = [ConfiguredTarget: Int]()
655-
taskStore.forEachTask { task in
657+
frozenTaskStore.forEachTask { task in
656658
if let target = task.forTarget, !task.isGate {
657659
targetTaskCounts[target] = (targetTaskCounts[target] ?? 0) + 1
658660
}
@@ -663,7 +665,7 @@ package final class BuildDescriptionBuilder {
663665
// Create the build description.
664666
let buildDescription: BuildDescription
665667
do {
666-
buildDescription = try BuildDescription(inDir: path, signature: signature, taskStore: taskStore, allOutputPaths: allOutputPaths, rootPathsPerTarget: rootPathsPerTarget, moduleCachePathPerTarget: moduleCachePathPerTarget, settingsPerTarget: settingsPerTarget, taskActionMap: taskActionMap, targetTaskCounts: targetTaskCounts, moduleSessionFilePath: moduleSessionFilePath, diagnostics: diagnosticsEngines.mapValues { engine in engine.diagnostics }, fs: fs, invalidationPaths: invalidationPaths, recursiveSearchPathResults: recursiveSearchPathResults, copiedPathMap: copiedPathMap, targetDependencies: targetDependencies, definingTargetsByModuleName: definingTargetsByModuleName, capturedBuildInfo: capturedBuildInfo, bypassActualTasks: bypassActualTasks, targetsBuildInParallel: targetsBuildInParallel)
668+
buildDescription = try BuildDescription(inDir: path, signature: signature, taskStore: frozenTaskStore, allOutputPaths: allOutputPaths, rootPathsPerTarget: rootPathsPerTarget, moduleCachePathPerTarget: moduleCachePathPerTarget, settingsPerTarget: settingsPerTarget, taskActionMap: taskActionMap, targetTaskCounts: targetTaskCounts, moduleSessionFilePath: moduleSessionFilePath, diagnostics: diagnosticsEngines.mapValues { engine in engine.diagnostics }, fs: fs, invalidationPaths: invalidationPaths, recursiveSearchPathResults: recursiveSearchPathResults, copiedPathMap: copiedPathMap, targetDependencies: targetDependencies, definingTargetsByModuleName: definingTargetsByModuleName, capturedBuildInfo: capturedBuildInfo, bypassActualTasks: bypassActualTasks, targetsBuildInParallel: targetsBuildInParallel)
667669
}
668670
catch {
669671
throw StubError.error("unable to create build description: \(error)")
@@ -1398,7 +1400,7 @@ package final class BuildDescriptionDeserializerDelegate: DeserializerDelegate,
13981400

13991401
package let uniquingCoordinator = UniquingDeserializationCoordinator()
14001402

1401-
package var taskStore: TaskStore? = nil
1403+
package var taskStore: FrozenTaskStore? = nil
14021404

14031405
package init(workspace: Workspace, platformRegistry: PlatformRegistry, sdkRegistry: SDKRegistry, specRegistry: SpecRegistry) {
14041406
self.workspace = workspace

Sources/SWBTaskExecution/BuildDescriptionManager.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ package final class BuildDescriptionManager: Sendable {
559559
let delegate = BuildDescriptionDeserializerDelegate(workspace: workspaceContext.workspace, platformRegistry: workspaceContext.core.platformRegistry, sdkRegistry: workspaceContext.core.sdkRegistry, specRegistry: workspaceContext.core.specRegistry)
560560
let taskStoreContents = try fs.read(path.join("task-store.msgpack"))
561561
let taskStoreDeserializer = MsgPackDeserializer(taskStoreContents, delegate: delegate)
562-
let taskStore: TaskStore = try taskStoreDeserializer.deserialize()
562+
let taskStore: FrozenTaskStore = try taskStoreDeserializer.deserialize()
563563
delegate.taskStore = taskStore
564564
let byteString = try fs.read(path.join("description.msgpack"))
565565
let deserializer = MsgPackDeserializer(byteString, delegate: delegate)

0 commit comments

Comments
 (0)