Parallelize retrieving resolved packages (#8220)

Parallelize retrieving resolved packages

### Motivation:

This PR brings back the parallelization reverted
[here](#8217).

As raised in the revertal, checkouts and registry downloads mutate the
`WorkspaceState` – and running these operations in parallel can lead to
a crash since the `WorkspaceState` is not thread-safe.

### Modifications:

Based on
[this](#8217 (comment))
suggestion, I'm making `WorkspaceState` an `actor` to make access to it
thread-safe.

Additionally, I turned `ManagedDependencies` into a `struct` to ensure
its thread-safety, too. There are two methods of this struct that mutate
self – `add` and `remove`. I changed this to return a new copy of
`ManagedDependencies` instead and add counterpart methods in
`WorkspaceState` where the mutation is done in-place. Since
`ManagedDependencies` is a `Collection`, making it into an `actor`
because of two mutable methods didn't feel like the right fit. However,
let me know what you think about it.

I added comments to the key modifications to make it easier to find the
more interesting modifications since there's a lot of noise since I had
to add loads of `async` and `await`. You can find these comments below.

### Result:

[Retrieving resolved
packages](https://github.com/swiftlang/swift-package-manager/blob/0340bb12a56f9696b3966ad82c2aee1594135377/Sources/Workspace/Workspace%2BDependencies.swift#L428)
in parallel should not lead to crashes.

As suggested by @bkhouri
[here](#8217 (comment)),
I ran `WorkspaceTests` multiple times to see if I run into any
thread-safety issues:
```
for ((i=1; i<=30; i++)); do swift test --filter WorkspaceTests.WorkspaceTests; done
for ((i=1; i<=10; i++)); do swift test --filter WorkspaceTests; done
```

Author: fortmarek

Sources/Commands/PackageCommands/ResetCommands.swift

@@ -38,15 +38,15 @@ extension SwiftPackageCommand {
         }
     }
 
-    struct Reset: SwiftCommand {
+    struct Reset: AsyncSwiftCommand {
         static let configuration = CommandConfiguration(
             abstract: "Reset the complete cache/build directory")
 
         @OptionGroup(visibility: .hidden)
         var globalOptions: GlobalOptions
 
-        func run(_ swiftCommandState: SwiftCommandState) throws {
-            try swiftCommandState.getActiveWorkspace().reset(observabilityScope: swiftCommandState.observabilityScope)
+        func run(_ swiftCommandState: SwiftCommandState) async throws {
+            try await swiftCommandState.getActiveWorkspace().reset(observabilityScope: swiftCommandState.observabilityScope)
         }
     }
 }

Sources/CoreCommands/BuildSystemSupport.swift

@@ -38,10 +38,15 @@ private struct NativeBuildSystemFactory: BuildSystemFactory {
     ) async throws -> any BuildSystem {
         _ = try await swiftCommandState.getRootPackageInformation()
         let testEntryPointPath = productsBuildParameters?.testProductStyle.explicitlySpecifiedEntryPointPath
+        let cacheBuildManifest = if cacheBuildManifest {
+            try await self.swiftCommandState.canUseCachedBuildManifest()
+        } else {
+            false
+        }
         return try BuildOperation(
             productsBuildParameters: try productsBuildParameters ?? self.swiftCommandState.productsBuildParameters,
             toolsBuildParameters: try toolsBuildParameters ?? self.swiftCommandState.toolsBuildParameters,
-            cacheBuildManifest: cacheBuildManifest && self.swiftCommandState.canUseCachedBuildManifest(),
+            cacheBuildManifest: cacheBuildManifest,
             packageGraphLoader: packageGraphLoader ?? {
                 try await self.swiftCommandState.loadPackageGraph(
                     explicitProduct: explicitProduct,

Sources/CoreCommands/SwiftCommandState.swift

@@ -689,7 +689,7 @@ public final class SwiftCommandState {
         try _manifestLoader.get()
     }
 
-    public func canUseCachedBuildManifest() throws -> Bool {
+    public func canUseCachedBuildManifest() async throws -> Bool {
         if !self.options.caching.cacheBuildManifest {
             return false
         }
@@ -706,7 +706,7 @@ public final class SwiftCommandState {
         // Perform steps for build manifest caching if we can enabled it.
         //
         // FIXME: We don't add edited packages in the package structure command yet (SR-11254).
-        let hasEditedPackages = try self.getActiveWorkspace().state.dependencies.contains(where: \.isEdited)
+        let hasEditedPackages = try await self.getActiveWorkspace().state.dependencies.contains(where: \.isEdited)
         if hasEditedPackages {
             return false
         }

Sources/Workspace/ManagedDependency.swift

@@ -172,12 +172,18 @@ extension Workspace.ManagedDependency: CustomStringConvertible {
 
 extension Workspace {
     /// A collection of managed dependencies.
-    final public class ManagedDependencies {
+    public struct ManagedDependencies {
         private var dependencies: [PackageIdentity: ManagedDependency]
 
         init() {
             self.dependencies = [:]
         }
+        
+        private init(
+            _ dependencies: [PackageIdentity: ManagedDependency]
+        ) {
+            self.dependencies = dependencies
+        }
 
         init(_ dependencies: [ManagedDependency]) throws {
             // rdar://86857825 do not use Dictionary(uniqueKeysWithValues:) as it can crash the process when input is incorrect such as in older versions of SwiftPM
@@ -204,12 +210,16 @@ extension Workspace {
             return .none
         }
 
-        public func add(_ dependency: ManagedDependency) {
-            self.dependencies[dependency.packageRef.identity] = dependency
+        public func add(_ dependency: ManagedDependency) -> Self {
+            var dependencies = dependencies
+            dependencies[dependency.packageRef.identity] = dependency
+            return ManagedDependencies(dependencies)
         }
 
-        public func remove(_ identity: PackageIdentity) {
-            self.dependencies[identity] = nil
+        public func remove(_ identity: PackageIdentity) -> Self {
+            var dependencies = dependencies
+            dependencies[identity] = nil
+            return ManagedDependencies(dependencies)
         }
     }
 }

Sources/Workspace/Workspace+BinaryArtifacts.swift

@@ -811,7 +811,7 @@ extension Workspace {
         var artifactsToDownload: [BinaryArtifactsManager.RemoteArtifact] = []
         var artifactsToExtract: [ManagedArtifact] = []
 
-        for artifact in state.artifacts {
+        for artifact in await state.artifacts {
             if !manifestArtifacts.local
                 .contains(where: { $0.packageRef == artifact.packageRef && $0.targetName == artifact.targetName }) &&
                 !manifestArtifacts.remote
@@ -822,7 +822,7 @@ extension Workspace {
         }
 
         for artifact in manifestArtifacts.local {
-            let existingArtifact = self.state.artifacts[
+            let existingArtifact = await self.state.artifacts[
                 packageIdentity: artifact.packageRef.identity,
                 targetName: artifact.targetName
             ]
@@ -859,7 +859,7 @@ extension Workspace {
         }
 
         for artifact in manifestArtifacts.remote {
-            let existingArtifact = self.state.artifacts[
+            let existingArtifact = await self.state.artifacts[
                 packageIdentity: artifact.packageRef.identity,
                 targetName: artifact.targetName
             ]
@@ -891,9 +891,9 @@ extension Workspace {
         }
 
         // Remove the artifacts and directories which are not needed anymore.
-        observabilityScope.trap {
+        await observabilityScope.trap {
             for artifact in artifactsToRemove {
-                state.artifacts.remove(packageIdentity: artifact.packageRef.identity, targetName: artifact.targetName)
+                await state.artifacts.remove(packageIdentity: artifact.packageRef.identity, targetName: artifact.targetName)
 
                 if isAtArtifactsDirectory(artifact) {
                     try fileSystem.removeFileTree(artifact.path)
@@ -930,15 +930,15 @@ extension Workspace {
 
         // Add the new artifacts
         for artifact in artifactsToAdd {
-            self.state.artifacts.add(artifact)
+            await self.state.artifacts.add(artifact)
         }
 
         guard !observabilityScope.errorsReported else {
             throw Diagnostics.fatalError
         }
 
-        observabilityScope.trap {
-            try self.state.save()
+        await observabilityScope.trap {
+            try await self.state.save()
         }
 
         func isAtArtifactsDirectory(_ artifact: ManagedArtifact) -> Bool {

Sources/Workspace/Workspace+Dependencies.swift

@@ -169,7 +169,7 @@ extension Workspace {
         }
 
         // Update the resolved file.
-        try self.saveResolvedFile(
+        try await self.saveResolvedFile(
             resolvedPackagesStore: resolvedPackagesStore,
             dependencyManifests: updatedDependencyManifests,
             originHash: resolvedFileOriginHash,
@@ -219,7 +219,7 @@ extension Workspace {
         case .update(let forceResolution):
             return try await resolveAndUpdateResolvedFile(forceResolution: forceResolution)
         case .bestEffort:
-            guard !self.state.dependencies.hasEditedDependencies() else {
+            guard await !self.state.dependencies.hasEditedDependencies() else {
                 return try await resolveAndUpdateResolvedFile(forceResolution: false)
             }
             guard self.fileSystem.exists(self.location.resolvedVersionsFile) else {
@@ -410,9 +410,10 @@ extension Workspace {
         //
         // We require cloning if there is no checkout or if the checkout doesn't
         // match with the pin.
+        let dependencies = await state.dependencies
         let requiredResolvedPackages = resolvedPackagesStore.resolvedPackages.values.filter { pin in
             // also compare the location in case it has changed
-            guard let dependency = state.dependencies[comparingLocation: pin.packageRef] else {
+            guard let dependency = dependencies[comparingLocation: pin.packageRef] else {
                 return true
             }
             switch dependency.state {
@@ -426,26 +427,31 @@ extension Workspace {
         }
 
         // Retrieve the required resolved packages.
-        for resolvedPackage in requiredResolvedPackages {
-            await observabilityScope.makeChildScope(
-                description: "retrieving resolved package versions for dependencies",
-                metadata: resolvedPackage.packageRef.diagnosticsMetadata
-            ).trap {
-                switch resolvedPackage.packageRef.kind {
-                case .localSourceControl, .remoteSourceControl:
-                    _ = try await self.checkoutRepository(
-                        package: resolvedPackage.packageRef,
-                        at: resolvedPackage.state,
-                        observabilityScope: observabilityScope
-                    )
-                case .registry:
-                    _ = try await self.downloadRegistryArchive(
-                        package: resolvedPackage.packageRef,
-                        at: resolvedPackage.state,
-                        observabilityScope: observabilityScope
-                    )
-                default:
-                    throw InternalError("invalid resolved package type \(resolvedPackage.packageRef.kind)")
+        await withThrowingTaskGroup(of: Void.self) { taskGroup in
+            for resolvedPackage in requiredResolvedPackages {
+                let observabilityScope = observabilityScope.makeChildScope(
+                    description: "retrieving resolved package versions for dependencies",
+                    metadata: resolvedPackage.packageRef.diagnosticsMetadata
+                )
+                taskGroup.addTask {
+                    await observabilityScope.trap {
+                        switch resolvedPackage.packageRef.kind {
+                        case .localSourceControl, .remoteSourceControl:
+                            _ = try await self.checkoutRepository(
+                                package: resolvedPackage.packageRef,
+                                at: resolvedPackage.state,
+                                observabilityScope: observabilityScope
+                            )
+                        case .registry:
+                            _ = try await self.downloadRegistryArchive(
+                                package: resolvedPackage.packageRef,
+                                at: resolvedPackage.state,
+                                observabilityScope: observabilityScope
+                            )
+                        default:
+                            throw InternalError("invalid resolved package type \(resolvedPackage.packageRef.kind)")
+                        }
+                    }
                 }
             }
         }
@@ -523,7 +529,7 @@ extension Workspace {
         }
 
         // load and update the `Package.resolved` store with any changes from loading the top level dependencies
-        guard let resolvedPackagesStore = self.loadAndUpdateResolvedPackagesStore(
+        guard let resolvedPackagesStore = await self.loadAndUpdateResolvedPackagesStore(
             dependencyManifests: currentManifests,
             rootManifestsMinimumToolsVersion: rootManifestsMinimumToolsVersion,
             observabilityScope: observabilityScope
@@ -554,7 +560,7 @@ extension Workspace {
             case .notRequired:
                 // since nothing changed we can exit early,
                 // but need update resolved file and download an missing binary artifact
-                try self.saveResolvedFile(
+                try await self.saveResolvedFile(
                     resolvedPackagesStore: resolvedPackagesStore,
                     dependencyManifests: currentManifests,
                     originHash: resolvedFileOriginHash,
@@ -625,7 +631,7 @@ extension Workspace {
         }
 
         // Update the resolved file.
-        try self.saveResolvedFile(
+        try await self.saveResolvedFile(
             resolvedPackagesStore: resolvedPackagesStore,
             dependencyManifests: updatedDependencyManifests,
             originHash: resolvedFileOriginHash,
@@ -679,15 +685,15 @@ extension Workspace {
 
         // First remove the checkouts that are no longer required.
         for (packageRef, state) in packageStateChanges {
-            observabilityScope.makeChildScope(
+            await observabilityScope.makeChildScope(
                 description: "removing unneeded checkouts",
                 metadata: packageRef.diagnosticsMetadata
             ).trap {
                 switch state {
                 case .added, .updated, .unchanged:
                     break
                 case .removed:
-                    try self.remove(package: packageRef)
+                    try await self.remove(package: packageRef)
                 }
             }
         }
@@ -772,8 +778,8 @@ extension Workspace {
                     state: .custom(version: version, path: path),
                     subpath: RelativePath(validating: "")
                 )
-                self.state.dependencies.add(dependency)
-                try self.state.save()
+                await self.state.add(dependency: dependency)
+                try await self.state.save()
                 return path
             } else {
                 throw InternalError("invalid container for \(package.identity) of type \(package.kind)")
@@ -800,8 +806,8 @@ extension Workspace {
                 throw InternalError("invalid package type: \(package.kind)")
             }
 
-            self.state.dependencies.add(dependency)
-            try self.state.save()
+            await self.state.add(dependency: dependency)
+            try await self.state.save()
             return path
         }
     }
@@ -889,7 +895,7 @@ extension Workspace {
         dependencyManifests: DependencyManifests,
         rootManifestsMinimumToolsVersion: ToolsVersion,
         observabilityScope: ObservabilityScope
-    ) -> ResolvedPackagesStore? {
+    ) async -> ResolvedPackagesStore? {
         guard let resolvedPackagesStore = observabilityScope.trap({ try self.resolvedPackagesStore.load() }) else {
             return nil
         }
@@ -899,7 +905,7 @@ extension Workspace {
         else {
             return nil
         }
-        for dependency in self.state.dependencies.filter(\.packageRef.kind.isResolvable) {
+        for dependency in await self.state.dependencies.filter(\.packageRef.kind.isResolvable) {
             // a required dependency that is already loaded (managed) should be represented in the `Package.resolved` store.
             // also comparing location as it may have changed at this point
             if requiredDependencies.contains(where: { $0.equalsIncludingLocation(dependency.packageRef) }) {
@@ -1011,13 +1017,13 @@ extension Workspace {
             // Get the existing managed dependency for this package ref, if any.
 
             // first find by identity only since edit location may be different by design
-            var currentDependency = self.state.dependencies[binding.package.identity]
+            var currentDependency = await self.state.dependencies[binding.package.identity]
             // Check if this is an edited dependency.
             if case .edited(let basedOn, _) = currentDependency?.state, let originalReference = basedOn?.packageRef {
                 packageStateChanges[originalReference.identity] = (originalReference, .unchanged)
             } else {
                 // if not edited, also compare by location since it may have changed
-                currentDependency = self.state.dependencies[comparingLocation: binding.package]
+                currentDependency = await self.state.dependencies[comparingLocation: binding.package]
             }
 
             switch binding.boundVersion {
@@ -1119,7 +1125,7 @@ extension Workspace {
             }
         }
         // Set the state of any old package that might have been removed.
-        for packageRef in self.state.dependencies.lazy.map(\.packageRef)
+        for packageRef in await self.state.dependencies.lazy.map(\.packageRef)
             where packageStateChanges[packageRef.identity] == nil
         {
             packageStateChanges[packageRef.identity] = (packageRef, .removed)

Sources/Workspace/Workspace+Editing.swift

@@ -28,7 +28,7 @@ extension Workspace {
         observabilityScope: ObservabilityScope
     ) async throws {
         // Look up the dependency and check if we can edit it.
-        guard let dependency = self.state.dependencies[.plain(packageIdentity)] else {
+        guard let dependency = await self.state.dependencies[.plain(packageIdentity)] else {
             observabilityScope.emit(.dependencyNotFound(packageName: packageIdentity))
             return
         }
@@ -157,10 +157,10 @@ extension Workspace {
         }
 
         // Save the new state.
-        try self.state.dependencies.add(
-            dependency.edited(subpath: RelativePath(validating: packageIdentity), unmanagedPath: path)
+        try await self.state.add(
+            dependency: dependency.edited(subpath: RelativePath(validating: packageIdentity), unmanagedPath: path)
         )
-        try self.state.save()
+        try await self.state.save()
     }
 
     /// Unedit a managed dependency. See public API unedit(packageName:forceRemove:).
@@ -222,8 +222,8 @@ extension Workspace {
             )
         } else {
             // The original dependency was removed, update the managed dependency state.
-            self.state.dependencies.remove(dependency.packageRef.identity)
-            try self.state.save()
+            await self.state.remove(identity: dependency.packageRef.identity)
+            try await self.state.save()
         }
 
         // Resolve the dependencies if workspace root is provided. We do this to