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

2017-04-13 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno closed this revision.
bruno added a comment.

Updating: already done in r298175


https://reviews.llvm.org/D29001



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2017-01-22 Thread 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;
+def remark_module_lock_timeout : Remark<
+  "timed out waiting to acquire lock file for module '%0'">, 
InGroup;
 def err_module_cycle : Error<"cyclic dependency in module '%0': %1">, 
   DefaultFatal;
 def err_module_prebuilt : Error<


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,