Skip to content

Commit 3732802

Browse files
author
Nathan Hawes
committed
[ParseableInterface] Don't serialize swiftmodule dependencies in the module cache or prebuilt module cache
*Their* dependencies are already being serialized out, so this shouldn't affect up-to-date-checking except by alowing the regular and prebuilt module caches to be relocated without invalidating their contents. In the case of the prebuilt module cache, this gets us closer to supporting relocation across machines.
1 parent 58d0ee0 commit 3732802

File tree

3 files changed

+73
-69
lines changed

3 files changed

+73
-69
lines changed

‎lib/Frontend/ParseableInterfaceModuleLoader.cpp‎

Lines changed: 68 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -356,48 +356,39 @@ class swift::ParseableInterfaceBuilder {
356356
return false;
357357
}
358358

359-
/// Populate the provided \p Deps with \c FileDependency entries including:
360-
///
361-
/// - All the dependencies mentioned by \p SubInstance's DependencyTracker,
362-
/// that were read while compiling the module.
363-
///
364-
/// - For any file in the latter set that is itself a .swiftmodule
365-
/// living in \p cachePath, all of _its_ dependencies, copied
366-
/// out to avoid having to do recursive scanning when rechecking this
367-
/// dependency in the future.
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+
if (moduleCachePath.empty() && prebuiltCachePath.empty())
363+
return false;
364+
365+
auto Ext = llvm::sys::path::extension(DepName);
366+
auto Ty = file_types::lookupTypeForExtension(Ext);
367+
368+
if (Ty != file_types::TY_SwiftModuleFile)
369+
return false;
370+
if (!moduleCachePath.empty() && DepName.startswith(moduleCachePath))
371+
return true;
372+
return !prebuiltCachePath.empty() && DepName.startswith(prebuiltCachePath);
373+
}
374+
375+
/// Populate the provided \p Deps with \c FileDependency entries for all
376+
/// dependencies \p SubInstance's DependencyTracker recorded while compiling
377+
/// the module, excepting .swiftmodules in \p moduleCachePath or
378+
/// \p prebuiltCachePath. Those have _their_ dependencies added instead, both
379+
/// to avoid having to do recursive scanning when rechecking this dependency
380+
/// in future and to make the module caches relocatable.
368381
bool collectDepsForSerialization(CompilerInstance &SubInstance,
369382
SmallVectorImpl<FileDependency> &Deps,
370383
bool IsHashBased) {
371384
StringRef SDKPath = SubInstance.getASTContext().SearchPathOpts.SDKPath;
372-
373385
auto DTDeps = SubInstance.getDependencyTracker()->getDependencies();
374386
SmallVector<StringRef, 16> InitialDepNames(DTDeps.begin(), DTDeps.end());
375387
InitialDepNames.push_back(interfacePath);
376388
llvm::StringSet<> AllDepNames;
377-
for (auto const &DepName : InitialDepNames) {
378-
if (AllDepNames.insert(DepName).second && dependencyTracker) {
379-
dependencyTracker->addDependency(DepName, /*isSystem*/false);
380-
}
381-
382-
/// Lazily load the dependency buffer if we need it. If we're not
383-
/// dealing with a hash-based dependencies, and if the dependency is
384-
/// not a .swiftmodule, we can avoid opening the buffer.
385-
std::unique_ptr<llvm::MemoryBuffer> DepBuf = nullptr;
386-
auto getDepBuf = [&]() -> llvm::MemoryBuffer * {
387-
if (DepBuf) return DepBuf.get();
388-
if (auto Buf = getBufferOfDependency(fs, DepName, interfacePath,
389-
diags, diagnosticLoc)) {
390-
DepBuf = std::move(Buf);
391-
return DepBuf.get();
392-
}
393-
return nullptr;
394-
};
395-
396-
auto Status = getStatusOfDependency(fs, DepName, interfacePath,
397-
diags, diagnosticLoc);
398-
if (!Status)
399-
return true;
400389

390+
for (auto const &DepName : InitialDepNames) {
391+
// Adjust the paths of dependences in the SDK to be relative to it
401392
bool IsSDKRelative = false;
402393
StringRef DepNameToStore = DepName;
403394
if (SDKPath.size() > 1 && DepName.startswith(SDKPath)) {
@@ -418,32 +409,29 @@ class swift::ParseableInterfaceBuilder {
418409
}
419410
}
420411

421-
if (IsHashBased) {
422-
auto buf = getDepBuf();
423-
if (!buf) return true;
424-
uint64_t hash = xxHash64(buf->getBuffer());
425-
Deps.push_back(
426-
FileDependency::hashBased(DepNameToStore, IsSDKRelative,
427-
Status->getSize(), hash));
428-
} else {
429-
uint64_t mtime =
430-
Status->getLastModificationTime().time_since_epoch().count();
431-
Deps.push_back(
432-
FileDependency::modTimeBased(DepNameToStore, IsSDKRelative,
433-
Status->getSize(), mtime));
412+
if (AllDepNames.insert(DepName).second && dependencyTracker) {
413+
dependencyTracker->addDependency(DepName, /*isSystem*/IsSDKRelative);
434414
}
435415

436-
if (moduleCachePath.empty())
437-
continue;
416+
/// Lazily load the dependency buffer if we need it. If we're not
417+
/// dealing with a hash-based dependencies, and if the dependency is
418+
/// not a .swiftmodule, we can avoid opening the buffer.
419+
std::unique_ptr<llvm::MemoryBuffer> DepBuf = nullptr;
420+
auto getDepBuf = [&]() -> llvm::MemoryBuffer * {
421+
if (DepBuf) return DepBuf.get();
422+
if (auto Buf = getBufferOfDependency(fs, DepName, interfacePath,
423+
diags, diagnosticLoc)) {
424+
DepBuf = std::move(Buf);
425+
return DepBuf.get();
426+
}
427+
return nullptr;
428+
};
438429

439-
// If Dep is itself a .swiftmodule in the cache dir, pull out its deps
440-
// and include them in our own, so we have a single-file view of
441-
// transitive deps: removes redundancies, and avoids opening and reading
442-
// multiple swiftmodules during future loads.
443-
auto Ext = llvm::sys::path::extension(DepName);
444-
auto Ty = file_types::lookupTypeForExtension(Ext);
445-
if (Ty == file_types::TY_SwiftModuleFile &&
446-
DepName.startswith(moduleCachePath)) {
430+
// If Dep is itself a cached .swiftmodule, pull out its deps and include
431+
// them in our own, so we have a single-file view of transitive deps:
432+
// removes redundancies, makes the cache more relocatable, and avoids
433+
// opening and reading multiple swiftmodules during future loads.
434+
if (isCachedModule(DepName)) {
447435
auto buf = getDepBuf();
448436
if (!buf) return true;
449437
SmallVector<FileDependency, 16> SubDeps;
@@ -459,10 +447,32 @@ class swift::ParseableInterfaceBuilder {
459447
if (AllDepNames.insert(SubDep.getPath()).second) {
460448
Deps.push_back(SubDep);
461449
if (dependencyTracker)
462-
dependencyTracker->addDependency(SubDep.getPath(),
463-
/*IsSystem=*/false);
450+
dependencyTracker->addDependency(
451+
SubDep.getPath(), /*IsSystem=*/SubDep.isSDKRelative());
464452
}
465453
}
454+
continue;
455+
}
456+
457+
// Otherwise, include this dependency directly
458+
auto Status = getStatusOfDependency(fs, DepName, interfacePath,
459+
diags, diagnosticLoc);
460+
if (!Status)
461+
return true;
462+
463+
if (IsHashBased) {
464+
auto buf = getDepBuf();
465+
if (!buf) return true;
466+
uint64_t hash = xxHash64(buf->getBuffer());
467+
Deps.push_back(
468+
FileDependency::hashBased(DepNameToStore, IsSDKRelative,
469+
Status->getSize(), hash));
470+
} else {
471+
uint64_t mtime =
472+
Status->getLastModificationTime().time_since_epoch().count();
473+
Deps.push_back(
474+
FileDependency::modTimeBased(DepNameToStore, IsSDKRelative,
475+
Status->getSize(), mtime));
466476
}
467477
}
468478
return false;

test/ParseableInterface/ModuleCache/SDKDependencies.swiftinterface renamed to test/ParseableInterface/ModuleCache/SDKDependencies.swift

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
// swift-interface-format-version: 1.0
2-
// swift-module-flags: -module-name SDKDependencies
3-
41
// RUN: %empty-directory(%t)
52

63
// 1) Build a prebuilt cache for our SDK
@@ -46,11 +43,10 @@
4643
// RUN: cat %t/MCP/ExportedLib-*.swiftmodule | %FileCheck %s -check-prefix=EXLIB
4744
// RUN: cat %t/MCP/SdkLib-*.swiftmodule | %FileCheck %s -check-prefixes=EXLIB,SDKLIB
4845
//
49-
// EXLIB-DAG: /prebuilt-cache/ExportedLib.swiftmodule
46+
// EXLIB: dependencies:
5047
// EXLIB-DAG: /my-sdk/usr/include/module.modulemap
5148
// EXLIB-DAG: /my-sdk/usr/include/SomeCModule.h
5249
// EXLIB-DAG: /my-sdk/ExportedLib.swiftinterface
53-
// SDKLIB-DAG: /prebuilt-cache/SdkLib.swiftmodule
5450
// SDKLIB-DAG: /my-sdk/SdkLib.swiftinterface
5551
//
5652
// RUN: %empty-directory(%t/MCP)
@@ -60,7 +56,8 @@
6056
// 4) Move the SDK without changing its contents
6157
//
6258
// RUN: mv %t/my-sdk %t/my-new-sdk
63-
// RUN: %target-swift-frontend -typecheck -I %t/my-new-sdk -sdk %t/my-new-sdk -prebuilt-module-cache-path %t/prebuilt-cache -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies %s
59+
// RUN: mv %t/prebuilt-cache %t/new-prebuilt-cache
60+
// RUN: %target-swift-frontend -typecheck -I %t/my-new-sdk -sdk %t/my-new-sdk -prebuilt-module-cache-path %t/new-prebuilt-cache -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies %s
6461
//
6562
// Check SdkLib and ExportedLib are in the module cache
6663
// RUN: test -f %t/MCP/SdkLib-*.swiftmodule
@@ -74,11 +71,9 @@
7471
// RUN: cat %t/MCP/ExportedLib-*.swiftmodule | %FileCheck %s -check-prefix=NEW-EXLIB
7572
// RUN: cat %t/MCP/SdkLib-*.swiftmodule | %FileCheck %s -check-prefixes=NEW-EXLIB,NEW-SDKLIB
7673
//
77-
// NEW-EXLIB-DAG: /prebuilt-cache/ExportedLib.swiftmodule
7874
// NEW-EXLIB-DAG: /my-new-sdk/usr/include/module.modulemap
7975
// NEW-EXLIB-DAG: /my-new-sdk/usr/include/SomeCModule.h
8076
// NEW-EXLIB-DAG: /my-new-sdk/ExportedLib.swiftinterface
81-
// NEW-SDKLIB-DAG: /prebuilt-cache/SdkLib.swiftmodule
8277
// NEW-SDKLIB-DAG: /my-new-sdk/SdkLib.swiftinterface
8378
//
8479
// RUN: %empty-directory(%t/MCP)
@@ -88,7 +83,7 @@
8883
// 5) Now change the SDK's content and check it no longer uses the prebuilt modules
8984
//
9085
// RUN: echo "// size change" >> %t/my-new-sdk/SdkLib.swiftinterface
91-
// RUN: %target-swift-frontend -typecheck -I %t/my-new-sdk -sdk %t/my-new-sdk -prebuilt-module-cache-path %t/prebuilt-cache -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies %s
86+
// RUN: %target-swift-frontend -typecheck -I %t/my-new-sdk -sdk %t/my-new-sdk -prebuilt-module-cache-path %t/new-prebuilt-cache -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies %s
9287
//
9388
// Check SDKLib and ExportedLib are in the module cache
9489
// RUN: test -f %t/MCP/SdkLib-*.swiftmodule
@@ -104,7 +99,7 @@
10499
// RUN: %empty-directory(%t/MCP)
105100
//
106101
// RUN: echo "// size change" >> %t/my-new-sdk/usr/include/SomeCModule.h
107-
// RUN: %target-swift-frontend -typecheck -I %t/my-new-sdk -sdk %t/my-new-sdk -prebuilt-module-cache-path %t/prebuilt-cache -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies %s
102+
// RUN: %target-swift-frontend -typecheck -I %t/my-new-sdk -sdk %t/my-new-sdk -prebuilt-module-cache-path %t/new-prebuilt-cache -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies %s
108103
//
109104
// Check SDKLib and ExportLib are in the module cache
110105
// RUN: test -f %t/MCP/SdkLib-*.swiftmodule

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
// CHECK-OTHERMODULE: {{MODULE_NAME.*blob data = 'OtherModule'}}
4040
// CHECK-OTHERMODULE: {{FILE_DEPENDENCY.*Swift.swiftmodule([/\\].+[.]swiftmodule)?'}}
4141
// CHECK-OTHERMODULE: {{FILE_DEPENDENCY.*LeafModule.swiftinterface'}}
42-
// CHECK-OTHERMODULE: {{FILE_DEPENDENCY.*LeafModule-.*.swiftmodule'}}
4342
// CHECK-OTHERMODULE: {{FILE_DEPENDENCY.*OtherModule.swiftinterface'}}
4443
// CHECK-OTHERMODULE: FUNC_DECL
4544
//

0 commit comments

Comments
 (0)