Skip to content

Revert #664 #681

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 4 commits into from
May 27, 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
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ extension IncrementalCompilationState.IncrementalDependencyAndInputSetup {
else {
return buildInitialGraphFromSwiftDepsAndCollectInputsInvalidatedByChangedExternals()
}
guard graph.populateInputDependencySourceMap(for: .inputsAddedSincePriors) else {
guard graph.populateInputDependencySourceMap() else {
return nil
}
graph.dotFileWriter?.write(graph)
Expand Down Expand Up @@ -256,8 +256,7 @@ extension IncrementalCompilationState.IncrementalDependencyAndInputSetup {
{
let graph = ModuleDependencyGraph(self, .buildingWithoutAPrior)
assert(outputFileMap.onlySourceFilesHaveSwiftDeps())

guard graph.populateInputDependencySourceMap(for: .buildingFromSwiftDeps) else {
guard graph.populateInputDependencySourceMap() else {
return nil
}

Expand Down
132 changes: 68 additions & 64 deletions Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,19 @@ import SwiftOptions
@_spi(Testing) public var nodeFinder = NodeFinder()

/// Maps input files (e.g. .swift) to and from the DependencySource object.
@_spi(Testing) public private(set) var inputDependencySourceMap = InputDependencySourceMap()
///
// FIXME: The map between swiftdeps and swift files is absolutely *not*
// a bijection. In particular, more than one swiftdeps file can be encountered
// in the course of deserializing priors *and* reading the output file map
// *and* re-reading swiftdeps files after frontends complete
// that correspond to the same swift file. These cause two problems:
// - overwrites in this data structure that lose data and
// - cache misses in `getInput(for:)` that cause the incremental build to
// turn over when e.g. entries in the output file map change. This should be
// replaced by a multi-map from swift files to dependency sources,
// and a regular map from dependency sources to swift files -
// since that direction really is one-to-one.
@_spi(Testing) public private(set) var inputDependencySourceMap = BidirectionalMap<TypedVirtualPath, DependencySource>()

// The set of paths to external dependencies known to be in the graph
public internal(set) var fingerprintedExternalDependencies = Set<FingerprintedExternalDependency>()
Expand Down Expand Up @@ -54,20 +66,22 @@ import SwiftOptions
self.creationPhase = phase
}

@_spi(Testing) public func source(requiredFor input: TypedVirtualPath,
function: String = #function,
file: String = #file,
line: Int = #line) -> DependencySource {
guard let source = inputDependencySourceMap.sourceIfKnown(for: input)
else {
private func addMapEntry(_ input: TypedVirtualPath, _ dependencySource: DependencySource) {
assert(input.type == .swift && dependencySource.typedFile.type == .swiftDeps)
inputDependencySourceMap[input] = dependencySource
}

@_spi(Testing) public func getSource(for input: TypedVirtualPath,
function: String = #function,
file: String = #file,
line: Int = #line) -> DependencySource {
guard let source = inputDependencySourceMap[input] else {
fatalError("\(input.file.basename) not found in inputDependencySourceMap, \(file):\(line) in \(function)")
}
return source
}

@_spi(Testing) public func input(neededFor source: DependencySource) -> TypedVirtualPath? {
guard let input = inputDependencySourceMap.input(ifKnownFor: source)
else {
@_spi(Testing) public func getInput(for source: DependencySource) -> TypedVirtualPath? {
guard let input = inputDependencySourceMap[source] else {
info.diagnosticEngine.emit(warning: "Failed to find source file for '\(source.file.basename)', recovering with a full rebuild. Next build will be incremental.")
return nil
}
Expand Down Expand Up @@ -141,7 +155,7 @@ extension ModuleDependencyGraph {
return TransitivelyInvalidatedInputSet()
}
return collectInputsRequiringCompilationAfterProcessing(
dependencySource: source(requiredFor: input))
dependencySource: getSource(for: input))
}
}

Expand All @@ -161,17 +175,17 @@ extension ModuleDependencyGraph {
/// speculatively scheduled in the first wave.
func collectInputsInvalidatedBy(input: TypedVirtualPath
) -> TransitivelyInvalidatedInputArray {
let changedSource = source(requiredFor: input)
let changedSource = getSource(for: input)
let allDependencySourcesToRecompile =
collectSwiftDepsUsing(dependencySource: changedSource)

return allDependencySourcesToRecompile.compactMap {
depedencySource in
guard depedencySource != changedSource else {return nil}
let dependentInput = inputDependencySourceMap.input(ifKnownFor: depedencySource)
let dependentSource = inputDependencySourceMap[depedencySource]
info.reporter?.report(
"Found dependent of \(input.file.basename):", dependentInput)
return dependentInput
"Found dependent of \(input.file.basename):", dependentSource)
return dependentSource
}
}

Expand All @@ -188,7 +202,7 @@ extension ModuleDependencyGraph {
/// Does the graph contain any dependency nodes for a given source-code file?
func containsNodes(forSourceFile file: TypedVirtualPath) -> Bool {
precondition(file.type == .swift)
guard let source = inputDependencySourceMap.sourceIfKnown(for: file) else {
guard let source = inputDependencySourceMap[file] else {
return false
}
return containsNodes(forDependencySource: source)
Expand All @@ -198,46 +212,17 @@ extension ModuleDependencyGraph {
return nodeFinder.findNodes(for: source).map {!$0.isEmpty}
?? false
}

/// Returns: false on error
func populateInputDependencySourceMap(
for purpose: InputDependencySourceMap.AdditionPurpose
) -> Bool {

/// Return true on success
func populateInputDependencySourceMap() -> Bool {
let ofm = info.outputFileMap
let diags = info.diagnosticEngine
var allFound = true
for input in info.inputFiles {
if let source = ofm.dependencySource(for: input, diagnosticEngine: diags) {
inputDependencySourceMap.addEntry(input, source, for: purpose)
} else {
// Don't break in order to report all failures.
allFound = false
}
}
return allFound
}
}
extension OutputFileMap {
fileprivate func dependencySource(
for sourceFile: TypedVirtualPath,
diagnosticEngine: DiagnosticsEngine
) -> DependencySource? {
assert(sourceFile.type == FileType.swift)
guard let swiftDepsPath = existingOutput(inputFile: sourceFile.fileHandle,
outputType: .swiftDeps)
else {
// The legacy driver fails silently here.
diagnosticEngine.emit(
.remarkDisabled("\(sourceFile.file.basename) has no swiftDeps file")
)
return nil
let de = info.diagnosticEngine
return info.inputFiles.reduce(true) { okSoFar, input in
ofm.getDependencySource(for: input, diagnosticEngine: de)
.map {source in addMapEntry(input, source); return okSoFar } ?? false
}
assert(VirtualPath.lookup(swiftDepsPath).extension == FileType.swiftDeps.rawValue)
let typedSwiftDepsFile = TypedVirtualPath(file: swiftDepsPath, type: .swiftDeps)
return DependencySource(typedSwiftDepsFile)
}
}

// MARK: - Scheduling the 2nd wave
extension ModuleDependencyGraph {
/// After `source` has been compiled, figure out what other source files need compiling.
Expand All @@ -247,7 +232,7 @@ extension ModuleDependencyGraph {
func collectInputsRequiringCompilation(byCompiling input: TypedVirtualPath
) -> TransitivelyInvalidatedInputSet? {
precondition(input.type == .swift)
let dependencySource = source(requiredFor: input)
let dependencySource = getSource(for: input)
return collectInputsRequiringCompilationAfterProcessing(
dependencySource: dependencySource)
}
Expand Down Expand Up @@ -342,7 +327,7 @@ extension ModuleDependencyGraph {
) -> TransitivelyInvalidatedInputSet? {
var invalidatedInputs = TransitivelyInvalidatedInputSet()
for invalidatedSwiftDeps in collectSwiftDepsUsingInvalidated(nodes: directlyInvalidatedNodes) {
guard let invalidatedInput = input(neededFor: invalidatedSwiftDeps) else {
guard let invalidatedInput = getInput(for: invalidatedSwiftDeps) else {
return nil
}
invalidatedInputs.insert(invalidatedInput)
Expand Down Expand Up @@ -466,6 +451,27 @@ extension ModuleDependencyGraph {
}
}

extension OutputFileMap {
fileprivate func getDependencySource(
for sourceFile: TypedVirtualPath,
diagnosticEngine: DiagnosticsEngine
) -> DependencySource? {
assert(sourceFile.type == FileType.swift)
guard let swiftDepsPath = existingOutput(inputFile: sourceFile.fileHandle,
outputType: .swiftDeps)
else {
// The legacy driver fails silently here.
diagnosticEngine.emit(
.remarkDisabled("\(sourceFile.file.basename) has no swiftDeps file")
)
return nil
}
assert(VirtualPath.lookup(swiftDepsPath).extension == FileType.swiftDeps.rawValue)
let typedSwiftDepsFile = TypedVirtualPath(file: swiftDepsPath, type: .swiftDeps)
return DependencySource(typedSwiftDepsFile)
}
}

// MARK: - tracking traced nodes
extension ModuleDependencyGraph {

Expand Down Expand Up @@ -590,11 +596,10 @@ extension ModuleDependencyGraph {
.record(def: dependencyKey, use: self.allNodes[useID])
assert(isNewUse, "Duplicate use def-use arc in graph?")
}
for (input, dependencySource) in inputDependencySourceMap {
graph.inputDependencySourceMap.addEntry(input,
dependencySource,
for: .readingPriors)
for (input, source) in inputDependencySourceMap {
graph.addMapEntry(input, source)
}

return self.graph
}

Expand Down Expand Up @@ -890,7 +895,7 @@ extension ModuleDependencyGraph {
}
}

graph.inputDependencySourceMap.enumerateToSerializePriors { input, dependencySource in
for (input, dependencySource) in graph.inputDependencySourceMap {
self.addIdentifier(input.file.name)
self.addIdentifier(dependencySource.file.name)
}
Expand Down Expand Up @@ -1035,8 +1040,7 @@ extension ModuleDependencyGraph {
}
}
}
graph.inputDependencySourceMap.enumerateToSerializePriors {
input, dependencySource in
for (input, dependencySource) in graph.inputDependencySourceMap {
serializer.stream.writeRecord(serializer.abbreviations[.mapNode]!) {
$0.append(RecordID.mapNode)
$0.append(serializer.lookupIdentifierCode(for: input.file.name))
Expand Down Expand Up @@ -1154,7 +1158,7 @@ extension Set where Element == ModuleDependencyGraph.Node {
}
}

extension InputDependencySourceMap {
extension BidirectionalMap where T1 == TypedVirtualPath, T2 == DependencySource {
fileprivate func matches(_ other: Self) -> Bool {
self == other
}
Expand All @@ -1172,6 +1176,6 @@ extension ModuleDependencyGraph {
_ mockInput: TypedVirtualPath,
_ mockDependencySource: DependencySource
) {
inputDependencySourceMap.addEntry(mockInput, mockDependencySource, for: .mocking)
addMapEntry(mockInput, mockDependencySource)
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,7 @@ extension ModuleDependencyGraph.Tracer {
path.compactMap { node in
node.dependencySource.map {
source in
graph.inputDependencySourceMap.input(ifKnownFor: source).map {
input in
graph.inputDependencySourceMap[source].map { input in
"\(node.key) in \(input.file.basename)"
}
?? "\(node.key)"
Expand Down