Start adopting Swift Concurrency

Sources/DocCCommandLine/Action/Actions/Convert/ConvertAction.swift

@@ -336,20 +336,17 @@ public struct ConvertAction: AsyncAction {
             }
         }
 
-        let analysisProblems: [Problem]
-        let conversionProblems: [Problem]
         do {
-            conversionProblems = try signposter.withIntervalSignpost("Process") {
-                try ConvertActionConverter.convert(
-                    context: context,
-                    outputConsumer: outputConsumer,
-                    htmlContentConsumer: htmlConsumer,
-                    sourceRepository: sourceRepository,
-                    emitDigest: emitDigest,
-                    documentationCoverageOptions: documentationCoverageOptions
-                )
-            }
-            analysisProblems = context.problems
+            let processInterval = signposter.beginInterval("Process", id: signposter.makeSignpostID())
+            try await ConvertActionConverter.convert(
+                context: context,
+                outputConsumer: outputConsumer,
+                htmlContentConsumer: htmlConsumer,
+                sourceRepository: sourceRepository,
+                emitDigest: emitDigest,
+                documentationCoverageOptions: documentationCoverageOptions
+            )
+            signposter.endInterval("Process", processInterval)
         } catch {
             if emitDigest {
                 let problem = Problem(description: (error as? (any DescribedError))?.errorDescription ?? error.localizedDescription, source: nil)
@@ -359,7 +356,7 @@ public struct ConvertAction: AsyncAction {
             throw error
         }
 
-        var didEncounterError = analysisProblems.containsErrors || conversionProblems.containsErrors
+        var didEncounterError = context.problems.containsErrors
         let hasTutorial = context.knownPages.contains(where: {
             guard let kind = try? context.entity(with: $0).kind else { return false }
             return kind == .tutorial || kind == .tutorialArticle

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

@@ -218,7 +218,7 @@ public class DocumentationContext {
         self.linkResolver = LinkResolver(dataProvider: dataProvider)
 
         ResolvedTopicReference.enableReferenceCaching(for: inputs.id)
-        try register()
+        try await register()
     }
 
     /// Perform semantic analysis on a given `document` at a given `source` location and append any problems found to `problems`.
@@ -1950,7 +1950,7 @@ public class DocumentationContext {
     /**
      Register a documentation bundle with this context.
      */
-    private func register() throws {
+    private func register() async throws {
         try shouldContinueRegistration()
 
         let currentFeatureFlags: FeatureFlags?
@@ -1982,111 +1982,61 @@ public class DocumentationContext {
             }
         }
 
-        // Note: Each bundle is registered and processed separately.
-        // Documents and symbols may both reference each other so the bundle is registered in 4 steps
-
-        // In the bundle discovery phase all tasks run in parallel as they don't depend on each other.
-        let discoveryGroup = DispatchGroup()
-        let discoveryQueue = DispatchQueue(label: "org.swift.docc.Discovery", qos: .unspecified, attributes: .concurrent, autoreleaseFrequency: .workItem)
-
-        let discoveryError = Synchronized<(any Error)?>(nil)
+        // Documents and symbols may both reference each other so the inputs is registered in 4 steps
 
-        // Load all bundle symbol graphs into the loader.
-        var symbolGraphLoader: SymbolGraphLoader!
-        var hierarchyBasedResolver: PathHierarchyBasedLinkResolver!
-
-        discoveryGroup.async(queue: discoveryQueue) { [unowned self] in
-            symbolGraphLoader = SymbolGraphLoader(
+        // Load symbol information and construct data structures that only rely on symbol information.
+        async let loadSymbols = { [signposter, inputs, dataProvider, configuration] in
+            var symbolGraphLoader = SymbolGraphLoader(
                 bundle: inputs,
                 dataProvider: dataProvider,
                 symbolGraphTransformer: configuration.convertServiceConfiguration.symbolGraphTransformer
             )
 
-            do {
-                try signposter.withIntervalSignpost("Load symbols", id: signposter.makeSignpostID()) {
+            try signposter.withIntervalSignpost("Load symbols", id: signposter.makeSignpostID()) {
+                try autoreleasepool {
                     try symbolGraphLoader.loadAll()
                 }
-                hierarchyBasedResolver = signposter.withIntervalSignpost("Build PathHierarchy", id: signposter.makeSignpostID()) {
+            }
+            try shouldContinueRegistration()
+            let hierarchyBasedResolver = signposter.withIntervalSignpost("Build PathHierarchy", id: signposter.makeSignpostID()) {
+                autoreleasepool {
                     PathHierarchyBasedLinkResolver(pathHierarchy: PathHierarchy(
                         symbolGraphLoader: symbolGraphLoader,
                         bundleName: urlReadablePath(inputs.displayName),
                         knownDisambiguatedPathComponents: configuration.convertServiceConfiguration.knownDisambiguatedSymbolPathComponents
                     ))
                 }
-
-                self.snippetResolver = SnippetResolver(symbolGraphLoader: symbolGraphLoader)
-            } catch {
-                // Pipe the error out of the dispatch queue.
-                discoveryError.sync({
-                    if $0 == nil { $0 = error }
-                })
             }
-        }
+
+            let snippetResolver = SnippetResolver(symbolGraphLoader: symbolGraphLoader)
+
+            return (symbolGraphLoader, hierarchyBasedResolver, snippetResolver)
+        }()
 
-        // First, all the resources are added since they don't reference anything else.
-        discoveryGroup.async(queue: discoveryQueue) { [unowned self] in
-            do {
-                try signposter.withIntervalSignpost("Load resources", id: signposter.makeSignpostID()) {
-                    try self.registerMiscResources()
-                }
-            } catch {
-                // Pipe the error out of the dispatch queue.
-                discoveryError.sync({
-                    if $0 == nil { $0 = error }
-                })
+        // Load resources like images and videos
+        async let loadResources: Void = try signposter.withIntervalSignpost("Load resources", id: signposter.makeSignpostID()) {
+            try autoreleasepool {
+                try self.registerMiscResources()
             }
         }
 
-        // Second, all the documents and symbols are added.
-        //
-        // Note: Documents and symbols may look up resources at this point but shouldn't lookup other documents or
-        //       symbols or attempt to resolve links/references since the topic graph may not contain all documents
-        //       or all symbols yet.
-        var result: (
-            tutorialTableOfContentsResults: [SemanticResult<TutorialTableOfContents>],
-            tutorials: [SemanticResult<Tutorial>],
-            tutorialArticles: [SemanticResult<TutorialArticle>],
-            articles: [SemanticResult<Article>],
-            documentationExtensions: [SemanticResult<Article>]
-        )!
-
-        discoveryGroup.async(queue: discoveryQueue) { [unowned self] in
-            do {
-                result = try signposter.withIntervalSignpost("Load documents", id: signposter.makeSignpostID()) {
-                    try self.registerDocuments()
-                }
-            } catch {
-                // Pipe the error out of the dispatch queue.
-                discoveryError.sync({
-                    if $0 == nil { $0 = error }
-                })
+        // Load documents
+        async let loadDocuments = try signposter.withIntervalSignpost("Load documents", id: signposter.makeSignpostID()) {
+            try autoreleasepool {
+                try self.registerDocuments()
             }
         }
 
-        discoveryGroup.async(queue: discoveryQueue) { [unowned self] in
-            do {
-                try signposter.withIntervalSignpost("Load external resolvers", id: signposter.makeSignpostID()) {
-                    try linkResolver.loadExternalResolvers(dependencyArchives: configuration.externalDocumentationConfiguration.dependencyArchives)
-                }
-            } catch {
-                // Pipe the error out of the dispatch queue.
-                discoveryError.sync({
-                    if $0 == nil { $0 = error }
-                })
+        // Load any external resolvers
+        async let loadExternalResolvers: Void = try signposter.withIntervalSignpost("Load external resolvers", id: signposter.makeSignpostID()) {
+            try autoreleasepool {
+                try linkResolver.loadExternalResolvers(dependencyArchives: configuration.externalDocumentationConfiguration.dependencyArchives)
             }
         }
 
-        discoveryGroup.wait()
-
-        try shouldContinueRegistration()
-
-        // Re-throw discovery errors
-        if let encounteredError = discoveryError.sync({ $0 }) {
-            throw encounteredError
-        }
-
         // All discovery went well, process the inputs.
-        let (tutorialTableOfContentsResults, tutorials, tutorialArticles, allArticles, documentationExtensions) = result
+        let (tutorialTableOfContentsResults, tutorials, tutorialArticles, allArticles, documentationExtensions) = try await loadDocuments
+        try shouldContinueRegistration()
         var (otherArticles, rootPageArticles) = splitArticles(allArticles)
 
         let globalOptions = (allArticles + documentationExtensions).compactMap { article in
@@ -2126,7 +2076,10 @@ public class DocumentationContext {
             options = globalOptions.first
         }
 
+        let (symbolGraphLoader, hierarchyBasedResolver, snippetResolver) = try await loadSymbols
+        try shouldContinueRegistration()
         self.linkResolver.localResolver = hierarchyBasedResolver
+        self.snippetResolver = snippetResolver
         hierarchyBasedResolver.addMappingForRoots(bundle: inputs)
         for tutorial in tutorials {
             hierarchyBasedResolver.addTutorial(tutorial)
@@ -2139,9 +2092,10 @@ public class DocumentationContext {
         }
 
         registerRootPages(from: rootPageArticles)
+
         try registerSymbols(symbolGraphLoader: symbolGraphLoader, documentationExtensions: documentationExtensions)
         // We don't need to keep the loader in memory after we've registered all symbols.
-        symbolGraphLoader = nil
+        _ = consume symbolGraphLoader
 
         try shouldContinueRegistration()
 
@@ -2168,12 +2122,17 @@ public class DocumentationContext {
             try shouldContinueRegistration()
         }
 
+        _ = try await loadExternalResolvers
+
         // Third, any processing that relies on resolving other content is done, mainly resolving links.
         preResolveExternalLinks(semanticObjects:
             tutorialTableOfContentsResults.map(referencedSemanticObject) +
             tutorials.map(referencedSemanticObject) +
             tutorialArticles.map(referencedSemanticObject))
 
+        // References to resources aren't used until the links are resolved
+        _ = try await loadResources
+
         resolveLinks(
             tutorialTableOfContents: tutorialTableOfContentsResults,
             tutorials: tutorials,

Sources/SwiftDocC/Utility/Collection+ConcurrentPerform.swift

@@ -144,3 +144,103 @@ extension Collection where Index == Int, Self: SendableMetatype {
         return allResults.sync({ $0 })
     }
 }
+
+extension Collection {
+    /// Concurrently performs work on slices of the collection's elements, combining the partial results into a final result.
+    ///
+    /// This method is intended as a building block that other higher-level `concurrent...` methods can be built upon.
+    /// That said, calling code can opt to use this method directly as opposed to writing overly specific single-use helper methods.
+    ///
+    /// - Parameters:
+    ///   - taskName: A human readable name of the tasks that the collection uses to perform this work.
+    ///   - batchWork: The concurrent work to perform on each slice of the collection's elements.
+    ///   - initialResult: The initial result to accumulate the partial results into.
+    ///   - combineResults: A closure that updates the accumulated result with a partial result from performing the work over one slice of the collection's elements.
+    /// - Returns: The final result of accumulating all partial results, out of order, into the initial result.
+    func _concurrentPerform<Result, PartialResult>(
+        taskName: String? = nil,
+        batchWork: (consuming SubSequence) throws -> PartialResult,
+        initialResult: Result,
+        combineResults: (inout Result, consuming PartialResult) -> Void
+    ) async throws -> Result {
+        try await withThrowingTaskGroup(of: PartialResult.self, returning: Result.self) { taskGroup in
+            try await withoutActuallyEscaping(batchWork) { work in
+                try await withoutActuallyEscaping(combineResults) { combineResults in
+                    var remaining = self[...]
+
+                    // Don't run more tasks in parallel than there are cores to run them
+                    let maxParallelTasks: Int = ProcessInfo.processInfo.processorCount
+                    // Finding the right number of tasks is a balancing act.
+                    // If the tasks are too small, then there's increased overhead from scheduling a lot of tasks and accumulating their results.
+                    // If the tasks are too large, then there's a risk that some tasks take longer to complete than others, increasing the amount of idle time.
+                    //
+                    // Here, we aim to schedule at most 10 tasks per core but create fewer tasks if the collection is fairly small to avoid some concurrent overhead.
+                    // The table below shows the approximate number of tasks per CPU core and the number of elements per task, within parenthesis,
+                    // for different collection sizes and number of CPU cores, given a minimum task size of 20 elements:
+                    //
+                    //               |     500    |    1000    |    2500    |    5000    |    10000    |    25000
+                    //     ----------|------------|------------|------------|------------|-------------|-------------
+                    //       8 cores |  ~3,2 (20) |  ~6,3 (20) |  ~9,8 (32) |  ~9,9 (63) |  ~9,9 (126) |  ~9,9 (313)
+                    //      12 cores |  ~2,1 (20) |  ~4,2 (20) | ~10,0 (21) | ~10,0 (42) | ~10,0  (84) | ~10,0 (209)
+                    //      16 cores |  ~1,6 (20) |  ~3,2 (20) |  ~7,9 (20) |  ~9,8 (32) |  ~9,9  (63) | ~10,0 (157)
+                    //      32 cores |  ~0,8 (20) |  ~1,6 (20) |  ~4,0 (20) |  ~7,9 (20) |  ~9,8  (32) |  ~9,9  (79)
+                    //
+                    let numberOfElementsPerTask: Int = Swift.max(
+                        Int(Double(remaining.count) / Double(maxParallelTasks * 10) + 1),
+                        20 // (this is a completely arbitrary task size threshold)
+                    )
+
+                    // Start the first round of work.
+                    // If the collection is big, this will add one task per core.
+                    // If the collection is small, this will only add a few tasks.
+                    for _ in 0..<maxParallelTasks {
+                        if !remaining.isEmpty {
+                            let slice = remaining.prefix(numberOfElementsPerTask)
+                            remaining = remaining.dropFirst(numberOfElementsPerTask)
+
+                            // Start work of one slice of the known pages
+                            #if compiler(<6.2)
+                            taskGroup.addTask {
+                                return try work(slice)
+                            }
+                            #else
+                            taskGroup.addTask(name: taskName) {
+                                return try work(slice)
+                            }
+                            #endif
+                        }
+                    }
+
+                    var result = initialResult
+
+                    for try await partialResult in taskGroup {
+                        // Check if the larger task group has been cancelled and if so, avoid doing any further work.
+                        try Task.checkCancellation()
+
+                        combineResults(&result, partialResult)
+
+                        // Now that one task has finished, and one core is available for work,
+                        // see if we have more slices to process and add one more task to process that slice.
+                        if !remaining.isEmpty {
+                            let slice = remaining.prefix(numberOfElementsPerTask)
+                            remaining = remaining.dropFirst(numberOfElementsPerTask)
+
+                            // Start work of one slice of the known pages
+                            #if compiler(<6.2)
+                            taskGroup.addTask {
+                                return try work(slice)
+                            }
+                            #else
+                            taskGroup.addTask(name: taskName) {
+                                return try work(slice)
+                            }
+                            #endif
+                        }
+                    }
+
+                    return result
+                }
+            }
+        }
+    }
+}

Tests/DocCCommandLineTests/FileWritingHTMLContentConsumerTests.swift

@@ -178,7 +178,7 @@ final class FileWritingHTMLContentConsumerTests: XCTestCase {
             prettyPrintOutput: true
         )
 
-        _ = try ConvertActionConverter.convert(
+        try await ConvertActionConverter.convert(
             context: context,
             outputConsumer: TestOutputConsumer(),
             htmlContentConsumer: htmlConsumer,
@@ -530,7 +530,7 @@ final class FileWritingHTMLContentConsumerTests: XCTestCase {
                     prettyPrintOutput: true
                 )
 
-                _ = try ConvertActionConverter.convert(
+                try await ConvertActionConverter.convert(
                     context: context,
                     outputConsumer: TestOutputConsumer(),
                     htmlContentConsumer: htmlConsumer,

Tests/DocCCommandLineTests/MergeActionTests.swift

@@ -940,8 +940,7 @@ class MergeActionTests: XCTestCase {
 
             let outputConsumer = ConvertFileWritingConsumer(targetFolder: outputPath, bundleRootFolder: catalogDir, fileManager: fileSystem, context: context, indexer: indexer, transformForStaticHostingIndexHTML: nil, bundleID: inputs.id)
 
-            let convertProblems = try ConvertActionConverter.convert(context: context, outputConsumer: outputConsumer, htmlContentConsumer: nil, sourceRepository: nil, emitDigest: false, documentationCoverageOptions: .noCoverage)
-            XCTAssert(convertProblems.isEmpty, "Unexpected problems: \(context.problems.map(\.diagnostic.summary).joined(separator: "\n"))", file: file, line: line)
+            try await ConvertActionConverter.convert(context: context, outputConsumer: outputConsumer, htmlContentConsumer: nil, sourceRepository: nil, emitDigest: false, documentationCoverageOptions: .noCoverage)
 
             let navigatorProblems = indexer.finalize(emitJSON: true, emitLMDB: false)
             XCTAssert(navigatorProblems.isEmpty, "Unexpected problems: \(context.problems.map(\.diagnostic.summary).joined(separator: "\n"))", file: file, line: line)