Skip to content

[Incremental] Obviate the need for a reverse mapping from swiftdeps to swift #728

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

Merged
merged 2 commits into from
Jun 25, 2021
Merged
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
2 changes: 0 additions & 2 deletions Sources/SwiftDriver/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,11 @@ add_library(SwiftDriver
Execution/ParsableOutput.swift
Execution/ProcessProtocol.swift

"IncrementalCompilation/ModuleDependencyGraphParts/InputDependencySourceMap.swift"
"IncrementalCompilation/ModuleDependencyGraphParts/Integrator.swift"
"IncrementalCompilation/ModuleDependencyGraphParts/Node.swift"
"IncrementalCompilation/ModuleDependencyGraphParts/NodeFinder.swift"
"IncrementalCompilation/ModuleDependencyGraphParts/DependencySource.swift"
"IncrementalCompilation/ModuleDependencyGraphParts/Tracer.swift"
"IncrementalCompilation/BidirectionalMap.swift"
"IncrementalCompilation/BuildRecord.swift"
"IncrementalCompilation/BuildRecordInfo.swift"
"IncrementalCompilation/DependencyGraphDotFileWriter.swift"
Expand Down
116 changes: 0 additions & 116 deletions Sources/SwiftDriver/IncrementalCompilation/BidirectionalMap.swift

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ public class DependencyGraphDotFileWriter {
self.info = info
}

func write(_ sfdg: SourceFileDependencyGraph) {
let basename = sfdg.dependencySource.shortDescription
func write(_ sfdg: SourceFileDependencyGraph, for file: TypedVirtualPath) {
let basename = file.file.basename
write(sfdg, basename: basename)
}

Expand All @@ -41,6 +41,7 @@ fileprivate extension DependencyGraphDotFileWriter {
try! info.fileSystem.writeFileContents(path) { stream in
var s = DOTDependencyGraphSerializer<Graph>(
graph,
graphID: basename,
stream,
includeExternals: info.dependencyDotFilesIncludeExternals,
includeAPINotes: info.dependencyDotFilesIncludeAPINotes)
Expand All @@ -58,16 +59,12 @@ fileprivate extension DependencyGraphDotFileWriter {

// MARK: - Making dependency graphs exportable
fileprivate protocol ExportableGraph {
var graphID: String {get}
associatedtype Node: ExportableNode
func forEachExportableNode(_ visit: (Node) -> Void)
func forEachExportableArc(_ visit: (Node, Node) -> Void)
}

extension SourceFileDependencyGraph: ExportableGraph {
fileprivate var graphID: String {
return try! VirtualPath(path: sourceFileName).basename
}
fileprivate func forEachExportableNode<Node: ExportableNode>(_ visit: (Node) -> Void) {
forEachNode { visit($0 as! Node) }
}
Expand Down Expand Up @@ -180,17 +177,20 @@ fileprivate extension DependencyKey.Designator {
fileprivate struct DOTDependencyGraphSerializer<Graph: ExportableGraph> {
private let includeExternals: Bool
private let includeAPINotes: Bool
private let graphID: String
private let graph: Graph
private var nodeIDs = [Graph.Node: Int]()
private var out: WritableByteStream

fileprivate init(
_ graph: Graph,
graphID: String,
_ stream: WritableByteStream,
includeExternals: Bool,
includeAPINotes: Bool
) {
self.graph = graph
self.graphID = graphID
self.out = stream
self.includeExternals = includeExternals
self.includeAPINotes = includeAPINotes
Expand All @@ -205,7 +205,7 @@ fileprivate struct DOTDependencyGraphSerializer<Graph: ExportableGraph> {
}

private func emitPrelude() {
out <<< "digraph " <<< graph.graphID.quoted <<< " {\n"
out <<< "digraph " <<< graphID.quoted <<< " {\n"
}
private mutating func emitLegend() {
for dummy in DependencyKey.Designator.oneOfEachKind {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ public struct DependencyKey: CustomStringConvertible {
case let .externalDepend(externalDependency):
return "import '\(externalDependency.shortDescription)'"
case let .sourceFileProvide(name: name):
return "source file \((try? VirtualPath(path: name).basename) ?? name)"
return "source file from \((try? VirtualPath(path: name).basename) ?? name)"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ public struct InvalidatedSet<ClosureLevel, Element: Hashable>: Sequence {
public func compactMap<R>(_ transform: (Element) -> R? ) -> InvalidatedArray<ClosureLevel, R> {
InvalidatedArray(contents.compactMap(transform))
}
public func filter(_ isIncluded: (Element) -> Bool) -> InvalidatedArray<ClosureLevel, Element> {
InvalidatedArray(contents.filter(isIncluded))
}
}

extension InvalidatedSet where Element: Comparable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ extension IncrementalCompilationState.FirstWaveComputer {
return nil

case .upToDate:
reporter?.report("Scheduing changed input", input)
reporter?.report("Scheduling changed input", input)
case .newlyAdded:
reporter?.report("Scheduling new", input)
case .needsCascadingBuild:
Expand Down Expand Up @@ -269,12 +269,13 @@ extension IncrementalCompilationState.FirstWaveComputer {

return inputsToBeCertainlyRecompiled.reduce(into: Set()) {
speculativelyRecompiledInputs, certainlyRecompiledInput in
let speculativeDependents = moduleDependencyGraph.collectInputsInvalidatedBy(input: certainlyRecompiledInput)
let speculativeDependents = moduleDependencyGraph.collectInputsInvalidatedBy(changedInput: certainlyRecompiledInput)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the names cleanup!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My pleasure!

for speculativeDependent in speculativeDependents
where !inputsToBeCertainlyRecompiled.contains(speculativeDependent) {
if speculativelyRecompiledInputs.insert(speculativeDependent).inserted {
reporter?.report(
"Immediately scheduling dependent on \(certainlyRecompiledInput.file.basename)", speculativeDependent)
"Immediately scheduling dependent on \(certainlyRecompiledInput.file.basename)",
speculativeDependent)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -542,22 +542,46 @@ extension OutputFileMap {
}

// MARK: SourceFiles

/// Handy information about the source files in the current invocation
@_spi(Testing) public struct SourceFiles {
/// The current files in same order as the invocation
let currentInOrder: [TypedVirtualPath]
private let previous: Set<VirtualPath>

/// The set of current files (actually the handles)
let currentSet: Set<VirtualPath.Handle>

/// Handles of the input files in the previous invocation
private let previousSet: Set<VirtualPath.Handle>

/// The files that were in the previous but not in the current invocation
let disappeared: [VirtualPath]

init(inputFiles: [TypedVirtualPath], buildRecord: BuildRecord?) {
self.currentInOrder = inputFiles.filter {$0.type == .swift}
let currentSet = Set(currentInOrder.map {$0.file} )
self.previous = buildRecord.map {
Set($0.inputInfos.keys)
} ?? Set()

self.disappeared = previous.filter {!currentSet.contains($0)}
self.currentSet = currentInOrder.reduce(into: Set()) {
currentSet, currentFile in
currentSet.insert(currentFile.fileHandle)
}
guard let buildRecord = buildRecord else {
self.previousSet = Set()
self.disappeared = []
return
}
var previous = Set<VirtualPath.Handle>()
var disappeared = [VirtualPath]()
for prevPath in buildRecord.inputInfos.keys {
let handle = prevPath.intern()
previous.insert(handle)
if !currentSet.contains(handle) {
disappeared.append(prevPath)
}
}
self.previousSet = previous
self.disappeared = disappeared.sorted {$0.name < $1.name}
}

func isANewInput(_ file: VirtualPath) -> Bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this traffic in handles?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into it--good thought!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that changing SourceFile to only traffic in handles would be a good idea, but it leaks out into other code and it might be better to keep this PR as narrow as reasonable. I'll address this idea in a future PR.

!previous.contains(file)
!previousSet.contains(file.intern())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ extension IncrementalCompilationState.IncrementalDependencyAndInputSetup {
let nodesDirectlyInvalidatedByExternals =
graph.collectNodesInvalidatedByChangedOrAddedExternals()
// Wait till the last minute to do the transitive closure as an optimization.
guard let inputsInvalidatedByExternals = graph.collectInputsUsingInvalidated(
guard let inputsInvalidatedByExternals = graph.collectInputsInBuildUsingInvalidated(
nodes: nodesDirectlyInvalidatedByExternals)
else {
return nil
Expand Down
Loading