diff --git a/Sources/Frontend/Scan.swift b/Sources/Frontend/Scan.swift index 755e35648..376105d07 100644 --- a/Sources/Frontend/Scan.swift +++ b/Sources/Frontend/Scan.swift @@ -70,7 +70,8 @@ final class Scan { let indexLogger = logger.contextualized(with: "index") let plan = try driver.plan(logger: indexLogger) - let pipeline = IndexPipeline(plan: plan, graph: graph, logger: indexLogger) + let syncSourceGraph = SynchronizedSourceGraph(graph: graph) + let pipeline = IndexPipeline(plan: plan, graph: syncSourceGraph, logger: indexLogger) try pipeline.perform() logger.endInterval(indexInterval) } diff --git a/Sources/Indexer/IndexPipeline.swift b/Sources/Indexer/IndexPipeline.swift index caa60b166..06fcec182 100644 --- a/Sources/Indexer/IndexPipeline.swift +++ b/Sources/Indexer/IndexPipeline.swift @@ -4,10 +4,10 @@ import SourceGraph public struct IndexPipeline { private let plan: IndexPlan - private let graph: SourceGraph + private let graph: SynchronizedSourceGraph private let logger: ContextualLogger - public init(plan: IndexPlan, graph: SourceGraph, logger: ContextualLogger) { + public init(plan: IndexPlan, graph: SynchronizedSourceGraph, logger: ContextualLogger) { self.plan = plan self.graph = graph self.logger = logger diff --git a/Sources/Indexer/InfoPlistIndexer.swift b/Sources/Indexer/InfoPlistIndexer.swift index fd80244ad..b00dc4870 100644 --- a/Sources/Indexer/InfoPlistIndexer.swift +++ b/Sources/Indexer/InfoPlistIndexer.swift @@ -4,10 +4,10 @@ import SystemPackage final class InfoPlistIndexer: Indexer { private let infoPlistFiles: Set - private let graph: SourceGraph + private let graph: SynchronizedSourceGraph private let logger: ContextualLogger - required init(infoPlistFiles: Set, graph: SourceGraph, logger: Logger = .init(), configuration: Configuration = .shared) { + required init(infoPlistFiles: Set, graph: SynchronizedSourceGraph, logger: Logger = .init(), configuration: Configuration = .shared) { self.infoPlistFiles = infoPlistFiles self.graph = graph self.logger = logger.contextualized(with: "index:infoplist") diff --git a/Sources/Indexer/SwiftIndexer.swift b/Sources/Indexer/SwiftIndexer.swift index 3231bc6ec..3b065c5c4 100644 --- a/Sources/Indexer/SwiftIndexer.swift +++ b/Sources/Indexer/SwiftIndexer.swift @@ -12,13 +12,13 @@ public struct IndexUnit { final class SwiftIndexer: Indexer { private let sourceFiles: [SourceFile: [IndexUnit]] - private let graph: SourceGraph + private let graph: SynchronizedSourceGraph private let logger: ContextualLogger private let configuration: Configuration required init( sourceFiles: [SourceFile: [IndexUnit]], - graph: SourceGraph, + graph: SynchronizedSourceGraph, logger: ContextualLogger, configuration: Configuration = .shared ) { @@ -82,7 +82,7 @@ final class SwiftIndexer: Indexer { let sourceFile: SourceFile private let units: [IndexUnit] - private let graph: SourceGraph + private let graph: SynchronizedSourceGraph private let logger: ContextualLogger private let configuration: Configuration private var retainAllDeclarations: Bool @@ -91,7 +91,7 @@ final class SwiftIndexer: Indexer { sourceFile: SourceFile, units: [IndexUnit], retainAllDeclarations: Bool, - graph: SourceGraph, + graph: SynchronizedSourceGraph, logger: ContextualLogger, configuration: Configuration ) { @@ -202,11 +202,11 @@ final class SwiftIndexer: Indexer { } graph.withLock { - graph.addUnsafe(references) - graph.addUnsafe(newDeclarations) + graph.addWithoutLock(references) + graph.addWithoutLock(newDeclarations) if retainAllDeclarations { - graph.markRetainedUnsafe(newDeclarations) + graph.markRetainedWithoutLock(newDeclarations) } } @@ -252,10 +252,10 @@ final class SwiftIndexer: Indexer { private func establishDeclarationHierarchy() { graph.withLock { for (parent, decls) in childDeclsByParentUsr { - guard let parentDecl = graph.explicitDeclaration(withUsr: parent) else { + guard let parentDecl = graph.explicitDeclarationWithoutLock(withUsr: parent) else { if varParameterUsrs.contains(parent) { // These declarations are children of a parameter and are redundant. - decls.forEach { graph.removeUnsafe($0) } + decls.forEach { graph.removeWithoutLock($0) } } continue @@ -273,7 +273,7 @@ final class SwiftIndexer: Indexer { private func associateLatentReferences() { for (usr, refs) in referencesByUsr { graph.withLock { - if let decl = graph.explicitDeclaration(withUsr: usr) { + if let decl = graph.explicitDeclarationWithoutLock(withUsr: usr) { for ref in refs { associateUnsafe(ref, with: decl) } @@ -454,14 +454,14 @@ final class SwiftIndexer: Indexer { for param in params { let paramDecl = param.makeDeclaration(withParent: functionDecl) functionDecl.unusedParameters.insert(paramDecl) - graph.addUnsafe(paramDecl) + graph.addWithoutLock(paramDecl) if retainAllDeclarations { - graph.markRetainedUnsafe(paramDecl) + graph.markRetainedWithoutLock(paramDecl) } if (functionDecl.isObjcAccessible && configuration.retainObjcAccessible) || ignoredParamNames.contains(param.name) { - graph.markRetainedUnsafe(paramDecl) + graph.markRetainedWithoutLock(paramDecl) } } } diff --git a/Sources/Indexer/XCDataModelIndexer.swift b/Sources/Indexer/XCDataModelIndexer.swift index f5311f03c..12e25ecbc 100644 --- a/Sources/Indexer/XCDataModelIndexer.swift +++ b/Sources/Indexer/XCDataModelIndexer.swift @@ -4,10 +4,10 @@ import SystemPackage final class XCDataModelIndexer: Indexer { private let files: Set - private let graph: SourceGraph + private let graph: SynchronizedSourceGraph private let logger: ContextualLogger - required init(files: Set, graph: SourceGraph, logger: Logger = .init(), configuration: Configuration = .shared) { + required init(files: Set, graph: SynchronizedSourceGraph, logger: Logger = .init(), configuration: Configuration = .shared) { self.files = files self.graph = graph self.logger = logger.contextualized(with: "index:xcdatamodel") diff --git a/Sources/Indexer/XCMappingModelIndexer.swift b/Sources/Indexer/XCMappingModelIndexer.swift index 2c98181c2..363caacb3 100644 --- a/Sources/Indexer/XCMappingModelIndexer.swift +++ b/Sources/Indexer/XCMappingModelIndexer.swift @@ -4,10 +4,10 @@ import SystemPackage final class XCMappingModelIndexer: Indexer { private let files: Set - private let graph: SourceGraph + private let graph: SynchronizedSourceGraph private let logger: ContextualLogger - required init(files: Set, graph: SourceGraph, logger: Logger = .init(), configuration: Configuration = .shared) { + required init(files: Set, graph: SynchronizedSourceGraph, logger: Logger = .init(), configuration: Configuration = .shared) { self.files = files self.graph = graph self.logger = logger.contextualized(with: "index:xcmappingmodel") diff --git a/Sources/Indexer/XibIndexer.swift b/Sources/Indexer/XibIndexer.swift index e6507e555..f2bc29a64 100644 --- a/Sources/Indexer/XibIndexer.swift +++ b/Sources/Indexer/XibIndexer.swift @@ -4,10 +4,10 @@ import SystemPackage final class XibIndexer: Indexer { private let xibFiles: Set - private let graph: SourceGraph + private let graph: SynchronizedSourceGraph private let logger: ContextualLogger - required init(xibFiles: Set, graph: SourceGraph, logger: Logger = .init(), configuration: Configuration = .shared) { + required init(xibFiles: Set, graph: SynchronizedSourceGraph, logger: Logger = .init(), configuration: Configuration = .shared) { self.xibFiles = xibFiles self.graph = graph self.logger = logger.contextualized(with: "index:xib") diff --git a/Sources/SourceGraph/SourceGraph.swift b/Sources/SourceGraph/SourceGraph.swift index 9c5b1109f..668bf8a10 100644 --- a/Sources/SourceGraph/SourceGraph.swift +++ b/Sources/SourceGraph/SourceGraph.swift @@ -27,7 +27,6 @@ public final class SourceGraph { private var allExplicitDeclarationsByUsr: [String: Declaration] = [:] private var moduleToExportingModules: [String: Set] = [:] - private let lock = UnfairLock() private let configuration: Configuration init(configuration: Configuration = .shared) { @@ -68,62 +67,42 @@ public final class SourceGraph { } func markRedundantProtocol(_ declaration: Declaration, references: Set, inherited: Set) { - withLock { - redundantProtocols[declaration] = (references, inherited) - } + redundantProtocols[declaration] = (references, inherited) } func markRedundantPublicAccessibility(_ declaration: Declaration, modules: Set) { - withLock { - redundantPublicAccessibility[declaration] = modules - } + redundantPublicAccessibility[declaration] = modules } func unmarkRedundantPublicAccessibility(_ declaration: Declaration) { - withLock { - _ = redundantPublicAccessibility.removeValue(forKey: declaration) - } + _ = redundantPublicAccessibility.removeValue(forKey: declaration) } func markIgnored(_ declaration: Declaration) { - withLock { - _ = ignoredDeclarations.insert(declaration) - } + _ = ignoredDeclarations.insert(declaration) } public func markRetained(_ declaration: Declaration) { - withLock { - markRetainedUnsafe(declaration) - } - } - - public func markRetainedUnsafe(_ declaration: Declaration) { _ = retainedDeclarations.insert(declaration) } - public func markRetainedUnsafe(_ declarations: Set) { + public func markRetained(_ declarations: Set) { retainedDeclarations.formUnion(declarations) } func markAssignOnlyProperty(_ declaration: Declaration) { - withLock { - _ = assignOnlyProperties.insert(declaration) - } + _ = assignOnlyProperties.insert(declaration) } func markMainAttributed(_ declaration: Declaration) { - withLock { - _ = mainAttributedDeclarations.insert(declaration) - } + _ = mainAttributedDeclarations.insert(declaration) } public func isRetained(_ declaration: Declaration) -> Bool { - withLock { - retainedDeclarations.contains(declaration) - } + retainedDeclarations.contains(declaration) } - public func addUnsafe(_ declaration: Declaration) { + public func add(_ declaration: Declaration) { allDeclarations.insert(declaration) allDeclarationsByKind[declaration.kind, default: []].insert(declaration) @@ -132,7 +111,7 @@ public final class SourceGraph { } } - public func addUnsafe(_ declarations: Set) { + public func add(_ declarations: Set) { allDeclarations.formUnion(declarations) for declaration in declarations { @@ -144,13 +123,7 @@ public final class SourceGraph { } } - func remove(_ declaration: Declaration) { - withLock { - removeUnsafe(declaration) - } - } - - public func removeUnsafe(_ declaration: Declaration) { + public func remove(_ declaration: Declaration) { declaration.parent?.declarations.remove(declaration) allDeclarations.remove(declaration) allDeclarationsByKind[declaration.kind]?.remove(declaration) @@ -160,59 +133,47 @@ public final class SourceGraph { declaration.usrs.forEach { allExplicitDeclarationsByUsr.removeValue(forKey: $0) } } - public func addUnsafe(_ reference: Reference) { + public func add(_ reference: Reference) { _ = allReferences.insert(reference) allReferencesByUsr[reference.usr, default: []].insert(reference) } - public func addUnsafe(_ references: Set) { + public func add(_ references: Set) { allReferences.formUnion(references) references.forEach { allReferencesByUsr[$0.usr, default: []].insert($0) } } public func add(_ reference: Reference, from declaration: Declaration) { - withLock { - if reference.isRelated { - _ = declaration.related.insert(reference) - } else { - _ = declaration.references.insert(reference) - } - - addUnsafe(reference) + if reference.isRelated { + _ = declaration.related.insert(reference) + } else { + _ = declaration.references.insert(reference) } + + add(reference) } func remove(_ reference: Reference) { - withLock { - _ = allReferences.remove(reference) - allReferences.subtract(reference.descendentReferences) - allReferencesByUsr[reference.usr]?.remove(reference) - } + _ = allReferences.remove(reference) + allReferences.subtract(reference.descendentReferences) + allReferencesByUsr[reference.usr]?.remove(reference) if let parent = reference.parent { - withLock { - parent.references.remove(reference) - parent.related.remove(reference) - } + parent.references.remove(reference) + parent.related.remove(reference) } } public func add(_ assetReference: AssetReference) { - withLock { - _ = assetReferences.insert(assetReference) - } + _ = assetReferences.insert(assetReference) } func markUsed(_ declaration: Declaration) { - withLock { - _ = usedDeclarations.insert(declaration) - } + _ = usedDeclarations.insert(declaration) } func isUsed(_ declaration: Declaration) -> Bool { - withLock { - usedDeclarations.contains(declaration) - } + usedDeclarations.contains(declaration) } func isExternal(_ reference: Reference) -> Bool { @@ -220,30 +181,18 @@ public final class SourceGraph { } public func addIndexedSourceFile(_ file: SourceFile) { - withLock { - indexedSourceFiles.append(file) - } + indexedSourceFiles.append(file) } public func addIndexedModules(_ modules: Set) { - withLock { - indexedModules.formUnion(modules) - } + indexedModules.formUnion(modules) } public func addExportedModule(_ module: String, exportedBy exportingModules: Set) { - withLock { - moduleToExportingModules[module, default: []].formUnion(exportingModules) - } + moduleToExportingModules[module, default: []].formUnion(exportingModules) } - func isModule(_ module: String, exportedBy exportingModule: String) -> Bool { - withLock { - isModuleUnsafe(module, exportedBy: exportingModule) - } - } - - private func isModuleUnsafe(_ module: String, exportedBy exportingModule: String) -> Bool { + public func isModule(_ module: String, exportedBy exportingModule: String) -> Bool { let exportingModules = moduleToExportingModules[module, default: []] if exportingModules.contains(exportingModule) { @@ -253,25 +202,21 @@ public final class SourceGraph { // Recursively check if the module is exported transitively. return exportingModules.contains { nestedExportingModule in - isModuleUnsafe(nestedExportingModule, exportedBy: exportingModule) && - isModuleUnsafe(module, exportedBy: nestedExportingModule) + isModule(nestedExportingModule, exportedBy: exportingModule) && + isModule(module, exportedBy: nestedExportingModule) } } func markUnusedModuleImport(_ statement: ImportStatement) { - withLock { - let location = statement.location.relativeTo(.current) - let usr = "import-\(statement.module)-\(location)" - let decl = Declaration(kind: .module, usrs: [usr], location: statement.location) - decl.name = statement.module - unusedModuleImports.insert(decl) - } + let location = statement.location.relativeTo(.current) + let usr = "import-\(statement.module)-\(location)" + let decl = Declaration(kind: .module, usrs: [usr], location: statement.location) + decl.name = statement.module + unusedModuleImports.insert(decl) } func markExtension(_ extensionDecl: Declaration, extending extendedDecl: Declaration) { - withLock { - _ = extensions[extendedDecl, default: []].insert(extensionDecl) - } + _ = extensions[extendedDecl, default: []].insert(extensionDecl) } func inheritedTypeReferences(of decl: Declaration, seenDeclarations: Set = []) -> Set { @@ -310,10 +255,6 @@ public final class SourceGraph { return immediate.union(allSubclasses) } - public func withLock(_ block: () -> T) -> T { - lock.perform(block) - } - func extendedDeclarationReference(forExtension extensionDeclaration: Declaration) throws -> Reference? { guard let extendedKind = extensionDeclaration.kind.extendedKind else { throw PeripheryError.sourceGraphIntegrityError(message: "Unknown extended reference kind for extension '\(extensionDeclaration.kind.rawValue)'") diff --git a/Sources/SourceGraph/SynchronizedSourceGraph.swift b/Sources/SourceGraph/SynchronizedSourceGraph.swift new file mode 100644 index 000000000..1a61a28c4 --- /dev/null +++ b/Sources/SourceGraph/SynchronizedSourceGraph.swift @@ -0,0 +1,80 @@ +import Foundation +import Shared + +/// A wrapper around for SourceGraph with synchronization for use during indexing. +public final class SynchronizedSourceGraph { + private let graph: SourceGraph + private let lock = UnfairLock() + + public init(graph: SourceGraph) { + self.graph = graph + } + + public func withLock(_ block: () -> T) -> T { + lock.perform(block) + } + + public func indexingComplete() { + graph.indexingComplete() + } + + public func markRetained(_ declaration: Declaration) { + withLock { + graph.markRetained(declaration) + } + } + + public func addIndexedSourceFile(_ file: SourceFile) { + withLock { + graph.addIndexedSourceFile(file) + } + } + + public func addIndexedModules(_ modules: Set) { + withLock { + graph.addIndexedModules(modules) + } + } + + public func addExportedModule(_ module: String, exportedBy exportingModules: Set) { + withLock { + graph.addExportedModule(module, exportedBy: exportingModules) + } + } + + public func add(_ assetReference: AssetReference) { + withLock { + graph.add(assetReference) + } + } + + // MARK: - Without Lock + + public func removeWithoutLock(_ declaration: Declaration) { + graph.remove(declaration) + } + + public func explicitDeclarationWithoutLock(withUsr usr: String) -> Declaration? { + graph.explicitDeclaration(withUsr: usr) + } + + public func markRetainedWithoutLock(_ declaration: Declaration) { + graph.markRetained(declaration) + } + + public func markRetainedWithoutLock(_ declarations: Set) { + graph.markRetained(declarations) + } + + public func addWithoutLock(_ declaration: Declaration) { + graph.add(declaration) + } + + public func addWithoutLock(_ references: Set) { + graph.add(references) + } + + public func addWithoutLock(_ declarations: Set) { + graph.add(declarations) + } +} diff --git a/Tests/Shared/SourceGraphTestCase.swift b/Tests/Shared/SourceGraphTestCase.swift index 237c96619..89376d669 100644 --- a/Tests/Shared/SourceGraphTestCase.swift +++ b/Tests/Shared/SourceGraphTestCase.swift @@ -54,7 +54,7 @@ open class SourceGraphTestCase: XCTestCase { graph = SourceGraph() let pipeline = IndexPipeline( plan: newPlan, - graph: graph, + graph: SynchronizedSourceGraph(graph: graph), logger: Logger().contextualized(with: "index") ) try! pipeline.perform()