Skip to content

Commit 8dda019

Browse files
committed
ModuleInterface: lock .swiftinterface while generating module cache
This ensures only one process is generating module cache from an interface file so that we don't blow up memory usage when multiple processes are doing the same. The locking mechanism is similar to that of Clang's. A better approach is that the build system takes care of the module building step as a formal dependency. rdar://52839445
1 parent 93d07c6 commit 8dda019

File tree

5 files changed

+97
-7
lines changed

5 files changed

+97
-7
lines changed

include/swift/AST/DiagnosticsFrontend.def

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,12 @@ ERROR(unknown_forced_module_loading_mode,none,
343343
"unknown value for SWIFT_FORCE_MODULE_LOADING variable: '%0'",
344344
(StringRef))
345345

346+
REMARK(interface_file_lock_failure,none,
347+
"could not acquire lock file for module interface '%0'", (StringRef))
348+
349+
REMARK(interface_file_lock_timed_out,none,
350+
"timed out waiting to acquire lock file for module interface '%0'", (StringRef))
351+
346352
#ifndef DIAG_NO_UNDEF
347353
# if defined(DIAG)
348354
# undef DIAG

lib/Frontend/ModuleInterfaceBuilder.cpp

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "llvm/Support/Errc.h"
3737
#include "llvm/Support/Regex.h"
3838
#include "llvm/Support/StringSaver.h"
39+
#include "llvm/Support/LockFileManager.h"
3940

4041
using namespace swift;
4142
using FileDependency = SerializationOptions::FileDependency;
@@ -240,7 +241,7 @@ bool ModuleInterfaceBuilder::collectDepsForSerialization(
240241
return false;
241242
}
242243

243-
bool ModuleInterfaceBuilder::buildSwiftModule(
244+
bool ModuleInterfaceBuilder::buildSwiftModuleInternal(
244245
StringRef OutPath, bool ShouldSerializeDeps,
245246
std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer) {
246247
bool SubError = false;
@@ -382,3 +383,70 @@ bool ModuleInterfaceBuilder::buildSwiftModule(
382383
});
383384
return !RunSuccess || SubError;
384385
}
386+
387+
bool ModuleInterfaceBuilder::buildSwiftModule(StringRef OutPath,
388+
bool ShouldSerializeDeps,
389+
std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer,
390+
llvm::function_ref<void()> RemarkRebuild) {
391+
392+
while (1) {
393+
// Attempt to lock the interface file. Only one process is allowed to build
394+
// module from the interface so we don't consume too much memory when multiple
395+
// processes are doing the same.
396+
// FIXME: We should surface the module building step to the build system so
397+
// we don't need to synchronize here.
398+
llvm::LockFileManager Locked(interfacePath);
399+
switch (Locked) {
400+
case llvm::LockFileManager::LFS_Error:{
401+
// ModuleInterfaceBuilder takes care of correctness and locks are only
402+
// necessary for performance. Fallback to building the module in case of any lock
403+
// related errors.
404+
if (RemarkRebuild) {
405+
diags.diagnose(SourceLoc(), diag::interface_file_lock_failure,
406+
interfacePath);
407+
}
408+
// Clear out any potential leftover.
409+
Locked.unsafeRemoveLockFile();
410+
LLVM_FALLTHROUGH;
411+
}
412+
case llvm::LockFileManager::LFS_Owned: {
413+
if (RemarkRebuild) {
414+
RemarkRebuild();
415+
}
416+
return buildSwiftModuleInternal(OutPath, ShouldSerializeDeps, ModuleBuffer);
417+
}
418+
case llvm::LockFileManager::LFS_Shared: {
419+
// Someone else is responsible for building the module. Wait for them to
420+
// finish.
421+
switch (Locked.waitForUnlock()) {
422+
case llvm::LockFileManager::Res_Success: {
423+
// This process may have a different module output path. If the other
424+
// process doesn't build the interface to this output path, we should try
425+
// building ourselves.
426+
auto bufferOrError = llvm::MemoryBuffer::getFile(OutPath);
427+
if (!bufferOrError)
428+
continue;
429+
*ModuleBuffer = std::move(bufferOrError.get());
430+
return false;
431+
}
432+
case llvm::LockFileManager::Res_OwnerDied: {
433+
continue; // try again to get the lock.
434+
}
435+
case llvm::LockFileManager::Res_Timeout: {
436+
// Since ModuleInterfaceBuilder takes care of correctness, we try waiting for
437+
// another process to complete the build so swift does not do it done
438+
// twice. If case of timeout, build it ourselves.
439+
if (RemarkRebuild) {
440+
diags.diagnose(SourceLoc(), diag::interface_file_lock_timed_out,
441+
interfacePath);
442+
}
443+
// Clear the lock file so that future invocations can make progress.
444+
Locked.unsafeRemoveLockFile();
445+
continue;
446+
}
447+
}
448+
break;
449+
}
450+
}
451+
}
452+
}

