[PATCH] D29001: [Modules] Fallback to building the module if a timeout occurs

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno created this revision.
Herald added a subscriber: fhahn.

After https://reviews.llvm.org/D28299 gets in, clang will use PCMCache to manage memory buffers for pcm files. This means the lock file manager isn't needed for correctness anymore, but only as an optimization: clang waits for other processes to finish up building a module if they are already doing it.

Emit remark instead of errors to notify users when a timeout occures and actually build the module in case that happens.


https://reviews.llvm.org/D29001

Files:
  include/clang/Basic/DiagnosticCommonKinds.td
  lib/Frontend/CompilerInstance.cpp


Index: lib/Frontend/CompilerInstance.cpp
===================================================================
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -1180,10 +1180,14 @@
     llvm::LockFileManager Locked(ModuleFileName);
     switch (Locked) {
     case llvm::LockFileManager::LFS_Error:
-      Diags.Report(ModuleNameLoc, diag::err_module_lock_failure)
+      // PCMCache takes care of correctness and locks are only necessary for
+      // performance. If there are errors creating the lock, do not use it
+      // and fallback to building the module ourselves.
+      Diags.Report(ModuleNameLoc, diag::remark_module_lock_failure)
           << Module->Name << Locked.getErrorMessage();
-      return false;
-
+      // Clear the lock file in case there's some leftover around.
+      Locked.unsafeRemoveLockFile();
+      // FALLTHROUGH
     case llvm::LockFileManager::LFS_Owned:
       // We're responsible for building the module ourselves.
       if (!compileModuleImpl(ImportingInstance, ModuleNameLoc, Module,
@@ -1203,11 +1207,14 @@
       case llvm::LockFileManager::Res_OwnerDied:
         continue; // try again to get the lock.
       case llvm::LockFileManager::Res_Timeout:
-        Diags.Report(ModuleNameLoc, diag::err_module_lock_timeout)
+        // Since PCMCache takes care of correctness, we try waiting for another
+        // process to complete the build so that this isn't done twice. If we
+        // reach a timeout, it's not a problem, try to build it ourselves then.
+        Diags.Report(ModuleNameLoc, diag::remark_module_lock_timeout)
             << Module->Name;
         // Clear the lock file so that future invokations can make progress.
         Locked.unsafeRemoveLockFile();
-        return false;
+        continue;
       }
       break;
     }
Index: include/clang/Basic/DiagnosticCommonKinds.td
===================================================================
--- include/clang/Basic/DiagnosticCommonKinds.td
+++ include/clang/Basic/DiagnosticCommonKinds.td
@@ -88,10 +88,10 @@
   "module '%0' %select{is incompatible with|requires}1 feature '%2'">;
 def err_module_header_missing : Error<
   "%select{|umbrella }0header '%1' not found">;
-def err_module_lock_failure : Error<
-  "could not acquire lock file for module '%0': %1">, DefaultFatal;
-def err_module_lock_timeout : Error<
-  "timed out waiting to acquire lock file for module '%0'">, DefaultFatal;
+def remark_module_lock_failure : Remark<
+  "could not acquire lock file for module '%0': %1">, InGroup<ModuleBuild>;
+def remark_module_lock_timeout : Remark<
+  "timed out waiting to acquire lock file for module '%0'">, InGroup<ModuleBuild>;
 def err_module_cycle : Error<"cyclic dependency in module '%0': %1">, 
   DefaultFatal;
 def err_module_prebuilt : Error<


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29001.85312.patch
Type: text/x-patch
Size: 2818 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170123/545d43f0/attachment.bin>