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.
swiftlang · May 25, 2022 · 5a68281 · 5a68281
1 parent 3e496c7
commit 5a68281
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 36 deletions.
diff --git 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<NavigatorIndex.PageType> = [
+        .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
@@ -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
@@ -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)