From 5a68281ee4cea2c9e3b2d03e476399921f549278 Mon Sep 17 00:00:00 2001 From: Ethan Kusters Date: Sun, 22 May 2022 13:40:50 -0700 Subject: [PATCH] Navigator index generation performance improvements The primary performance win here comes from updating the logic that creates a normalized navigator index identifier from a ResolvedTopicReference to not rely on creating an additional ResolvedTopicReference. --- .../Navigator/NavigatorIndex+Ext.swift | 16 ++++-- .../Indexing/Navigator/NavigatorIndex.swift | 50 ++++++----------- .../Indexing/NavigatorIndexTests.swift | 53 +++++++++++++++++++ 3 files changed, 83 insertions(+), 36 deletions(-) diff --git a/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex+Ext.swift b/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex+Ext.swift index 4f19b3718b..cd3e468421 100644 --- a/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex+Ext.swift +++ b/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex+Ext.swift @@ -75,6 +75,9 @@ public class FileSystemRenderNodeProvider: RenderNodeProvider { } extension RenderNode { + private static let typesThatShouldNotUseNavigatorTitle: Set = [ + .framework, .class, .structure, .enumeration, .protocol, .typeAlias, .associatedType + ] /// Returns a navigator title preferring the fragments inside the metadata, if applicable. func navigatorTitle() -> String? { @@ -83,7 +86,9 @@ extension RenderNode { // FIXME: Use `metadata.navigatorTitle` for all Swift symbols (github.com/apple/swift-docc/issues/176). if identifier.sourceLanguage == .swift || (metadata.navigatorTitle ?? []).isEmpty { let pageType = navigatorPageType() - guard ![.framework, .class, .structure, .enumeration, .protocol, .typeAlias, .associatedType].contains(pageType) else { return metadata.title } + guard !Self.typesThatShouldNotUseNavigatorTitle.contains(pageType) else { + return metadata.title + } fragments = metadata.fragments } else { fragments = metadata.navigatorTitle @@ -96,8 +101,13 @@ extension RenderNode { public func navigatorPageType() -> NavigatorIndex.PageType { // This is a workaround to support plist keys. - if metadata.roleHeading?.lowercased() == "property list key" { return .propertyListKey } - else if metadata.roleHeading?.lowercased() == "property list key reference" { return .propertyListKeyReference } + if let roleHeading = metadata.roleHeading?.lowercased() { + if roleHeading == "property list key" { + return .propertyListKey + } else if roleHeading == "property list key reference" { + return .propertyListKeyReference + } + } switch self.kind { case .article: diff --git a/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift b/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift index 40afdd43f6..8385992f52 100644 --- a/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift +++ b/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift @@ -409,12 +409,14 @@ public class NavigatorIndex { } extension ResolvedTopicReference { - func navigatorIndexIdentifier( + func normalizedNavigatorIndexIdentifier( forLanguage languageIdentifier: InterfaceLanguage.ID ) -> NavigatorIndex.Identifier { + let normalizedPath = NodeURLGenerator.fileSafeReferencePath(self, lowercased: true) + return NavigatorIndex.Identifier( - bundleIdentifier: bundleIdentifier, - path: path, + bundleIdentifier: bundleIdentifier.lowercased(), + path: "/" + normalizedPath, fragment: fragment, languageIdentifier: languageIdentifier ) @@ -586,10 +588,6 @@ extension NavigatorIndex { throw Error.navigatorIndexIsNil } - guard let title = (usePageTitle) ? renderNode.metadata.title : renderNode.navigatorTitle() else { - throw Error.missingTitle(description: "\(renderNode.identifier.absoluteString.singleQuoted) has an empty title and so can't have a usable entry in the index.") - } - // Process the language let interfaceLanguage = renderNode.identifier.sourceLanguage let interfaceLanguageID = interfaceLanguage.id.lowercased() @@ -605,13 +603,20 @@ extension NavigatorIndex { idToLanguage[interfaceLanguageID] = language } - let identifier = renderNode.identifier.navigatorIndexIdentifier(forLanguage: language.mask) - guard identifierToNode[identifier] == nil else { + let normalizedIdentifier = renderNode + .identifier + .normalizedNavigatorIndexIdentifier(forLanguage: language.mask) + + guard identifierToNode[normalizedIdentifier] == nil else { return // skip as item exists already. } + guard let title = (usePageTitle) ? renderNode.metadata.title : renderNode.navigatorTitle() else { + throw Error.missingTitle(description: "\(renderNode.identifier.absoluteString.singleQuoted) has an empty title and so can't have a usable entry in the index.") + } + // Get the identifier path - let identifierPath = NodeURLGenerator().urlForReference(renderNode.identifier, lowercased: true).path + let identifierPath = normalizedIdentifier.path // Store the language inside the availability index. navigatorIndex.availabilityIndex.add(language: language) @@ -688,7 +693,7 @@ extension NavigatorIndex { let fragment = "\(title)#\(index)".addingPercentEncoding(withAllowedCharacters: .urlPathAllowed)! let groupIdentifier = Identifier( - bundleIdentifier: identifier.bundleIdentifier, + bundleIdentifier: normalizedIdentifier.bundleIdentifier, path: identifierPath, fragment: fragment, languageIdentifier: language.mask @@ -737,20 +742,14 @@ extension NavigatorIndex { } } - let normalizedIdentifier = renderNode - .identifier - .normalizedForNavigation - .navigatorIndexIdentifier(forLanguage: language.mask) - // Keep track of the node identifierToNode[normalizedIdentifier] = navigatorNode identifierToChildren[normalizedIdentifier] = children pendingUncuratedReferences.insert(normalizedIdentifier) // Track a multiple curated node - if multiCuratedUnvisited.contains(normalizedIdentifier) { + if multiCuratedUnvisited.remove(normalizedIdentifier) != nil { multiCurated[normalizedIdentifier] = navigatorNode - multiCuratedUnvisited.remove(normalizedIdentifier) } // Bump the nodes counter. @@ -1206,21 +1205,6 @@ fileprivate extension Error { } -extension ResolvedTopicReference { - - /// Returns a normalized instance useful to build a navigator index. - /// - Note: This logic relies on what `PresentationURLGenerator.presentationURLForReference(_: _:)` does in the last line of code. - /// Changing the logic of this normalization method without fixing the other logic would generate a mismatch and break the navigator index process. - var normalizedForNavigation: ResolvedTopicReference { - let normalizedPath = NodeURLGenerator().urlForReference(self).path - return ResolvedTopicReference(bundleIdentifier: bundleIdentifier.lowercased(), - path: normalizedPath.lowercased(), - fragment: fragment, - sourceLanguages: sourceLanguages) - } - -} - extension LMDB.Database { enum NodeError: Error { /// A database error that includes the path of a specific node and the original database error. diff --git a/Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift b/Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift index 5427cf42af..b182619d47 100644 --- a/Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift +++ b/Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift @@ -1446,6 +1446,59 @@ Root XCTAssertEqual("e47cfd13c4af", pathHasher.hash("/mykit/myclass/myfunc")) } } + + func testNormalizedNavigatorIndexIdentifier() throws { + let topicReference = ResolvedTopicReference( + bundleIdentifier: "org.swift.example", + path: "/documentation/path/sub-path", + fragment: nil, + sourceLanguage: .swift + ) + + XCTAssertEqual( + topicReference.normalizedNavigatorIndexIdentifier(forLanguage: 0), + NavigatorIndex.Identifier( + bundleIdentifier: "org.swift.example", + path: "/documentation/path/sub-path", + fragment: nil, + languageIdentifier: 0 + ) + ) + + let topicReferenceWithCapitalization = ResolvedTopicReference( + bundleIdentifier: "org.Swift.Example", + path: "/documentation/Path/subPath", + fragment: nil, + sourceLanguage: .swift + ) + + XCTAssertEqual( + topicReferenceWithCapitalization.normalizedNavigatorIndexIdentifier(forLanguage: 1), + NavigatorIndex.Identifier( + bundleIdentifier: "org.swift.example", + path: "/documentation/path/subpath", + fragment: nil, + languageIdentifier: 1 + ) + ) + + let topicReferenceWithFragment = ResolvedTopicReference( + bundleIdentifier: "org.Swift.Example", + path: "/documentation/Path/subPath", + fragment: "FRAGMENT", + sourceLanguage: .swift + ) + + XCTAssertEqual( + topicReferenceWithFragment.normalizedNavigatorIndexIdentifier(forLanguage: 1), + NavigatorIndex.Identifier( + bundleIdentifier: "org.swift.example", + path: "/documentation/path/subpath", + fragment: "FRAGMENT", + languageIdentifier: 1 + ) + ) + } func generatedNavigatorIndex(for testBundleName: String, bundleIdentifier: String) throws -> NavigatorIndex { let (bundle, context) = try testBundleAndContext(named: testBundleName)