Skip to content

Commit d09c906

Browse files
author
David Ungar
committed
Fix overly conservative fingerprint bug & fix tests.
1 parent 62e0da9 commit d09c906

File tree

12 files changed

+62
-57
lines changed

12 files changed

+62
-57
lines changed

‎lib/AST/AbstractSourceFileDepGraphFactory.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,14 @@ void AbstractSourceFileDepGraphFactory::addAUsedDecl(
7979
const DependencyKey &defKey, const DependencyKey &useKey) {
8080
auto *defNode =
8181
g.findExistingNodeOrCreateIfNew(defKey, None, false /* = !isProvides */);
82+
// If the depended-upon node is defined in this file, then don't
83+
// create an arc to the user, when the user is the whole file.
84+
// Otherwise, if the defNode's type-body fingerprint changes,
85+
// the whole file will be marked as dirty, losing the benefit of the
86+
// fingerprint.
87+
if (defNode->getIsProvides() &&
88+
useKey.getKind() == NodeKind::sourceFileProvide)
89+
return;
8290
auto nullableUse = g.findExistingNode(useKey);
8391
assert(nullableUse.isNonNull() && "Use must be an already-added provides");
8492
auto *useNode = nullableUse.get();

‎test/Frontend/Inputs/type-fingerprint/a.swift

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎test/Frontend/Inputs/type-fingerprint/b0.swift

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎test/Frontend/Inputs/type-fingerprint/b1.swift

Lines changed: 0 additions & 8 deletions
This file was deleted.

‎test/Frontend/Inputs/type-fingerprint/definesAB-after.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
struct A {
2+
var x = 17
3+
}
4+
struct B {
5+
}

‎test/Frontend/Inputs/type-fingerprint/definesAB-before.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
struct A {
2+
}
3+
struct B {
4+
}

‎test/Frontend/Inputs/type-fingerprint/main.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1 @@
1-
func foo() {
2-
B1()
3-
}
1+

‎test/Frontend/Inputs/type-fingerprint/ofm.json

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
{
2-
"./a.swift": {
3-
"object": "./a.o",
4-
"swift-dependencies": "./a.swiftdeps"
5-
},
6-
"./b.swift": {
7-
"object": "./b.o",
8-
"swift-dependencies": "./b.swiftdeps"
9-
},
10-
"./main.swift": {
2+
"main.swift": {
113
"object": "./main.o",
124
"swift-dependencies": "./main.swiftdeps"
135
},
6+
"definesAB.swift": {
7+
"object": "./definesAB.o",
8+
"swift-dependencies": "./definesAB.swiftdeps"
9+
},
10+
"usesA.swift": {
11+
"object": "./usesA.o",
12+
"swift-dependencies": "./usesA.swiftdeps"
13+
},
14+
"usesB.swift": {
15+
"object": "./usesB.o",
16+
"swift-dependencies": "./usesB.swiftdeps"
17+
},
1418
"": {
1519
"swift-dependencies": "./main~buildrecord.swiftdeps"
1620
}
1721
}
22+

‎test/Frontend/Inputs/type-fingerprint/usesA.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
let a = A()

‎test/Frontend/Inputs/type-fingerprint/usesB.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
let b = B()

‎test/Frontend/type-fingerprint.swift

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,38 +8,37 @@
88

99
// Establish status quo
1010

11+
1112
// RUN: %empty-directory(%t)
12-
// RUN: cp %S/Inputs/type-fingerprint/{main,a}.swift %t
13-
// RUN: cp %S/Inputs/type-fingerprint/ofm.json %t
14-
// RUN: cp %S/Inputs/type-fingerprint/b0.swift %t/b.swift
13+
// RUN: cp %S/Inputs/type-fingerprint/* %t
14+
// RUN: cp %t/definesAB{-before,}.swift
1515

1616
// Seeing weird failure on CI, so set the mod times
1717
// RUN: touch -t 200101010101 %t/*.swift
1818

19-
// RUN: cd %t && %swiftc_driver -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental ./main.swift ./a.swift ./b.swift -module-name main -output-file-map ofm.json >&output1
19+
// RUN: cd %t && %swiftc_driver -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift -module-name main -output-file-map ofm.json >&output1
2020

21-
// only-run-for-debugging: cp %t/b.swiftdeps %t/b1.swiftdeps
21+
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB1.swiftdeps
2222

2323
// Change one type, but uses of all types get recompiled
2424

25-
// RUN: cp %S/Inputs/type-fingerprint/b1.swift %t/b.swift
25+
// RUN: cp %t/definesAB{-after,}.swift
2626

27-
// Seeing weird failure on CI, so ensure that b.swift is newer
27+
// Seeing weird failure on CI, so ensure that definesAB.swift is newer
2828
// RUN: touch -t 200201010101 %t/*
2929
// RUN: touch -t 200101010101 %t/*.swift
30-
// RUN: touch -t 200301010101 %t/b.swift
30+
// RUN: touch -t 200301010101 %t/definesAB.swift
3131

32-
// RUN: cd %t && %swiftc_driver -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental ./main.swift ./a.swift ./b.swift -module-name main -output-file-map ofm.json >&output2
32+
// RUN: cd %t && %swiftc_driver -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift -module-name main -output-file-map ofm.json >&output2
3333

3434
// Save for debugging:
35-
// only-run-for-debugging: cp %t/b.swiftdeps %t/b2.swiftdeps
35+
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB1.swiftdeps
3636

3737
// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output2
3838

39-
// CHECK-MAINAB-RECOMPILED: Queuing (initial): {compile: b.o <= b.swift}
40-
// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: main.o <= main.swift}
41-
// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: a.o <= a.swift}
42-
39+
// CHECK-MAINAB-RECOMPILED: Queuing (initial): {compile: definesAB.o <= definesAB.swift}
40+
// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}
41+
// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
4342

4443
// =============================================================================
4544
// With the fingerprints
@@ -48,33 +47,33 @@
4847
// Establish status quo
4948

5049
// RUN: %empty-directory(%t)
51-
// RUN: cp %S/Inputs/type-fingerprint/{main,a}.swift %t
52-
// RUN: cp %S/Inputs/type-fingerprint/ofm.json %t
53-
// RUN: cp %S/Inputs/type-fingerprint/b0.swift %t/b.swift
50+
// RUN: cp %S/Inputs/type-fingerprint/* %t
51+
// RUN: cp %t/definesAB{-before,}.swift
5452

5553
// Seeing weird failure on CI, so set the mod times
5654
// RUN: touch -t 200101010101 %t/*.swift
5755

58-
// RUN: cd %t && %swiftc_driver -enable-batch-mode -j2 -incremental -driver-show-incremental ./main.swift ./a.swift ./b.swift -module-name main -output-file-map ofm.json >&output3
56+
// RUN: cd %t && %swiftc_driver -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift -module-name main -output-file-map ofm.json >&output3
57+
58+
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB3.swiftdeps
5959

60-
// only-run-for-debugging: cp %t/b.swiftdeps %t/b3.swiftdeps
6160

6261
// Change one type, only uses of that type get recompiled
6362

64-
// RUN: cp %S/Inputs/type-fingerprint/b1.swift %t/b.swift
63+
// RUN: cp %t/definesAB{-after,}.swift
6564

66-
// Seeing weird failure on CI, so ensure that b.swift is newer
65+
// Seeing weird failure on CI, so ensure that definesAB.swift is newer
6766
// RUN: touch -t 200201010101 %t/*
6867
// RUN: touch -t 200101010101 %t/*.swift
69-
// RUN: touch -t 200301010101 %t/b.swift
68+
// RUN: touch -t 200301010101 %t/definesAB.swift
7069

71-
// RUN: cd %t && %swiftc_driver -enable-batch-mode -j2 -incremental -driver-show-incremental ./main.swift ./a.swift ./b.swift -module-name main -output-file-map ofm.json >&output4
70+
// RUN: cd %t && %swiftc_driver -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift -module-name main -output-file-map ofm.json >&output4
7271

73-
// only-run-for-debugging: cp %t/b.swiftdeps %t/b4.swiftdeps
72+
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB4.swiftdeps
7473

7574
// RUN: %FileCheck -check-prefix=CHECK-MAINB-RECOMPILED %s < %t/output4
7675

77-
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: a.o <= a.swift}
78-
// CHECK-MAINB-RECOMPILED: Queuing because of dependencies discovered later: {compile: main.o <= main.swift}
79-
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: // CHECK-MAINB-RECOMPILED: a.o <= a.swift}
76+
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
77+
// CHECK-MAINB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}
78+
// CHECK-MAINB-RECOMPILED-NOT: Queuing because of dependencies discovered later: {compile: usesB.o <= usesB.swift}
8079

‎unittests/Driver/TypeBodyFingerprintsDependencyGraphTests.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -798,8 +798,9 @@ TEST(ModuleDepGraph, UseFingerprints) {
798798

799799
// Because when A1 changes, B1 and not B2 is affected, only jobs1 and job2
800800
// should be recompiled, except type fingerprints is off!
801+
// Include a dependency on A1, to ensure it does not muck things up.
801802

802-
simulateLoad(graph, &job0, {{NodeKind::nominal, {"A1@1", "A2@2"}}});
803+
simulateLoad(graph, &job0, {{NodeKind::nominal, {"A1@1", "A2@2", "A1->"}}});
803804
simulateLoad(graph, &job1, {{NodeKind::nominal, {"B1", "A1->"}}});
804805
simulateLoad(graph, &job2, {{NodeKind::nominal, {"C1", "A2->"}}});
805806
simulateLoad(graph, &job3, {{NodeKind::nominal, {"D1"}}});

0 commit comments

Comments
 (0)