Skip to content

Commit 58d622d

Browse files
author
Nathan Hawes
committed
[ParseableInterface] Don't serialize resource directory deps and stop adding cached modules to the dependency tracker
This patch modifies ParseableInterfaceBuilder::CollectDepsForSerialization to avoid serializing dependencies from the runtime resource path into the swiftmodules generated from .swiftinterface files. This means the module cache should now be relocatable across machines. It also modifies ParseableInterfaceModuleLoader to never add any dependencies from the module cache and prebuilt cache to the dependency tracker (in addition to the existing behaviour of not serializing them in the generated swiftmodules). As a result, CollectDepsForSerialization no longer checks if the dependencies it is given come from the cache as they are provided by the dependency tracker. It now asserts that's the case instead.
1 parent accc647 commit 58d622d

File tree

6 files changed

+69
-63
lines changed

6 files changed

+69
-63
lines changed

include/swift/Frontend/ParseableInterfaceModuleLoader.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ class ParseableInterfaceModuleLoader : public SerializedModuleLoaderBase {
139139
std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer,
140140
std::unique_ptr<llvm::MemoryBuffer> *ModuleDocBuffer) override;
141141

142+
bool isCached(StringRef DepPath) override;
143+
142144
public:
143145
static std::unique_ptr<ParseableInterfaceModuleLoader>
144146
create(ASTContext &ctx, StringRef cacheDir, StringRef prebuiltCacheDir,

include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ class SerializedModuleLoaderBase : public ModuleLoader {
8080
return false;
8181
}
8282

83+
/// Determines if the provided path is a cached artifact for dependency
84+
/// tracking purposes.
85+
virtual bool isCached(StringRef DepPath) {
86+
return false;
87+
}
88+
8389
public:
8490
virtual ~SerializedModuleLoaderBase();
8591
SerializedModuleLoaderBase(const SerializedModuleLoaderBase &) = delete;

lib/Frontend/ParseableInterfaceModuleLoader.cpp

Lines changed: 29 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -356,19 +356,6 @@ class swift::ParseableInterfaceBuilder {
356356
return false;
357357
}
358358

359-
/// Determines if the dependency with the provided path is a swiftmodule in
360-
/// either the module cache or prebuilt module cache
361-
bool isCachedModule(StringRef DepName) const {
362-
auto Ext = llvm::sys::path::extension(DepName);
363-
auto Ty = file_types::lookupTypeForExtension(Ext);
364-
365-
if (Ty != file_types::TY_SwiftModuleFile)
366-
return false;
367-
if (!moduleCachePath.empty() && DepName.startswith(moduleCachePath))
368-
return true;
369-
return !prebuiltCachePath.empty() && DepName.startswith(prebuiltCachePath);
370-
}
371-
372359
/// Populate the provided \p Deps with \c FileDependency entries for all
373360
/// dependencies \p SubInstance's DependencyTracker recorded while compiling
374361
/// the module, excepting .swiftmodules in \p moduleCachePath or
@@ -379,12 +366,31 @@ class swift::ParseableInterfaceBuilder {
379366
SmallVectorImpl<FileDependency> &Deps,
380367
bool IsHashBased) {
381368
StringRef SDKPath = SubInstance.getASTContext().SearchPathOpts.SDKPath;
369+
StringRef ResourcePath =
370+
SubInstance.getASTContext().SearchPathOpts.RuntimeResourcePath;
382371
auto DTDeps = SubInstance.getDependencyTracker()->getDependencies();
383372
SmallVector<StringRef, 16> InitialDepNames(DTDeps.begin(), DTDeps.end());
384373
InitialDepNames.push_back(interfacePath);
385374
llvm::StringSet<> AllDepNames;
386375

387376
for (auto const &DepName : InitialDepNames) {
377+
assert(moduleCachePath.empty() || !DepName.startswith(moduleCachePath));
378+
assert(prebuiltCachePath.empty() || !DepName.startswith(prebuiltCachePath));
379+
380+
/// Lazily load the dependency buffer if we need it. If we're not
381+
/// dealing with a hash-based dependencies, and if the dependency is
382+
/// not a .swiftmodule, we can avoid opening the buffer.
383+
std::unique_ptr<llvm::MemoryBuffer> DepBuf = nullptr;
384+
auto getDepBuf = [&]() -> llvm::MemoryBuffer * {
385+
if (DepBuf) return DepBuf.get();
386+
if (auto Buf = getBufferOfDependency(fs, DepName, interfacePath,
387+
diags, diagnosticLoc)) {
388+
DepBuf = std::move(Buf);
389+
return DepBuf.get();
390+
}
391+
return nullptr;
392+
};
393+
388394
// Adjust the paths of dependences in the SDK to be relative to it
389395
bool IsSDKRelative = false;
390396
StringRef DepNameToStore = DepName;
@@ -410,48 +416,10 @@ class swift::ParseableInterfaceBuilder {
410416
dependencyTracker->addDependency(DepName, /*isSystem*/IsSDKRelative);
411417
}
412418

413-
/// Lazily load the dependency buffer if we need it. If we're not
414-
/// dealing with a hash-based dependencies, and if the dependency is
415-
/// not a .swiftmodule, we can avoid opening the buffer.
416-
std::unique_ptr<llvm::MemoryBuffer> DepBuf = nullptr;
417-
auto getDepBuf = [&]() -> llvm::MemoryBuffer * {
418-
if (DepBuf) return DepBuf.get();
419-
if (auto Buf = getBufferOfDependency(fs, DepName, interfacePath,
420-
diags, diagnosticLoc)) {
421-
DepBuf = std::move(Buf);
422-
return DepBuf.get();
423-
}
424-
return nullptr;
425-
};
426-
427-
// If Dep is itself a cached .swiftmodule, pull out its deps and include
428-
// them in our own, so we have a single-file view of transitive deps:
429-
// removes redundancies, makes the cache more relocatable, and avoids
430-
// opening and reading multiple swiftmodules during future loads.
431-
if (isCachedModule(DepName)) {
432-
auto buf = getDepBuf();
433-
if (!buf) return true;
434-
SmallVector<FileDependency, 16> SubDeps;
435-
auto VI = serialization::validateSerializedAST(buf->getBuffer(),
436-
/*ExtendedValidationInfo=*/nullptr, &SubDeps);
437-
if (VI.status != serialization::Status::Valid) {
438-
diags.diagnose(diagnosticLoc,
439-
diag::error_extracting_dependencies_from_cached_module,
440-
DepName);
441-
return true;
442-
}
443-
for (auto const &SubDep : SubDeps) {
444-
if (AllDepNames.insert(SubDep.getPath()).second) {
445-
Deps.push_back(SubDep);
446-
if (dependencyTracker)
447-
dependencyTracker->addDependency(
448-
SubDep.getPath(), /*IsSystem=*/SubDep.isSDKRelative());
449-
}
450-
}
419+
// Don't serialize compiler-relative deps so the cache is relocatable.
420+
if (DepName.startswith(ResourcePath))
451421
continue;
452-
}
453422

454-
// Otherwise, include this dependency directly
455423
auto Status = getStatusOfDependency(fs, DepName, interfacePath,
456424
diags, diagnosticLoc);
457425
if (!Status)
@@ -761,11 +729,11 @@ class ParseableInterfaceModuleLoaderImpl {
761729
if (dependencyTracker)
762730
dependencyTracker->addDependency(fullPath, /*isSystem*/in.isSDKRelative());
763731
if (!dependencyIsUpToDate(in, fullPath)) {
764-
LLVM_DEBUG(llvm::dbgs() << "Dep " << in.getPath()
732+
LLVM_DEBUG(llvm::dbgs() << "Dep " << fullPath
765733
<< " is directly out of date\n");
766734
return false;
767735
}
768-
LLVM_DEBUG(llvm::dbgs() << "Dep " << in.getPath() << " is up to date\n");
736+
LLVM_DEBUG(llvm::dbgs() << "Dep " << fullPath << " is up to date\n");
769737
}
770738
return true;
771739
}
@@ -1047,6 +1015,12 @@ class ParseableInterfaceModuleLoaderImpl {
10471015

10481016
} // end anonymous namespace
10491017

1018+
bool ParseableInterfaceModuleLoader::isCached(StringRef DepPath) {
1019+
if (!CacheDir.empty() && DepPath.startswith(CacheDir))
1020+
return true;
1021+
return !PrebuiltCacheDir.empty() && DepPath.startswith(PrebuiltCacheDir);
1022+
}
1023+
10501024
/// Load a .swiftmodule associated with a .swiftinterface either from a
10511025
/// cache or by converting it in a subordinate \c CompilerInstance, caching
10521026
/// the results.

lib/Serialization/SerializedModuleLoader.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -645,9 +645,13 @@ ModuleDecl *SerializedModuleLoaderBase::loadModule(SourceLoc importLoc,
645645
isFramework)) {
646646
return nullptr;
647647
}
648-
if (dependencyTracker)
649-
dependencyTracker->addDependency(moduleInputBuffer->getBufferIdentifier(),
650-
/*isSystem=*/false);
648+
if (dependencyTracker) {
649+
// Don't record cached artifacts as dependencies.
650+
StringRef DepPath = moduleInputBuffer->getBufferIdentifier();
651+
if (!isCached(DepPath)) {
652+
dependencyTracker->addDependency(DepPath, /*isSystem=*/false);
653+
}
654+
}
651655
}
652656

653657
assert(moduleInputBuffer);

test/ParseableInterface/ModuleCache/SDKDependencies.swift

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,14 @@
77
// RUN: %target-swift-frontend -build-module-from-parseable-interface -serialize-parseable-module-interface-dependency-hashes -sdk %t/my-sdk -prebuilt-module-cache-path %t/prebuilt-cache -I %t/my-sdk -module-cache-path %t/MCP -o %t/prebuilt-cache/ExportedLib.swiftmodule -track-system-dependencies -module-name ExportedLib %t/my-sdk/ExportedLib.swiftinterface
88
// RUN: %target-swift-frontend -build-module-from-parseable-interface -serialize-parseable-module-interface-dependency-hashes -sdk %t/my-sdk -prebuilt-module-cache-path %t/prebuilt-cache -I %t/my-sdk -module-cache-path %t/MCP -o %t/prebuilt-cache/SdkLib.swiftmodule -track-system-dependencies -module-name SdkLib %t/my-sdk/SdkLib.swiftinterface
99
//
10-
// Check the prebuilt modules don't contain dependencies in the module cache or prebuilt cache
10+
// Check the prebuilt modules don't contain dependencies in the module cache, prebuilt cache, or resource dir
1111
// RUN: llvm-bcanalyzer -dump %t/prebuilt-cache/ExportedLib.swiftmodule | %FileCheck %s -check-prefix=PREBUILT
1212
// RUN: llvm-bcanalyzer -dump %t/prebuilt-cache/SdkLib.swiftmodule | %FileCheck %s -check-prefix=PREBUILT
1313
//
1414
// PREBUILT: MODULE_BLOCK
1515
// PREBUILT-NOT: FILE_DEPENDENCY {{.*}}/MCP/{{.*}}
1616
// PREBUILT-NOT: FILE_DEPENDENCY {{.*}}/prebuilt-cache/{{.*}}
17+
// PREBUILD-NOT: FILE_DEPENDENCY {{.*}}/lib/swift/{{.*}}
1718
//
1819
// Re-build them in the opposite order
1920
// RUN: %empty-directory(%t/prebuilt-cache)
@@ -45,6 +46,12 @@
4546
// RUN: llvm-bcanalyzer -dump %t/MCP/SdkLib-*.swiftmodule | %FileCheck %s -check-prefix=PREBUILT
4647
// RUN: llvm-bcanalyzer -dump %t/MCP/ExportedLib-*.swiftmodule | %FileCheck %s -check-prefix=PREBUILT
4748
//
49+
// Check we didn't emit anything from the cache in the .d file either
50+
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE
51+
//
52+
// DEPFILE-NOT: /MCP/
53+
// DEPFILE-NOT: /prebuilt-cache/
54+
//
4855
// RUN: %empty-directory(%t/MCP)
4956
// RUN: echo '2: PASSED'
5057

@@ -82,6 +89,9 @@
8289
// NOCACHE-NOT: /prebuilt-cache/
8390
// NOCACHE-NOT: /MCP/
8491
//
92+
// Check we didn't emit anything from the cache in the .d file either
93+
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE
94+
//
8595
// RUN: %empty-directory(%t/MCP)
8696
// RUN: echo '3: PASSED'
8797

@@ -115,6 +125,9 @@
115125
// RUN: cat %t/MCP/ExportedLib-*.swiftmodule | %FileCheck %s -check-prefix=NOCACHE -DLIB_NAME=ExportedLib
116126
// RUN: cat %t/MCP/SdkLib-*.swiftmodule | %FileCheck %s -check-prefix=NOCACHE -DLIB_NAME=SdkLib
117127
//
128+
// Check we didn't emit anything from the cache in the .d file either
129+
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE
130+
//
118131
// RUN: %empty-directory(%t/MCP)
119132
// RUN: echo '4: PASSED'
120133

@@ -156,6 +169,9 @@
156169
// RUN: llvm-bcanalyzer -dump %t/MCP/ExportedLib-*.swiftmodule | %FileCheck %s -check-prefix=PREBUILT
157170
// RUN: llvm-bcanalyzer -dump %t/MCP/SdkLib-*.swiftmodule | %FileCheck %s -check-prefix=PREBUILT
158171
//
172+
// Check we didn't emit anything from the cache in the .d file either
173+
// RUN: cat %t/dummy.d | %FileCheck %s -check-prefix=DEPFILE
174+
//
159175
// RUN: echo '5: PASSED'
160176

161177
import SdkLib

test/ParseableInterface/ModuleCache/module-cache-init.swift

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@
2424
// RUN: test -f %t/modulecache/LeafModule-*.swiftmodule
2525
// RUN: llvm-bcanalyzer -dump %t/modulecache/LeafModule-*.swiftmodule | %FileCheck %s -check-prefix=CHECK-LEAFMODULE
2626
// CHECK-LEAFMODULE: {{MODULE_NAME.*blob data = 'LeafModule'}}
27-
// CHECK-LEAFMODULE: {{FILE_DEPENDENCY.*Swift.swiftmodule([/\\].+[.]swiftmodule)?'}}
2827
// CHECK-LEAFMODULE: {{FILE_DEPENDENCY.*LeafModule.swiftinterface'}}
2928
// CHECK-LEAFMODULE: FUNC_DECL
29+
// RUN: llvm-bcanalyzer -dump %t/modulecache/LeafModule-*.swiftmodule | %FileCheck %s -check-prefix=CHECK-LEAFMODULE-NEGATIVE
30+
// CHECK-LEAFMODULE-NEGATIVE-NOT: {{FILE_DEPENDENCY.*Swift.swiftmodule([/\\].+[.]swiftmodule)?'}}
3031
//
3132
//
3233
// Phase 3: build TestModule into a .swiftmodule explicitly us OtherModule via OtherModule.swiftinterface, creating OtherModule-*.swiftmodule along the way.
@@ -37,7 +38,7 @@
3738
// RUN: test -f %t/TestModule.d
3839
// RUN: llvm-bcanalyzer -dump %t/modulecache/OtherModule-*.swiftmodule | %FileCheck %s -check-prefix=CHECK-OTHERMODULE
3940
// CHECK-OTHERMODULE: {{MODULE_NAME.*blob data = 'OtherModule'}}
40-
// CHECK-OTHERMODULE: {{FILE_DEPENDENCY.*Swift.swiftmodule([/\\].+[.]swiftmodule)?'}}
41+
// CHECK-OTHERMODULE-NOT: {{FILE_DEPENDENCY.*Swift.swiftmodule([/\\].+[.]swiftmodule)?'}}
4142
// CHECK-OTHERMODULE: {{FILE_DEPENDENCY.*LeafModule.swiftinterface'}}
4243
// CHECK-OTHERMODULE: {{FILE_DEPENDENCY.*OtherModule.swiftinterface'}}
4344
// CHECK-OTHERMODULE: FUNC_DECL
@@ -56,10 +57,13 @@
5657
// CHECK-DEPENDS: TestModule.swiftmodule :
5758
// CHECK-DEPENDS-DAG: LeafModule.swiftinterface
5859
// CHECK-DEPENDS-DAG: OtherModule.swiftinterface
59-
// CHECK-DEPENDS-DAG: {{OtherModule-[^ ]+.swiftmodule}}
6060
// CHECK-DEPENDS-DAG: Swift.swiftmodule
6161
// CHECK-DEPENDS-DAG: SwiftOnoneSupport.swiftmodule
62-
// CHECK-DEPENDS-DAG: {{LeafModule-[^ ]+.swiftmodule}}
62+
//
63+
// RUN: %FileCheck %s -check-prefix=CHECK-DEPENDS-NEGATIVE <%t/TestModule.d
64+
//
65+
// CHECK-DEPENDS-NEGATIVE-NOT: {{LeafModule-[^ ]+.swiftmodule}}
66+
// CHECK-DEPENDS-NEGATIVE-NOT: {{OtherModule-[^ ]+.swiftmodule}}
6367

6468
import OtherModule
6569

0 commit comments

Comments
 (0)