Skip to content

[Incremental Builds]optimize performance for ModuleDependencyGraph by reducing the number of external dependencies from N*M to N+M #1881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1350,6 +1350,30 @@ extension ModuleDependencyGraph {
}
}
}
if graph.nodeFinder.enableExperimatalExternalDependenciesHashOptimization {
var hasWrittenAllExternalDependentNodes = false
for key in graph.nodeFinder.externalDependencies {
serializer.stream.writeRecord(serializer.abbreviations[.dependsOnNode]!) {
$0.append(RecordID.dependsOnNode)
write(key: key, to: &$0)
}

for use in graph.nodeFinder.externalDependentNodes {
guard let useID = serializer.nodeIDs[use] else {
fatalError("Node ID was not registered! \(use)")
}

serializer.stream.writeRecord(serializer.abbreviations[.useIDNode]!) {
$0.append(RecordID.useIDNode)
$0.append(UInt32(useID))
}
if hasWrittenAllExternalDependentNodes {
break
}
}
hasWrittenAllExternalDependentNodes = true
}
}
for fingerprintedExternalDependency in graph.fingerprintedExternalDependencies {
serializer.stream.writeRecord(serializer.abbreviations[.externalDepNode]!) {
$0.append(RecordID.externalDepNode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,16 @@ extension ModuleDependencyGraph {
/// that would need to change if/when we can recompile a smaller unit than a
/// source file.)

/// Tracks def-use relationships by DependencyKey.
/// Tracks def-use relationships by DependencyKey except for external expendencies (Reduce the number of external dependencies from N*M to N+M).
@_spi(Testing) public private(set) var usesByDef = Multidictionary<DependencyKey, Node>()

/// Tracks external DependencyKey.
@_spi(Testing) public private(set) var externalDependencies = Set<DependencyKey>()

/// Tracks nodes which dependent external DependencyKey.
@_spi(Testing) public private(set) var externalDependentNodes = Set<Node>()

public let enableExperimatalExternalDependenciesHashOptimization = true
}
}
// MARK: - finding
Expand Down Expand Up @@ -78,6 +86,11 @@ extension ModuleDependencyGraph.NodeFinder {
/// definition node.
func uses(of def: Graph.Node) -> Set<Graph.Node> {
var uses = usesByDef[def.key, default: Set()]
if enableExperimatalExternalDependenciesHashOptimization {
if externalDependencies.contains(def.key) {
uses = externalDependentNodes
}
}
if let impl = findCorrespondingImplementation(of: def) {
uses.insert(impl)
}
Expand All @@ -99,7 +112,12 @@ extension ModuleDependencyGraph.NodeFinder {
}

func defsUsing(_ n: Graph.Node) -> Set<DependencyKey> {
usesByDef.keysContainingValue(n)
if enableExperimatalExternalDependenciesHashOptimization {
if externalDependentNodes.contains(n) {
return usesByDef.keysContainingValue(n).union(externalDependencies)
}
}
return usesByDef.keysContainingValue(n)
}
}

Expand All @@ -122,6 +140,13 @@ extension ModuleDependencyGraph.NodeFinder {
/// record def-use, return if is new use
mutating func record(def: DependencyKey, use: Graph.Node) -> Bool {
assert(verifyOKTODependUponSomeKey(use))
if enableExperimatalExternalDependenciesHashOptimization {
if let _ = def.designator.externalDependency {
let inserted1 = externalDependencies.insert(def).inserted
let inserted2 = externalDependentNodes.insert(use).inserted
return inserted1 || inserted2
}
}
return usesByDef.insertValue(use, forKey: def)
}
}
Expand All @@ -135,6 +160,12 @@ extension ModuleDependencyGraph.NodeFinder {
}

private mutating func removeUsings(of nodeToNotUse: Graph.Node) {
if enableExperimatalExternalDependenciesHashOptimization {
if externalDependentNodes.contains(nodeToNotUse) {
externalDependentNodes.remove(nodeToNotUse)
return
}
}
usesByDef.removeOccurrences(of: nodeToNotUse)
assert(defsUsing(nodeToNotUse).isEmpty)
}
Expand Down Expand Up @@ -164,6 +195,12 @@ extension ModuleDependencyGraph.NodeFinder {
definitionLocation: .known(newDependencySource))
assert(original.definitionLocation == .unknown,
"Would have to search every use in usesByDef if original could be a use.")

if enableExperimatalExternalDependenciesHashOptimization {
if externalDependentNodes.remove(original) != nil {
externalDependentNodes.insert(replacement)
}
}
if usesByDef.removeValue(original, forKey: original.key) != nil {
usesByDef.insertValue(replacement, forKey: original.key)
}
Expand Down