lib/Frontend/ModuleInterfaceBuilder.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ class ModuleInterfaceBuilder {
6767
version::Version &Vers, llvm::StringSaver &SubArgSaver,
6868
SmallVectorImpl<const char *> &SubArgs);
6969

70+
bool buildSwiftModuleInternal(StringRef OutPath, bool ShouldSerializeDeps,
71+
std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer);
7072
public:
7173
ModuleInterfaceBuilder(SourceManager &sourceMgr, DiagnosticEngine &diags,
7274
const SearchPathOptions &searchPathOpts,
@@ -102,7 +104,8 @@ class ModuleInterfaceBuilder {
102104
}
103105

104106
bool buildSwiftModule(StringRef OutPath, bool ShouldSerializeDeps,
105-
std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer);
107+
std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer,
108+
llvm::function_ref<void()> RemarkRebuild = nullptr);
106109
};
107110

108111
} // end namespace swift

lib/Frontend/ModuleInterfaceLoader.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -950,13 +950,11 @@ class ModuleInterfaceLoaderImpl {
950950
std::unique_ptr<llvm::MemoryBuffer> moduleBuffer;
951951

952952
// We didn't discover a module corresponding to this interface.
953-
954953
// Diagnose that we didn't find a loadable module, if we were asked to.
955-
if (remarkOnRebuildFromInterface) {
954+
auto remarkRebuild = [&]() {
956955
rebuildInfo.diagnose(ctx, diagnosticLoc, moduleName,
957956
interfacePath);
958-
}
959-
957+
};
960958
// If we found an out-of-date .swiftmodule, we still want to add it as
961959
// a dependency of the .swiftinterface. That way if it's updated, but
962960
// the .swiftinterface remains the same, we invalidate the cache and
@@ -966,7 +964,9 @@ class ModuleInterfaceLoaderImpl {
966964
builder.addExtraDependency(modulePath);
967965

968966
if (builder.buildSwiftModule(cachedOutputPath, /*shouldSerializeDeps*/true,
969-
&moduleBuffer))
967+
&moduleBuffer,
968+
remarkOnRebuildFromInterface ? remarkRebuild:
969+
llvm::function_ref<void()>()))
970970
return std::make_error_code(std::errc::invalid_argument);
971971

972972
assert(moduleBuffer &&

test/Driver/lock_interface.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: echo 'public func foo() {}' > %t/Foo.swift
3+
// RUN: %target-swift-frontend-typecheck -emit-module-interface-path %t/Foo.swiftinterface %t/Foo.swift -enable-library-evolution
4+
// RUN: touch %t/main.swift %t/file-01.swift %t/file-02.swift %t/file-03.swift
5+
// RUN: echo 'import Foo' > %t/file-01.swift
6+
// RUN: echo 'import Foo' > %t/file-02.swift
7+
// RUN: echo 'import Foo' > %t/file-03.swift
8+
// RUN: %swiftc_driver -j20 %t/main.swift %t/file-01.swift %t/file-02.swift %t/file-03.swift -I %t -Xfrontend -Rmodule-interface-rebuild &> %t/result.txt
9+
// RUN: %FileCheck %s -check-prefix=CHECK-REBUILD < %t/result.txt
10+
11+
// Ensure we only build Foo module once from the interface
12+
// CHECK-REBUILD: rebuilding module 'Foo' from interface
13+
// CHECK-REBUILD-NOT: rebuilding module 'Foo' from interface

0 commit comments

Comments
 (0)