Skip to content

[5.5][Incremental Imports] Print more info about externals when setting -driver-show-incremental #638

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 1 commit into from
May 7, 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
61 changes: 61 additions & 0 deletions Sources/SwiftDriver/IncrementalCompilation/DependencyKey.swift
Original file line number Diff line number Diff line change
Expand Up @@ -367,3 +367,64 @@ extension DependencyKey: Comparable {

extension DependencyKey.Designator: Comparable {
}

// MARK: - InvalidationReason
extension ExternalDependency {
/// When explaining incremental decisions, it helps to know why a particular external dependency
/// was investigated.
public enum Why: String, CustomStringConvertible {
/// An `Import` of this file was added to the source code.
case added

/// The imported file has changed.
case changed

/// Used when testing invalidation
case testing

/// Figure out the reason to invalidate or process a dependency.
/// Even if invalidation won't be reported to the caller, a new or added
/// incremental external dependency may require integration in order to
/// transitively close them, (e.g. if an imported module imports a module).
init?(should fed: FingerprintedExternalDependency,
whichIsNewToTheGraph isNewToTheGraph: Bool,
closeOverSwiftModulesIn graph: ModuleDependencyGraph) {
guard graph.info.isCrossModuleIncrementalBuildEnabled
else { return nil }
self.init(should: fed,
whichIsSignificantlyNew: isNewToTheGraph,
beInvestigatedIn: graph)
}


init?(shouldUsesOf fed: FingerprintedExternalDependency,
whichIsNewToTheGraph isNewToTheGraph: Bool,
beInvalidatedIn graph: ModuleDependencyGraph) {
if graph.phase.isCompilingAllInputsNoMatterWhat {
// going to compile every input anyway, less work for callers
return nil
}
let isSignificantlyNew = graph.phase.shouldNewExternalDependenciesTriggerInvalidation && isNewToTheGraph
self.init(should: fed,
whichIsSignificantlyNew: isSignificantlyNew,
beInvestigatedIn: graph)
}

/// Figure out the reason to invalidate or process a dependency
private init?(should fed: FingerprintedExternalDependency,
whichIsSignificantlyNew isSignificantlyNew: Bool,
beInvestigatedIn graph: ModuleDependencyGraph) {
if isSignificantlyNew {
self = .added
return
}
if graph.hasFileChanged(of: fed.externalDependency) {
self = .changed
return
}
return nil
}

public var description: String { rawValue }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,20 @@ extension IncrementalCompilationState {
func reportIncrementalCompilationHasBeenDisabled(_ why: String) {
report("Incremental compilation has been disabled, \(why)")
}

func reportInvalidated<Nodes: Sequence>(
_ nodes: Nodes,
by externalDependency: ExternalDependency,
_ why: ExternalDependency.Why
)
where Nodes.Element == ModuleDependencyGraph.Node
{
let whyString = why.description.capitalized
let depString = externalDependency.shortDescription
for node in nodes {
report("\(whyString): \(depString) -> \(node)")
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,32 @@ extension ModuleDependencyGraph {
}
}

var shouldNewExternalDependenciesTriggerInvalidation: Bool {
switch self {
case .buildingWithoutAPrior:
// Reading graph from a swiftdeps file,
// so every incremental external dependency found will be new to the
// graph. Don't invalidate just 'cause it's new.
return false

case .buildingAfterEachCompilation:
// Will be compiling every file, so no need to invalidate based on
// found external dependencies.
return false

// Reading a swiftdeps file after a compilation.
// A new external dependency represents an addition.
// So must invalidate based on it.
case .updatingAfterCompilation:
return true

case .updatingFromAPrior:
// If the graph was read from priors,
// then any new external dependency must also be an addition.
return true
}
}

var isCompilingAllInputsNoMatterWhat: Bool {
switch self {
case .buildingAfterEachCompilation:
Expand Down Expand Up @@ -240,6 +266,7 @@ extension ModuleDependencyGraph {
/// As an optimization, only return the nodes that have not been already traced, because the traced nodes
/// will have already been used to schedule jobs to run.
/*@_spi(Testing)*/ public func collectUntracedNodesUsing(
_ why: ExternalDependency.Why,
_ fingerprintedExternalDependency: FingerprintedExternalDependency
) -> DirectlyInvalidatedNodeSet {
// These nodes will depend on the *interface* of the external Decl.
Expand All @@ -252,10 +279,12 @@ extension ModuleDependencyGraph {
let node = Node(key: key,
fingerprint: fingerprintedExternalDependency.fingerprint,
dependencySource: nil)
return DirectlyInvalidatedNodeSet(
let untracedUses = DirectlyInvalidatedNodeSet(
nodeFinder
.uses(of: node)
.filter({ use in use.isUntraced }))
info.reporter?.reportInvalidated(untracedUses, by: fingerprintedExternalDependency.externalDependency, why)
return untracedUses
}

/// Find all the inputs known to need recompilation as a consequence of reading a swiftdeps or swiftmodule
Expand Down Expand Up @@ -308,38 +337,32 @@ extension ModuleDependencyGraph {
isPresentInTheGraph: Bool?)
-> DirectlyInvalidatedNodeSet {

/// Compute this up front as an optimization.
let isNewToTheGraph = isPresentInTheGraph != true && fingerprintedExternalDependencies.insert(fed).inserted

// If the graph already includes prior externals, then any new externals are changes
// Short-circuit conjunction may avoid the modTime query
let shouldTryToProcess = info.isCrossModuleIncrementalBuildEnabled &&
(isNewToTheGraph || hasFileChanged(of: fed.externalDependency))

// Do this no matter what in order to integrate any incremental external dependencies.
let invalidatedNodesFromIncrementalExternal = shouldTryToProcess
? collectNodesInvalidatedByAttemptingToProcess(fed, info)
: nil
let whyIntegrateForClosure = ExternalDependency.Why(
should: fed,
whichIsNewToTheGraph: isNewToTheGraph,
closeOverSwiftModulesIn: self)

if phase.isCompilingAllInputsNoMatterWhat {
// going to compile every input anyway, less work for callers
return DirectlyInvalidatedNodeSet()
let invalidatedNodesFromIncrementalExternal = whyIntegrateForClosure.flatMap { why in
collectNodesInvalidatedByAttemptingToProcess(why, fed)
}

/// When building a graph from scratch, an unchanged but new-to-the-graph external dependendcy should be ignored.
/// Otherwise, it represents an added Import
let callerWantsTheseChanges = (phase.isUpdating && isNewToTheGraph) ||
hasFileChanged(of: fed.externalDependency)

guard callerWantsTheseChanges else {
guard let whyInvalidate = ExternalDependency.Why(
shouldUsesOf: fed,
whichIsNewToTheGraph: isNewToTheGraph,
beInvalidatedIn: self)
else {
return DirectlyInvalidatedNodeSet()
}

// If there was an error integrating the external dependency, or if it was not an incremental one,
// return anything that uses that dependency.
return invalidatedNodesFromIncrementalExternal ?? collectUntracedNodesUsing(fed)
return invalidatedNodesFromIncrementalExternal ?? collectUntracedNodesUsing(whyInvalidate, fed)
}

private func hasFileChanged(of externalDependency: ExternalDependency
func hasFileChanged(of externalDependency: ExternalDependency
) -> Bool {
if let hasChanged = externalDependencyModTimeCache[externalDependency] {
return hasChanged
Expand All @@ -356,14 +379,17 @@ extension ModuleDependencyGraph {
/// Try to read and integrate an external dependency.
/// Return nil if it's not incremental, or if an error occurs.
private func collectNodesInvalidatedByAttemptingToProcess(
_ fed: FingerprintedExternalDependency,
_ info: IncrementalCompilationState.IncrementalDependencyAndInputSetup) -> DirectlyInvalidatedNodeSet? {
fed.incrementalDependencySource?
.read(in: info.fileSystem, reporter: info.reporter)
.map { unserializedDepGraph in
info.reporter?.report("Integrating changes from: \(fed.externalDependency)")
return Integrator.integrate(from: unserializedDepGraph, into: self)
}
_ why: ExternalDependency.Why,
_ fed: FingerprintedExternalDependency
) -> DirectlyInvalidatedNodeSet? {
guard let source = fed.incrementalDependencySource,
let unserializedDepGraph = source.read(in: info.fileSystem, reporter: info.reporter)
else {
return nil
}
let invalidatedNodes = Integrator.integrate(from: unserializedDepGraph, into: self)
info.reporter?.reportInvalidated(invalidatedNodes, by: fed.externalDependency, why)
return invalidatedNodes
}
}

Expand Down
2 changes: 1 addition & 1 deletion Tests/SwiftDriverTests/ModuleDependencyGraphTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,7 @@ extension ModuleDependencyGraph {
on fingerprintedExternalDependency: FingerprintedExternalDependency
) -> [DependencySource] {
var foundSources = [DependencySource]()
for dependent in collectUntracedNodesUsing(fingerprintedExternalDependency) {
for dependent in collectUntracedNodesUsing(.testing, fingerprintedExternalDependency) {
let dependencySource = dependent.dependencySource!
foundSources.append(dependencySource)
// findSwiftDepsToRecompileWhenWholeSwiftDepChanges is reflexive
Expand Down