Revert "ModuleInterface: lock .swiftinterface while generating module cache"

Author: shahmishal

include/swift/AST/DiagnosticsFrontend.def

@@ -343,12 +343,6 @@ ERROR(unknown_forced_module_loading_mode,none,
       "unknown value for SWIFT_FORCE_MODULE_LOADING variable: '%0'",
       (StringRef))
 
-REMARK(interface_file_lock_failure,none,
-      "could not acquire lock file for module interface '%0'", (StringRef))
-
-REMARK(interface_file_lock_timed_out,none,
-      "timed out waiting to acquire lock file for module interface '%0'", (StringRef))
-
 #ifndef DIAG_NO_UNDEF
 # if defined(DIAG)
 #  undef DIAG

lib/Frontend/ModuleInterfaceBuilder.cpp

@@ -36,7 +36,6 @@
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/StringSaver.h"
-#include "llvm/Support/LockFileManager.h"
 
 using namespace swift;
 using FileDependency = SerializationOptions::FileDependency;
@@ -241,7 +240,7 @@ bool ModuleInterfaceBuilder::collectDepsForSerialization(
   return false;
 }
 
-bool ModuleInterfaceBuilder::buildSwiftModuleInternal(
+bool ModuleInterfaceBuilder::buildSwiftModule(
     StringRef OutPath, bool ShouldSerializeDeps,
     std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer) {
   bool SubError = false;
@@ -385,70 +384,3 @@ bool ModuleInterfaceBuilder::buildSwiftModuleInternal(
   });
   return !RunSuccess || SubError;
 }
-
-bool ModuleInterfaceBuilder::buildSwiftModule(StringRef OutPath,
-                                              bool ShouldSerializeDeps,
-                          std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer,
-                          llvm::function_ref<void()> RemarkRebuild) {
-
-  while (1) {
-  // Attempt to lock the interface file. Only one process is allowed to build
-  // module from the interface so we don't consume too much memory when multiple
-  // processes are doing the same.
-  // FIXME: We should surface the module building step to the build system so
-  // we don't need to synchronize here.
-  llvm::LockFileManager Locked(interfacePath);
-  switch (Locked) {
-  case llvm::LockFileManager::LFS_Error:{
-    // ModuleInterfaceBuilder takes care of correctness and locks are only
-    // necessary for performance. Fallback to building the module in case of any lock
-    // related errors.
-    if (RemarkRebuild) {
-      diags.diagnose(SourceLoc(), diag::interface_file_lock_failure,
-                     interfacePath);
-    }
-    // Clear out any potential leftover.
-    Locked.unsafeRemoveLockFile();
-    LLVM_FALLTHROUGH;
-  }
-  case llvm::LockFileManager::LFS_Owned: {
-    if (RemarkRebuild) {
-      RemarkRebuild();
-    }
-    return buildSwiftModuleInternal(OutPath, ShouldSerializeDeps, ModuleBuffer);
-  }
-  case llvm::LockFileManager::LFS_Shared: {
-    // Someone else is responsible for building the module. Wait for them to
-    // finish.
-    switch (Locked.waitForUnlock()) {
-    case llvm::LockFileManager::Res_Success: {
-      // This process may have a different module output path. If the other
-      // process doesn't build the interface to this output path, we should try
-      // building ourselves.
-      auto bufferOrError = llvm::MemoryBuffer::getFile(OutPath);
-      if (!bufferOrError)
-        continue;
-      *ModuleBuffer = std::move(bufferOrError.get());
-      return false;
-    }
-    case llvm::LockFileManager::Res_OwnerDied: {
-      continue; // try again to get the lock.
-    }
-    case llvm::LockFileManager::Res_Timeout: {
-      // Since ModuleInterfaceBuilder takes care of correctness, we try waiting for
-      // another process to complete the build so swift does not do it done
-      // twice. If case of timeout, build it ourselves.
-      if (RemarkRebuild) {
-        diags.diagnose(SourceLoc(), diag::interface_file_lock_timed_out,
-                       interfacePath);
-      }
-      // Clear the lock file so that future invocations can make progress.
-      Locked.unsafeRemoveLockFile();
-      continue;
-    }
-    }
-    break;
-  }
-  }
-  }
-}

lib/Frontend/ModuleInterfaceBuilder.h

@@ -67,8 +67,6 @@ class ModuleInterfaceBuilder {
       version::Version &Vers, llvm::StringSaver &SubArgSaver,
       SmallVectorImpl<const char *> &SubArgs);
 
-  bool buildSwiftModuleInternal(StringRef OutPath, bool ShouldSerializeDeps,
-                                std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer);
 public:
   ModuleInterfaceBuilder(SourceManager &sourceMgr, DiagnosticEngine &diags,
                             const SearchPathOptions &searchPathOpts,
@@ -104,8 +102,7 @@ class ModuleInterfaceBuilder {
   }
 
   bool buildSwiftModule(StringRef OutPath, bool ShouldSerializeDeps,
-                        std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer,
-                        llvm::function_ref<void()> RemarkRebuild = nullptr);
+                        std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer);
 };
 
 } // end namespace swift

lib/Frontend/ModuleInterfaceLoader.cpp

@@ -950,11 +950,13 @@ class ModuleInterfaceLoaderImpl {
     std::unique_ptr<llvm::MemoryBuffer> moduleBuffer;
 
     // We didn't discover a module corresponding to this interface.
+
     // Diagnose that we didn't find a loadable module, if we were asked to.
-    auto remarkRebuild = [&]() {
+    if (remarkOnRebuildFromInterface) {
       rebuildInfo.diagnose(ctx, diagnosticLoc, moduleName,
                            interfacePath);
-    };
+    }
+
     // If we found an out-of-date .swiftmodule, we still want to add it as
     // a dependency of the .swiftinterface. That way if it's updated, but
     // the .swiftinterface remains the same, we invalidate the cache and
@@ -964,9 +966,7 @@ class ModuleInterfaceLoaderImpl {
       builder.addExtraDependency(modulePath);
 
     if (builder.buildSwiftModule(cachedOutputPath, /*shouldSerializeDeps*/true,
-                                 &moduleBuffer,
-                                 remarkOnRebuildFromInterface ? remarkRebuild:
-                                   llvm::function_ref<void()>()))
+                                 &moduleBuffer))
       return std::make_error_code(std::errc::invalid_argument);
 
     assert(moduleBuffer &&