From 82e15c13ce710ce49acafc3198983a027c7a9e7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 4 Dec 2024 18:50:33 +0100 Subject: [PATCH 1/3] Enable external link support by default --- .../ConvertActionConverter.swift | 4 +-- .../Link Resolution/PathHierarchy+Find.swift | 2 +- Sources/SwiftDocC/Utility/FeatureFlags.swift | 10 +++++-- .../ConvertAction+CommandInitialization.swift | 2 +- .../ArgumentParsing/Subcommands/Convert.swift | 21 +++++++++++--- .../DocumentationContextTests.swift | 2 -- .../ExternalPathHierarchyResolverTests.swift | 5 ---- .../Infrastructure/PathHierarchyTests.swift | 4 --- .../ConvertSubcommandTests.swift | 28 +++++++++++++++---- .../ConvertActionStaticHostableTests.swift | 6 +++- .../ConvertActionTests.swift | 3 +- 11 files changed, 59 insertions(+), 28 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift b/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift index 17a5db0a7..449f9f46e 100644 --- a/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift +++ b/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift @@ -156,7 +156,7 @@ package enum ConvertActionConverter { linkSummaries.append(contentsOf: nodeLinkSummaries) indexingRecords.append(contentsOf: nodeIndexingRecords) } - } else if FeatureFlags.current.isExperimentalLinkHierarchySerializationEnabled { + } else if FeatureFlags.current.isLinkHierarchySerializationEnabled { let nodeLinkSummaries = entity.externallyLinkableElementSummaries(context: context, renderNode: renderNode, includeTaskGroups: false) resultsGroup.async(queue: resultsSyncQueue) { @@ -189,7 +189,7 @@ package enum ConvertActionConverter { } } - if FeatureFlags.current.isExperimentalLinkHierarchySerializationEnabled { + if FeatureFlags.current.isLinkHierarchySerializationEnabled { signposter.withIntervalSignpost("Serialize link hierarchy", id: signposter.makeSignpostID()) { do { let serializableLinkInformation = try context.linkResolver.localResolver.prepareForSerialization(bundleID: bundle.id) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift index 0ead6bbe1..edef2315a 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift @@ -113,7 +113,7 @@ extension PathHierarchy { } let topLevelNames = Set(modules.map(\.name) + (onlyFindSymbols ? [] : [articlesContainer.name, tutorialContainer.name])) - if isAbsolute, FeatureFlags.current.isExperimentalLinkHierarchySerializationEnabled { + if isAbsolute, FeatureFlags.current.isLinkHierarchySerializationEnabled { throw Error.moduleNotFound( pathPrefix: pathForError(of: rawPath, droppingLast: remaining.count), remaining: Array(remaining), diff --git a/Sources/SwiftDocC/Utility/FeatureFlags.swift b/Sources/SwiftDocC/Utility/FeatureFlags.swift index def7e642d..8c7da6176 100644 --- a/Sources/SwiftDocC/Utility/FeatureFlags.swift +++ b/Sources/SwiftDocC/Utility/FeatureFlags.swift @@ -17,8 +17,14 @@ public struct FeatureFlags: Codable { /// Whether or not experimental support for device frames on images and video is enabled. public var isExperimentalDeviceFrameSupportEnabled = false - /// Whether or not experimental support for emitting a serialized version of the local link resolution information is enabled. - public var isExperimentalLinkHierarchySerializationEnabled = false + /// Whether or not support for emitting a serialized version of the local link resolution information is enabled. + public var isLinkHierarchySerializationEnabled = true + + @available(*, deprecated, renamed: "isLinkHierarchySerializationEnabled", message: "Use 'isLinkHierarchySerializationEnabled' instead. This deprecated API will be removed after 6.2 is released") + public var isExperimentalLinkHierarchySerializationEnabled: Bool { + get { isLinkHierarchySerializationEnabled } + set { isLinkHierarchySerializationEnabled = newValue } + } /// Whether or not experimental support for combining overloaded symbol pages is enabled. public var isExperimentalOverloadedSymbolPresentationEnabled = false diff --git a/Sources/SwiftDocCUtilities/ArgumentParsing/ActionExtensions/ConvertAction+CommandInitialization.swift b/Sources/SwiftDocCUtilities/ArgumentParsing/ActionExtensions/ConvertAction+CommandInitialization.swift index e8c8a31b4..2308ebbe4 100644 --- a/Sources/SwiftDocCUtilities/ArgumentParsing/ActionExtensions/ConvertAction+CommandInitialization.swift +++ b/Sources/SwiftDocCUtilities/ArgumentParsing/ActionExtensions/ConvertAction+CommandInitialization.swift @@ -21,7 +21,7 @@ extension ConvertAction { let outOfProcessResolver: OutOfProcessReferenceResolver? FeatureFlags.current.isExperimentalDeviceFrameSupportEnabled = convert.enableExperimentalDeviceFrameSupport - FeatureFlags.current.isExperimentalLinkHierarchySerializationEnabled = convert.enableExperimentalLinkHierarchySerialization + FeatureFlags.current.isLinkHierarchySerializationEnabled = convert.enableLinkHierarchySerialization FeatureFlags.current.isExperimentalOverloadedSymbolPresentationEnabled = convert.enableExperimentalOverloadedSymbolPresentation FeatureFlags.current.isMentionedInEnabled = convert.enableMentionedIn FeatureFlags.current.isParametersAndReturnsValidationEnabled = convert.enableParametersAndReturnsValidation diff --git a/Sources/SwiftDocCUtilities/ArgumentParsing/Subcommands/Convert.swift b/Sources/SwiftDocCUtilities/ArgumentParsing/Subcommands/Convert.swift index 95b0d098c..04b27e826 100644 --- a/Sources/SwiftDocCUtilities/ArgumentParsing/Subcommands/Convert.swift +++ b/Sources/SwiftDocCUtilities/ArgumentParsing/Subcommands/Convert.swift @@ -486,13 +486,19 @@ extension Docc { var allowArbitraryCatalogDirectories = false @Flag( - name: .customLong("enable-experimental-external-link-support"), + name: .customLong("external-link-support"), + inversion: .prefixedEnableDisable, help: ArgumentHelp("Support external links to this documentation output.", discussion: """ Write additional link metadata files to the output directory to support resolving documentation links to the documentation in that output directory. """) ) + var enableLinkHierarchySerialization = true + + // This flag only exist to allow developers to pass the previous '--enable-experimental-...' flag without errors. + @Flag(name: .customLong("enable-experimental-external-link-support"), help: .hidden) + @available(*, deprecated, message: "This deprecated API will be removed after 6.2 is released") var enableExperimentalLinkHierarchySerialization = false - + @Flag(help: .hidden) var experimentalModifyCatalogWithGeneratedCuration = false @@ -544,6 +550,7 @@ extension Docc { Convert.warnAboutDeprecatedOptionIfNeeded("experimental-parse-doxygen-commands", message: "This flag has no effect. Doxygen support is enabled by default.") Convert.warnAboutDeprecatedOptionIfNeeded("enable-experimental-parameters-and-returns-validation", message: "This flag has no effect. Parameter and return value validation is enabled by default.") Convert.warnAboutDeprecatedOptionIfNeeded("enable-experimental-mentioned-in", message: "This flag has no effect. Automatic mentioned in sections is enabled by default.") + Convert.warnAboutDeprecatedOptionIfNeeded("enable-experimental-external-link-support", message: "This flag has no effect. External link support is enabled by default.") Convert.warnAboutDeprecatedOptionIfNeeded("index", message: "Use '--emit-lmdb-index' indead.") emitLMDBIndex = emitLMDBIndex } @@ -583,9 +590,15 @@ extension Docc { } /// A user-provided value that is true if the user enables experimental serialization of the local link resolution information. + public var enableLinkHierarchySerialization: Bool { + get { featureFlags.enableLinkHierarchySerialization } + set { featureFlags.enableLinkHierarchySerialization = newValue } + } + + @available(*, deprecated, renamed: "enableLinkHierarchySerialization", message: "Use 'enableLinkHierarchySerialization' instead. This deprecated API will be removed after 6.2 is released") public var enableExperimentalLinkHierarchySerialization: Bool { - get { featureFlags.enableExperimentalLinkHierarchySerialization } - set { featureFlags.enableExperimentalLinkHierarchySerialization = newValue } + get { enableLinkHierarchySerialization } + set { enableLinkHierarchySerialization = newValue } } /// A user-provided value that is true if the user wants to in-place modify the provided documentation catalog to write generated curation to documentation extension files. diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift index 11326bef8..c9d79fa51 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift @@ -5471,8 +5471,6 @@ let expected = """ } func testResolveExternalLinkFromTechnologyRoot() async throws { - enableFeatureFlag(\.isExperimentalLinkHierarchySerializationEnabled) - let externalModuleName = "ExternalModuleName" func makeExternalDependencyFiles() async throws -> (SerializableLinkResolutionInformation, [LinkDestinationSummary]) { diff --git a/Tests/SwiftDocCTests/Infrastructure/ExternalPathHierarchyResolverTests.swift b/Tests/SwiftDocCTests/Infrastructure/ExternalPathHierarchyResolverTests.swift index 7af1fa06e..3b4c1b9fe 100644 --- a/Tests/SwiftDocCTests/Infrastructure/ExternalPathHierarchyResolverTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/ExternalPathHierarchyResolverTests.swift @@ -16,11 +16,6 @@ import SwiftDocCTestUtilities class ExternalPathHierarchyResolverTests: XCTestCase { - override func setUp() { - super.setUp() - enableFeatureFlag(\.isExperimentalLinkHierarchySerializationEnabled) - } - // These tests resolve absolute symbol links in both a local and external context to verify that external links work the same local links. func testUnambiguousAbsolutePaths() async throws { diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index 2f3a3014e..75e528a25 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -287,8 +287,6 @@ class PathHierarchyTests: XCTestCase { } func testAmbiguousPaths() async throws { - enableFeatureFlag(\.isExperimentalLinkHierarchySerializationEnabled) - let (_, context) = try await testBundleAndContext(named: "MixedLanguageFrameworkWithLanguageRefinements") let tree = context.linkResolver.localResolver.pathHierarchy @@ -4432,8 +4430,6 @@ class PathHierarchyTests: XCTestCase { } func testResolveExternalLinkFromTechnologyRoot() async throws { - enableFeatureFlag(\.isExperimentalLinkHierarchySerializationEnabled) - let catalog = Folder(name: "unit-test.docc", content: [ TextFile(name: "Root.md", utf8Content: """ # Some root page diff --git a/Tests/SwiftDocCUtilitiesTests/ArgumentParsing/ConvertSubcommandTests.swift b/Tests/SwiftDocCUtilitiesTests/ArgumentParsing/ConvertSubcommandTests.swift index 41027fbba..da5f2c638 100644 --- a/Tests/SwiftDocCUtilitiesTests/ArgumentParsing/ConvertSubcommandTests.swift +++ b/Tests/SwiftDocCUtilitiesTests/ArgumentParsing/ConvertSubcommandTests.swift @@ -367,16 +367,16 @@ class ConvertSubcommandTests: XCTestCase { let commandWithoutFlag = try Docc.Convert.parse([testBundleURL.path]) _ = try ConvertAction(fromConvertCommand: commandWithoutFlag) - XCTAssertFalse(commandWithoutFlag.enableExperimentalLinkHierarchySerialization) - XCTAssertFalse(FeatureFlags.current.isExperimentalLinkHierarchySerializationEnabled) + XCTAssertTrue(commandWithoutFlag.enableLinkHierarchySerialization) + XCTAssertTrue(FeatureFlags.current.isLinkHierarchySerializationEnabled) let commandWithFlag = try Docc.Convert.parse([ - "--enable-experimental-external-link-support", + "--disable-external-link-support", testBundleURL.path, ]) _ = try ConvertAction(fromConvertCommand: commandWithFlag) - XCTAssertTrue(commandWithFlag.enableExperimentalLinkHierarchySerialization) - XCTAssertTrue(FeatureFlags.current.isExperimentalLinkHierarchySerializationEnabled) + XCTAssertFalse(commandWithFlag.enableLinkHierarchySerialization) + XCTAssertFalse(FeatureFlags.current.isLinkHierarchySerializationEnabled) } func testExperimentalEnableOverloadedSymbolPresentation() throws { @@ -596,6 +596,24 @@ class ConvertSubcommandTests: XCTestCase { let disabledFlagConvert = try Docc.Convert.parse(["--disable-mentioned-in"]) XCTAssertEqual(disabledFlagConvert.enableMentionedIn, false) } + + func testExternalLinkSupportFlag() throws { + // The feature is enabled when no flag is passed. + let noFlagConvert = try Docc.Convert.parse([]) + XCTAssertEqual(noFlagConvert.enableLinkHierarchySerialization, true) + + // It's allowed to pass the previous "--enable-experimental-..." flag. + let oldFlagConvert = try Docc.Convert.parse(["--enable-experimental-external-link-support"]) + XCTAssertEqual(oldFlagConvert.enableLinkHierarchySerialization, true) + + // It's allowed to pass the redundant "--enable-..." flag. + let enabledFlagConvert = try Docc.Convert.parse(["--enable-external-link-support"]) + XCTAssertEqual(enabledFlagConvert.enableLinkHierarchySerialization, true) + + // Passing the "--disable-..." flag turns of the feature. + let disabledFlagConvert = try Docc.Convert.parse(["--disable-external-link-support"]) + XCTAssertEqual(disabledFlagConvert.enableLinkHierarchySerialization, false) + } // This test calls ``ConvertOptions.infoPlistFallbacks._unusedVersionForBackwardsCompatibility`` which is deprecated. // Deprecating the test silences the deprecation warning when running the tests. It doesn't skip the test. diff --git a/Tests/SwiftDocCUtilitiesTests/ConvertActionStaticHostableTests.swift b/Tests/SwiftDocCUtilitiesTests/ConvertActionStaticHostableTests.swift index 0c2c0498b..12d75e75b 100644 --- a/Tests/SwiftDocCUtilitiesTests/ConvertActionStaticHostableTests.swift +++ b/Tests/SwiftDocCUtilitiesTests/ConvertActionStaticHostableTests.swift @@ -47,7 +47,11 @@ class ConvertActionStaticHostableTests: StaticHostingBaseTests { _ = try await action.perform(logHandle: .none) // Test the content of the output folder. - var expectedContent = ["data", "documentation", "tutorials", "downloads", "images", "metadata.json" ,"videos", "index.html", "index"] + var expectedContent = [ + "data", "documentation", "tutorials", "downloads", "images", "videos", + "index.html", "index", + "metadata.json", "link-hierarchy.json", "linkable-entities.json" + ] expectedContent += templateFolder.content.filter { $0 is Folder }.map{ $0.name } let output = try fileManager.contentsOfDirectory(atPath: targetBundleURL.path) diff --git a/Tests/SwiftDocCUtilitiesTests/ConvertActionTests.swift b/Tests/SwiftDocCUtilitiesTests/ConvertActionTests.swift index 85fc99fa1..2217e4b2e 100644 --- a/Tests/SwiftDocCUtilitiesTests/ConvertActionTests.swift +++ b/Tests/SwiftDocCUtilitiesTests/ConvertActionTests.swift @@ -1559,7 +1559,6 @@ class ConvertActionTests: XCTestCase { XCTAssertFalse(testDataProvider.fileExists(atPath: result.outputs[0].appendingPathComponent("assets.json").path)) XCTAssertFalse(testDataProvider.fileExists(atPath: result.outputs[0].appendingPathComponent("diagnostics.json").path)) XCTAssertFalse(testDataProvider.fileExists(atPath: result.outputs[0].appendingPathComponent("indexing-records.json").path)) - XCTAssertFalse(testDataProvider.fileExists(atPath: result.outputs[0].appendingPathComponent("linkable-entities.json").path)) } } @@ -3164,6 +3163,8 @@ class ConvertActionTests: XCTestCase { │ ├─ image-name@2x.png │ ├─ image-name~dark.png │ ╰─ image-name~dark@2x.png + ├─ link-hierarchy.json + ├─ linkable-entities.json ├─ metadata.json ╰─ videos/ ╰─ unit-test/ From 0a469380e8d7e4e928e20459657d9f90e25e3706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Thu, 5 Dec 2024 13:34:10 +0100 Subject: [PATCH 2/3] Serialize linkable entities incrementally for performance --- .../ConvertActionConverter.swift | 13 ++++----- .../ConvertOutputConsumer.swift | 13 +++++++++ Sources/SwiftDocC/Utility/FeatureFlags.swift | 2 +- .../Convert/ConvertFileWritingConsumer.swift | 27 +++++++++++++++++++ .../ArgumentParsing/Subcommands/Convert.swift | 4 +-- .../TestRenderNodeOutputConsumer.swift | 2 ++ 6 files changed, 52 insertions(+), 9 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift b/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift index 449f9f46e..28e16e1a3 100644 --- a/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift +++ b/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift @@ -77,7 +77,6 @@ package enum ConvertActionConverter { // Arrays to gather additional metadata if `emitDigest` is `true`. var indexingRecords = [IndexingRecord]() - var linkSummaries = [LinkDestinationSummary]() var assets = [RenderReferenceType : [any RenderReference]]() var coverageInfo = [CoverageDataEntry]() let coverageFilterClosure = documentationCoverageOptions.generateFilterClosure() @@ -151,16 +150,18 @@ package enum ConvertActionConverter { let nodeLinkSummaries = entity.externallyLinkableElementSummaries(context: context, renderNode: renderNode, includeTaskGroups: true) let nodeIndexingRecords = try renderNode.indexingRecords(onPage: identifier) + for linkSummary in nodeLinkSummaries { + try outputConsumer.consumeIncremental(linkableElementSummary: linkSummary) + } resultsGroup.async(queue: resultsSyncQueue) { assets.merge(renderNode.assetReferences, uniquingKeysWith: +) - linkSummaries.append(contentsOf: nodeLinkSummaries) indexingRecords.append(contentsOf: nodeIndexingRecords) } } else if FeatureFlags.current.isLinkHierarchySerializationEnabled { let nodeLinkSummaries = entity.externallyLinkableElementSummaries(context: context, renderNode: renderNode, includeTaskGroups: false) - resultsGroup.async(queue: resultsSyncQueue) { - linkSummaries.append(contentsOf: nodeLinkSummaries) + for linkSummary in nodeLinkSummaries { + try outputConsumer.consumeIncremental(linkableElementSummary: linkSummary) } } } catch { @@ -180,7 +181,7 @@ package enum ConvertActionConverter { if emitDigest { signposter.withIntervalSignpost("Emit digest", id: signposter.makeSignpostID()) { do { - try outputConsumer.consume(linkableElementSummaries: linkSummaries) + try outputConsumer.finishedConsumingLinkElementSummaries() try outputConsumer.consume(indexingRecords: indexingRecords) try outputConsumer.consume(assets: assets) } catch { @@ -196,7 +197,7 @@ package enum ConvertActionConverter { try outputConsumer.consume(linkResolutionInformation: serializableLinkInformation) if !emitDigest { - try outputConsumer.consume(linkableElementSummaries: linkSummaries) + try outputConsumer.finishedConsumingLinkElementSummaries() } } catch { recordProblem(from: error, in: &conversionProblems, withIdentifier: "link-resolver") diff --git a/Sources/SwiftDocC/Infrastructure/ConvertOutputConsumer.swift b/Sources/SwiftDocC/Infrastructure/ConvertOutputConsumer.swift index f5e1ebd43..c7884e808 100644 --- a/Sources/SwiftDocC/Infrastructure/ConvertOutputConsumer.swift +++ b/Sources/SwiftDocC/Infrastructure/ConvertOutputConsumer.swift @@ -27,6 +27,12 @@ public protocol ConvertOutputConsumer { func consume(assetsInBundle bundle: DocumentationBundle) throws /// Consumes the linkable element summaries produced during a conversion. + /// > Warning: This method might be called concurrently. + func consumeIncremental(linkableElementSummary: LinkDestinationSummary) throws + /// Consumes the linkable element summaries produced during a conversion. + func finishedConsumingLinkElementSummaries() throws + + @available(*, deprecated, renamed: "consume(linkableElementSummary:)", message: "Use 'consume(linkableElementSummary:)' instead. This deprecated API will be removed after 6.3 is released") func consume(linkableElementSummaries: [LinkDestinationSummary]) throws /// Consumes the indexing records produced during a conversion. @@ -60,8 +66,15 @@ public extension ConvertOutputConsumer { func consume(linkResolutionInformation: SerializableLinkResolutionInformation) throws {} } +// Default implementations to avoid a source breaking change from introducing new protocol requirements +public extension ConvertOutputConsumer { + func consumeIncremental(linkableElementSummary: LinkDestinationSummary) throws {} + func finishedConsumingLinkElementSummaries() throws {} +} + // Default implementation so that conforming types don't need to implement deprecated API. public extension ConvertOutputConsumer { + func consume(linkableElementSummaries: [LinkDestinationSummary]) throws {} func _deprecated_consume(problems: [Problem]) throws {} } diff --git a/Sources/SwiftDocC/Utility/FeatureFlags.swift b/Sources/SwiftDocC/Utility/FeatureFlags.swift index 8c7da6176..5bd8948ec 100644 --- a/Sources/SwiftDocC/Utility/FeatureFlags.swift +++ b/Sources/SwiftDocC/Utility/FeatureFlags.swift @@ -20,7 +20,7 @@ public struct FeatureFlags: Codable { /// Whether or not support for emitting a serialized version of the local link resolution information is enabled. public var isLinkHierarchySerializationEnabled = true - @available(*, deprecated, renamed: "isLinkHierarchySerializationEnabled", message: "Use 'isLinkHierarchySerializationEnabled' instead. This deprecated API will be removed after 6.2 is released") + @available(*, deprecated, renamed: "isLinkHierarchySerializationEnabled", message: "Use 'isLinkHierarchySerializationEnabled' instead. This deprecated API will be removed after 6.3 is released") public var isExperimentalLinkHierarchySerializationEnabled: Bool { get { isLinkHierarchySerializationEnabled } set { isLinkHierarchySerializationEnabled = newValue } diff --git a/Sources/SwiftDocCUtilities/Action/Actions/Convert/ConvertFileWritingConsumer.swift b/Sources/SwiftDocCUtilities/Action/Actions/Convert/ConvertFileWritingConsumer.swift index 56e492585..ed2bb7a75 100644 --- a/Sources/SwiftDocCUtilities/Action/Actions/Convert/ConvertFileWritingConsumer.swift +++ b/Sources/SwiftDocCUtilities/Action/Actions/Convert/ConvertFileWritingConsumer.swift @@ -152,6 +152,33 @@ struct ConvertFileWritingConsumer: ConvertOutputConsumer, ExternalNodeConsumer { } } + private var linkableElementsData = Synchronized(Data()) + + /// Consumes one linkable element summary produced during a conversion. + func consumeIncremental(linkableElementSummary: LinkDestinationSummary) throws { + let data = try encode(linkableElementSummary) + linkableElementsData.sync { + if !$0.isEmpty { + $0.append(Data(",".utf8)) + } + $0.append(data) + } + } + + /// Finishes consuming the linkable element summaries produced during a conversion. + func finishedConsumingLinkElementSummaries() throws { + let linkableElementsURL = targetFolder.appendingPathComponent(Self.linkableEntitiesFileName, isDirectory: false) + let data = linkableElementsData.sync { accumulatedData in + var data = Data() + swap(&data, &accumulatedData) + data.insert(UTF8.CodeUnit(ascii: "["), at: 0) + data.append(UTF8.CodeUnit(ascii: "]")) + + return data + } + try fileManager.createFile(at: linkableElementsURL, contents: data) + } + func consume(linkableElementSummaries summaries: [LinkDestinationSummary]) throws { let linkableElementsURL = targetFolder.appendingPathComponent(Self.linkableEntitiesFileName, isDirectory: false) let data = try encode(summaries) diff --git a/Sources/SwiftDocCUtilities/ArgumentParsing/Subcommands/Convert.swift b/Sources/SwiftDocCUtilities/ArgumentParsing/Subcommands/Convert.swift index 04b27e826..739cb4c71 100644 --- a/Sources/SwiftDocCUtilities/ArgumentParsing/Subcommands/Convert.swift +++ b/Sources/SwiftDocCUtilities/ArgumentParsing/Subcommands/Convert.swift @@ -496,7 +496,7 @@ extension Docc { // This flag only exist to allow developers to pass the previous '--enable-experimental-...' flag without errors. @Flag(name: .customLong("enable-experimental-external-link-support"), help: .hidden) - @available(*, deprecated, message: "This deprecated API will be removed after 6.2 is released") + @available(*, deprecated, message: "This deprecated API will be removed after 6.3 is released") var enableExperimentalLinkHierarchySerialization = false @Flag(help: .hidden) @@ -595,7 +595,7 @@ extension Docc { set { featureFlags.enableLinkHierarchySerialization = newValue } } - @available(*, deprecated, renamed: "enableLinkHierarchySerialization", message: "Use 'enableLinkHierarchySerialization' instead. This deprecated API will be removed after 6.2 is released") + @available(*, deprecated, renamed: "enableLinkHierarchySerialization", message: "Use 'enableLinkHierarchySerialization' instead. This deprecated API will be removed after 6.3 is released") public var enableExperimentalLinkHierarchySerialization: Bool { get { enableLinkHierarchySerialization } set { enableLinkHierarchySerialization = newValue } diff --git a/Tests/SwiftDocCTests/TestRenderNodeOutputConsumer.swift b/Tests/SwiftDocCTests/TestRenderNodeOutputConsumer.swift index 67c0f5e3a..ec6b2813a 100644 --- a/Tests/SwiftDocCTests/TestRenderNodeOutputConsumer.swift +++ b/Tests/SwiftDocCTests/TestRenderNodeOutputConsumer.swift @@ -23,6 +23,8 @@ class TestRenderNodeOutputConsumer: ConvertOutputConsumer, ExternalNodeConsumer func consume(assetsInBundle bundle: DocumentationBundle) throws { } func consume(linkableElementSummaries: [LinkDestinationSummary]) throws { } + func consumeIncremental(linkableElementSummary: LinkDestinationSummary) throws { } + func finishedConsumingLinkElementSummaries() throws { } func consume(indexingRecords: [IndexingRecord]) throws { } func consume(assets: [RenderReferenceType: [any RenderReference]]) throws { } func consume(benchmarks: Benchmark) throws { } From 767294c5fc88157675296ff9f444e16e8067162a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Thu, 5 Dec 2024 13:41:36 +0100 Subject: [PATCH 3/3] Avoid waiting to serializer the link hierarchy --- .../ConvertActionConverter.swift | 53 +++++++++---------- .../Action/Actions/Merge/MergeAction.swift | 2 + .../MergeActionTests.swift | 4 ++ 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift b/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift index 28e16e1a3..a4839b426 100644 --- a/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift +++ b/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift @@ -112,6 +112,22 @@ package enum ConvertActionConverter { try outputConsumer.consume(externalRenderNode: externalRenderNode) } + let linkHierarchySerializationProblems = Synchronized<[Problem]>([]) + if FeatureFlags.current.isLinkHierarchySerializationEnabled { + resultsGroup.async(queue: resultsSyncQueue) { + signposter.withIntervalSignpost("Serialize link hierarchy", id: signposter.makeSignpostID()) { + do { + let serializableLinkInformation = try context.linkResolver.localResolver.prepareForSerialization(bundleID: bundle.id) + try outputConsumer.consume(linkResolutionInformation: serializableLinkInformation) + } catch { + linkHierarchySerializationProblems.sync { + recordProblem(from: error, in: &$0, withIdentifier: "link-resolver") + } + } + } + } + } + let renderSignpostHandle = signposter.beginInterval("Render", id: signposter.makeSignpostID(), "Render \(context.knownPages.count) pages") var conversionProblems: [Problem] = context.knownPages.concurrentPerform { identifier, results in @@ -173,44 +189,25 @@ package enum ConvertActionConverter { // Wait for any concurrent updates to complete. resultsGroup.wait() + conversionProblems += linkHierarchySerializationProblems.sync { $0 } + signposter.endInterval("Render", renderSignpostHandle) guard !Task.isCancelled else { return [] } // Write various metadata - if emitDigest { + if emitDigest || FeatureFlags.current.isLinkHierarchySerializationEnabled { signposter.withIntervalSignpost("Emit digest", id: signposter.makeSignpostID()) { do { try outputConsumer.finishedConsumingLinkElementSummaries() - try outputConsumer.consume(indexingRecords: indexingRecords) - try outputConsumer.consume(assets: assets) - } catch { - recordProblem(from: error, in: &conversionProblems, withIdentifier: "metadata") - } - } - } - - if FeatureFlags.current.isLinkHierarchySerializationEnabled { - signposter.withIntervalSignpost("Serialize link hierarchy", id: signposter.makeSignpostID()) { - do { - let serializableLinkInformation = try context.linkResolver.localResolver.prepareForSerialization(bundleID: bundle.id) - try outputConsumer.consume(linkResolutionInformation: serializableLinkInformation) - - if !emitDigest { - try outputConsumer.finishedConsumingLinkElementSummaries() + if emitDigest { + // Only emit the other digest files if `--emit-digest` is passed + try outputConsumer.consume(indexingRecords: indexingRecords) + try outputConsumer.consume(assets: assets) + try (_Deprecated(outputConsumer) as (any _DeprecatedConsumeProblemsAccess))._consume(problems: context.problems + conversionProblems) } } catch { - recordProblem(from: error, in: &conversionProblems, withIdentifier: "link-resolver") - } - } - } - - if emitDigest { - signposter.withIntervalSignpost("Emit digest", id: signposter.makeSignpostID()) { - do { - try (_Deprecated(outputConsumer) as (any _DeprecatedConsumeProblemsAccess))._consume(problems: context.problems + conversionProblems) - } catch { - recordProblem(from: error, in: &conversionProblems, withIdentifier: "problems") + recordProblem(from: error, in: &conversionProblems, withIdentifier: "metadata") } } } diff --git a/Sources/SwiftDocCUtilities/Action/Actions/Merge/MergeAction.swift b/Sources/SwiftDocCUtilities/Action/Actions/Merge/MergeAction.swift index cc94bc1b8..0e2ed913f 100644 --- a/Sources/SwiftDocCUtilities/Action/Actions/Merge/MergeAction.swift +++ b/Sources/SwiftDocCUtilities/Action/Actions/Merge/MergeAction.swift @@ -78,6 +78,8 @@ struct MergeAction: AsyncAction { } let renderIndex = try JSONDecoder().decode(RenderIndex.self, from: jsonIndexData) + // TODO: Combine link-hierarchy.json, linkable-entities.json, and metadata.json + try combinedJSONIndex.merge(renderIndex) } diff --git a/Tests/SwiftDocCUtilitiesTests/MergeActionTests.swift b/Tests/SwiftDocCUtilitiesTests/MergeActionTests.swift index 0853d154e..29fb222c8 100644 --- a/Tests/SwiftDocCUtilitiesTests/MergeActionTests.swift +++ b/Tests/SwiftDocCUtilitiesTests/MergeActionTests.swift @@ -925,6 +925,8 @@ class MergeActionTests: XCTestCase { │ ╰─ \(name.lowercased())-card.png ├─ index/ │ ╰─ index.json + ├─ link-hierarchy.json + ├─ linkable-entities.json ├─ metadata.json ╰─ videos/ ╰─ \(name) @@ -971,6 +973,8 @@ class MergeActionTests: XCTestCase { │ ╰─ second-card.png ├─ index/ │ ╰─ index.json + ├─ link-hierarchy.json + ├─ linkable-entities.json ├─ metadata.json ╰─ videos/ ├─ First/