Skip to content
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

[Incremental] Use source file fingerprints *without* top-level type bodies for incremental imports #36188

Merged
merged 3 commits into from
Feb 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 14 additions & 3 deletions include/swift/AST/RawComment.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ class BasicSourceFileInfo {

StringRef FilePath;
Fingerprint InterfaceHashIncludingTypeMembers = Fingerprint::ZERO();
/// Does *not* include the type-body hashes of the top level types.
/// Just the `SourceFile` hashes.
/// Used for incremental imports.
Fingerprint InterfaceHashExcludingTypeMembers = Fingerprint::ZERO();
llvm::sys::TimePoint<> LastModified = {};
uint64_t FileSize = 0;

Expand All @@ -113,14 +117,16 @@ class BasicSourceFileInfo {
public:
BasicSourceFileInfo(StringRef FilePath,
Fingerprint InterfaceHashIncludingTypeMembers,
Fingerprint InterfaceHashExcludingTypeMembers,
llvm::sys::TimePoint<> LastModified, uint64_t FileSize)
: FilePath(FilePath),
InterfaceHashIncludingTypeMembers(InterfaceHashIncludingTypeMembers),
InterfaceHashExcludingTypeMembers(InterfaceHashExcludingTypeMembers),
LastModified(LastModified), FileSize(FileSize) {}

/// Construct with a 'SourceFile'. 'getInterfaceHashIncludingTypeMembers()',
/// 'getLastModified()' and 'getFileSize()' are laizily pupulated when
/// accessed.
/// Construct with a `SourceFile`. `getInterfaceHashIncludingTypeMembers()`,
/// `getInterfaceHashExcludingTypeMembers()`, `getLastModified()` and `getFileSize()` are laizily
/// populated when accessed.
BasicSourceFileInfo(const SourceFile *SF);

bool isFromSourceFile() const;
Expand All @@ -132,6 +138,11 @@ class BasicSourceFileInfo {
return InterfaceHashIncludingTypeMembers;
}

Fingerprint getInterfaceHashExcludingTypeMembers() const {
const_cast<BasicSourceFileInfo *>(this)->populateWithSourceFileIfNeeded();
return InterfaceHashExcludingTypeMembers;
}

llvm::sys::TimePoint<> getLastModified() const {
const_cast<BasicSourceFileInfo *>(this)->populateWithSourceFileIfNeeded();
return LastModified;
Expand Down
4 changes: 3 additions & 1 deletion lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1550,7 +1550,9 @@ Fingerprint ModuleDecl::getFingerprint() const {
StableHasher hasher = StableHasher::defaultHasher();
SmallVector<Fingerprint, 16> FPs;
collectBasicSourceFileInfo([&](const BasicSourceFileInfo &bsfi) {
FPs.emplace_back(bsfi.getInterfaceHashIncludingTypeMembers());
// For incremental imports, the hash must be insensitive to type-body
// changes, so use the one without type members.
FPs.emplace_back(bsfi.getInterfaceHashExcludingTypeMembers());
});

// Sort the fingerprints lexicographically so we have a stable hash despite
Expand Down
2 changes: 2 additions & 0 deletions lib/AST/RawComment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,11 @@ void BasicSourceFileInfo::populateWithSourceFileIfNeeded() {

if (SF->hasInterfaceHash()) {
InterfaceHashIncludingTypeMembers = SF->getInterfaceHashIncludingTypeMembers();
InterfaceHashExcludingTypeMembers = SF->getInterfaceHash();
} else {
// FIXME: Parse the file with EnableInterfaceHash option.
InterfaceHashIncludingTypeMembers = Fingerprint::ZERO();
InterfaceHashExcludingTypeMembers = Fingerprint::ZERO();
}

return;
Expand Down
33 changes: 26 additions & 7 deletions lib/Serialization/ModuleFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -978,10 +978,17 @@ void ModuleFile::collectBasicSourceFileInfo(
while (Cursor < End) {
// FilePath (byte offset in 'SourceLocsTextData').
auto fileID = endian::readNext<uint32_t, little, unaligned>(Cursor);
// InterfaceHash (fixed length string).
auto fpStr = StringRef{reinterpret_cast<const char *>(Cursor),

// InterfaceHashIncludingTypeMembers (fixed length string).
auto fpStrIncludingTypeMembers = StringRef{reinterpret_cast<const char *>(Cursor),
Fingerprint::DIGEST_LENGTH};
Cursor += Fingerprint::DIGEST_LENGTH;

// InterfaceHashExcludingTypeMembers (fixed length string).
auto fpStrExcludingTypeMembers = StringRef{reinterpret_cast<const char *>(Cursor),
Fingerprint::DIGEST_LENGTH};
Cursor += Fingerprint::DIGEST_LENGTH;

// LastModified (nanoseconds since epoch).
auto timestamp = endian::readNext<uint64_t, little, unaligned>(Cursor);
// FileSize (num of bytes).
Expand All @@ -992,13 +999,25 @@ void ModuleFile::collectBasicSourceFileInfo(
size_t terminatorOffset = filePath.find('\0');
filePath = filePath.slice(0, terminatorOffset);

auto fingerprint = Fingerprint::fromString(fpStr);
if (!fingerprint) {
llvm::errs() << "Unconvertable fingerprint '" << fpStr << "'\n";
auto fingerprintIncludingTypeMembers =
Fingerprint::fromString(fpStrIncludingTypeMembers);
if (!fingerprintIncludingTypeMembers) {
llvm::errs() << "Unconvertible fingerprint including type members'"
<< fpStrIncludingTypeMembers << "'\n";
abort();
}

callback(BasicSourceFileInfo(filePath, fingerprint.getValue(), llvm::sys::TimePoint<>(std::chrono::nanoseconds(timestamp)), fileSize));
auto fingerprintExcludingTypeMembers =
Fingerprint::fromString(fpStrExcludingTypeMembers);
if (!fingerprintExcludingTypeMembers) {
llvm::errs() << "Unconvertible fingerprint excluding type members'"
<< fpStrExcludingTypeMembers << "'\n";
abort();
}
callback(BasicSourceFileInfo(filePath,
fingerprintIncludingTypeMembers.getValue(),
fingerprintExcludingTypeMembers.getValue(),
llvm::sys::TimePoint<>(std::chrono::nanoseconds(timestamp)),
fileSize));
}
}

Expand Down
19 changes: 15 additions & 4 deletions lib/Serialization/SerializeDoc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,12 @@ static void emitFileListRecord(llvm::BitstreamWriter &Out,
return;

auto fileID = FWriter.getTextOffset(absolutePath);
auto fingerprintStr = info.getInterfaceHashIncludingTypeMembers().getRawValue();

auto fingerprintStrIncludingTypeMembers =
info.getInterfaceHashIncludingTypeMembers().getRawValue();
auto fingerprintStrExcludingTypeMembers =
info.getInterfaceHashExcludingTypeMembers().getRawValue();

auto timestamp = std::chrono::duration_cast<std::chrono::nanoseconds>(
info.getLastModified().time_since_epoch())
.count();
Expand All @@ -814,9 +819,15 @@ static void emitFileListRecord(llvm::BitstreamWriter &Out,
endian::Writer writer(out, little);
// FilePath.
writer.write<uint32_t>(fileID);
// InterfaceHash (fixed length string).
assert(fingerprintStr.size() == Fingerprint::DIGEST_LENGTH);
out << fingerprintStr;

// InterfaceHashIncludingTypeMembers (fixed length string).
assert(fingerprintStrIncludingTypeMembers.size() == Fingerprint::DIGEST_LENGTH);
out << fingerprintStrIncludingTypeMembers;

// InterfaceHashExcludingTypeMembers (fixed length string).
assert(fingerprintStrExcludingTypeMembers.size() == Fingerprint::DIGEST_LENGTH);
out << fingerprintStrExcludingTypeMembers;

// LastModified (nanoseconds since epoch).
writer.write<uint64_t>(timestamp);
// FileSize (num of bytes).
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
public func fromC(parameter: Int = 0) {}

struct S {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
public func fromC(parameter: Int = 0) {}

struct S {
public func member() {}
}
public func other() {}
31 changes: 25 additions & 6 deletions test/Incremental/CrossModule/module-fingerprint.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
// RUN: %target-swift-ide-test -print-module-metadata -module-to-print B -enable-swiftsourceinfo -I %t -source-filename %s | %FileCheck %s --check-prefix=CHECK-CLEAN-B
// RUN: %target-swift-ide-test -print-module-metadata -module-to-print A -enable-swiftsourceinfo -I %t -source-filename %s | %FileCheck %s --check-prefix=CHECK-CLEAN-A

// CHECK-CLEAN-C: fingerprint=6e60fd224d614a59568a348e0ac9b55a
// CHECK-CLEAN-B: fingerprint=bfa14052e1df9253b7bf7e0eb7cfc505
// CHECK-CLEAN-A: fingerprint=a939d89f07c766a4607c5fe1a1715cf5
// CHECK-CLEAN-C: fingerprint=7957733a8ee9bc44f726a516108786eb
// CHECK-CLEAN-B: fingerprint=17bb71bdc7972a93446d524f47044156
// CHECK-CLEAN-A: fingerprint=048332944f6e149f2e8bed309d47283d

//
// Now change C and ensure that B rebuilds but A does not
Expand All @@ -32,6 +32,25 @@
// RUN: %target-swift-ide-test -print-module-metadata -module-to-print B -enable-swiftsourceinfo -I %t -source-filename %s | %FileCheck %s --check-prefix=CHECK-INCREMENTAL-B
// RUN: %target-swift-ide-test -print-module-metadata -module-to-print A -enable-swiftsourceinfo -I %t -source-filename %s | %FileCheck %s --check-prefix=CHECK-INCREMENTAL-A

// CHECK-INCREMENTAL-C: fingerprint=3e68d59b74032e18401ec978cdb11cf3
// CHECK-INCREMENTAL-B: fingerprint=bfa14052e1df9253b7bf7e0eb7cfc505
// CHECK-INCREMENTAL-A: fingerprint=a939d89f07c766a4607c5fe1a1715cf5
// CHECK-INCREMENTAL-C: fingerprint=263f083edcaaf08536f657d10082dacc
// CHECK-INCREMENTAL-B: fingerprint=17bb71bdc7972a93446d524f47044156
// CHECK-INCREMENTAL-A: fingerprint=048332944f6e149f2e8bed309d47283d

//
// Now change a top-level type of C and ensure that C's fingerprint does not change
//

// RUN: cp %S/Inputs/module-fingerprint/C2.swift %t/C.swift

// RUN: cd %t && %target-swiftc_driver -c -incremental -emit-dependencies -emit-module -emit-module-path %t/C.swiftmodule -enable-experimental-cross-module-incremental-build -module-name C -I %t -output-file-map %t/C.json -working-directory %t -driver-show-incremental -driver-show-job-lifecycle C.swift
// RUN: touch %t/C.swiftmodule
// RUN: cd %t && %target-swiftc_driver -c -incremental -emit-dependencies -emit-module -emit-module-path %t/B.swiftmodule -enable-experimental-cross-module-incremental-build -module-name B -I %t -output-file-map %t/B.json -working-directory %t -driver-show-incremental -driver-show-job-lifecycle B.swift
// RUN: cd %t && %target-swiftc_driver -c -incremental -emit-dependencies -emit-module -emit-module-path %t/A.swiftmodule -enable-experimental-cross-module-incremental-build -module-name A -I %t -output-file-map %t/A.json -working-directory %t -driver-show-incremental -driver-show-job-lifecycle A.swift

// RUN: %target-swift-ide-test -print-module-metadata -module-to-print C -enable-swiftsourceinfo -I %t -source-filename %s | %FileCheck %s --check-prefix=CHECK-INCREMENTAL2-C
// RUN: %target-swift-ide-test -print-module-metadata -module-to-print B -enable-swiftsourceinfo -I %t -source-filename %s | %FileCheck %s --check-prefix=CHECK-INCREMENTAL2-B
// RUN: %target-swift-ide-test -print-module-metadata -module-to-print A -enable-swiftsourceinfo -I %t -source-filename %s | %FileCheck %s --check-prefix=CHECK-INCREMENTAL2-A

// CHECK-INCREMENTAL2-C: fingerprint=263f083edcaaf08536f657d10082dacc
// CHECK-INCREMENTAL2-B: fingerprint=17bb71bdc7972a93446d524f47044156
// CHECK-INCREMENTAL2-A: fingerprint=048332944f6e149f2e8bed309d47283d
4 changes: 2 additions & 2 deletions test/Serialization/sourceinfo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ import MyModule
// RUN: %target-swiftc_driver -emit-module -module-name MyModule -o %t/Modules/MyModule.swiftmodule %S/Inputs/SourceInfo/File1.swift %S/Inputs/SourceInfo/File2.swift
// RUN: %target-swift-ide-test -print-module-metadata -module-to-print MyModule -enable-swiftsourceinfo -I %t/Modules -source-filename %s | %FileCheck %s

// CHECK: filepath=SOURCE_DIR{{[/\\]}}test{{[/\\]}}Serialization{{[/\\]}}Inputs{{[/\\]}}SourceInfo{{[/\\]}}File1.swift; hash=9da710e9b2de1fff2915639236b8929c; mtime={{[0-9]{4}-[0-9]{2}-[0-9]{2} .*}}; size=35
// CHECK: filepath=SOURCE_DIR{{[/\\]}}test{{[/\\]}}Serialization{{[/\\]}}Inputs{{[/\\]}}SourceInfo{{[/\\]}}File2.swift; hash=22b75a7717318d48f7a979906f35195e; mtime={{[0-9]{4}-[0-9]{2}-[0-9]{2} .*}}; size=57
// CHECK: filepath=SOURCE_DIR{{[/\\]}}test{{[/\\]}}Serialization{{[/\\]}}Inputs{{[/\\]}}SourceInfo{{[/\\]}}File1.swift; hash=9da710e9b2de1fff2915639236b8929c; hashExcludingTypeMembers=bef81a9bfc04156da679d8d579125d39; mtime={{[0-9]{4}-[0-9]{2}-[0-9]{2} .*}}; size=35
// CHECK: filepath=SOURCE_DIR{{[/\\]}}test{{[/\\]}}Serialization{{[/\\]}}Inputs{{[/\\]}}SourceInfo{{[/\\]}}File2.swift; hash=22b75a7717318d48f7a979906f35195e; hashExcludingTypeMembers=7a22dbe5fc611122201f7d6b53094531; mtime={{[0-9]{4}-[0-9]{2}-[0-9]{2} .*}}; size=57
2 changes: 2 additions & 0 deletions tools/swift-ide-test/swift-ide-test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2519,6 +2519,8 @@ static void printModuleMetadata(ModuleDecl *MD) {
MD->collectBasicSourceFileInfo([&](const BasicSourceFileInfo &info) {
OS << "filepath=" << info.getFilePath() << "; ";
OS << "hash=" << info.getInterfaceHashIncludingTypeMembers().getRawValue() << "; ";
OS << "hashExcludingTypeMembers="
<< info.getInterfaceHashExcludingTypeMembers().getRawValue() << "; ";
OS << "mtime=" << info.getLastModified() << "; ";
OS << "size=" << info.getFileSize();
OS << "\n";
Expand Down