Cut down the number of open/close system calls for output files.

For PCH files, have only one open/close for temporary + rename to be safe from race conditions.
For all other output files open/close the output file directly.

Depends on llvm r136310. rdar://9082880 & http://llvm.org/PR9374.

llvm-svn: 136315

clang/include/clang/Frontend/CompilerInstance.h

@@ -570,14 +570,16 @@ public:
   createOutputFile(StringRef OutputPath,
                    bool Binary = true, bool RemoveFileOnSignal = true,
                    StringRef BaseInput = "",
-                   StringRef Extension = "");
+                   StringRef Extension = "",
+                   bool UseTemporary = false);
 
   /// Create a new output file, optionally deriving the output path name.
   ///
   /// If \arg OutputPath is empty, then createOutputFile will derive an output
   /// path location as \arg BaseInput, with any suffix removed, and \arg
-  /// Extension appended. If OutputPath is not stdout createOutputFile will
-  /// create a new temporary file that must be renamed to OutputPath in the end.
+  /// Extension appended. If OutputPath is not stdout and \arg UseTemporary
+  /// is true, createOutputFile will create a new temporary file that must be
+  /// renamed to OutputPath in the end.
   ///
   /// \param OutputPath - If given, the path to the output file.
   /// \param Error [out] - On failure, the error message.
@@ -588,6 +590,8 @@ public:
   /// \param RemoveFileOnSignal - Whether the file should be registered with
   /// llvm::sys::RemoveFileOnSignal. Note that this is not safe for
   /// multithreaded use, as the underlying signal mechanism is not reentrant
+  /// \param UseTemporary - Create a new temporary file that must be renamed to
+  ///         OutputPath in the end
   /// \param ResultPathName [out] - If given, the result path name will be
   /// stored here on success.
   /// \param TempPathName [out] - If given, the temporary file path name
@@ -597,6 +601,7 @@ public:
                    bool Binary = true, bool RemoveFileOnSignal = true,
                    StringRef BaseInput = "",
                    StringRef Extension = "",
+                   bool UseTemporary = false,
                    std::string *ResultPathName = 0,
                    std::string *TempPathName = 0);
 

clang/lib/Frontend/ASTUnit.cpp

@@ -2309,23 +2309,17 @@ CXSaveError ASTUnit::Save(StringRef File) {
 
   // Write to a temporary file and later rename it to the actual file, to avoid
   // possible race conditions.
-  llvm::sys::Path TempPath(File);
-  if (TempPath.makeUnique(/*reuse_current=*/false, /*ErrMsg*/0))
+  llvm::SmallString<128> TempPath;
+  TempPath = File;
+  TempPath += "-%%%%%%%%";
+  int fd;
+  if (llvm::sys::fs::unique_file(TempPath.str(), fd, TempPath,
+                                 /*makeAbsolute=*/false))
     return CXSaveError_Unknown;
-  // makeUnique may or may not have created the file. Try deleting before
-  // opening so that we can use F_Excl for exclusive access.
-  TempPath.eraseFromDisk();
 
   // FIXME: Can we somehow regenerate the stat cache here, or do we need to 
   // unconditionally create a stat cache when we parse the file?
-  std::string ErrorInfo;
-  llvm::raw_fd_ostream Out(TempPath.c_str(), ErrorInfo,
-                           llvm::raw_fd_ostream::F_Binary |
-                           // if TempPath already exists, we should not try to
-                           // overwrite it, we want to avoid race conditions.
-                           llvm::raw_fd_ostream::F_Excl);
-  if (!ErrorInfo.empty() || Out.has_error())
-    return CXSaveError_Unknown;
+  llvm::raw_fd_ostream Out(fd, /*shouldClose=*/true);
 
   serialize(Out);
   Out.close();

clang/lib/Frontend/CompilerInstance.cpp

@@ -437,11 +437,13 @@ llvm::raw_fd_ostream *
 CompilerInstance::createOutputFile(StringRef OutputPath,
                                    bool Binary, bool RemoveFileOnSignal,
                                    StringRef InFile,
-                                   StringRef Extension) {
+                                   StringRef Extension,
+                                   bool UseTemporary) {
   std::string Error, OutputPathName, TempPathName;
   llvm::raw_fd_ostream *OS = createOutputFile(OutputPath, Error, Binary,
                                               RemoveFileOnSignal,
                                               InFile, Extension,
+                                              UseTemporary,
                                               &OutputPathName,
                                               &TempPathName);
   if (!OS) {
@@ -465,6 +467,7 @@ CompilerInstance::createOutputFile(StringRef OutputPath,
                                    bool RemoveFileOnSignal,
                                    StringRef InFile,
                                    StringRef Extension,
+                                   bool UseTemporary,
                                    std::string *ResultPathName,
                                    std::string *TempPathName) {
   std::string OutFile, TempFile;
@@ -480,8 +483,11 @@ CompilerInstance::createOutputFile(StringRef OutputPath,
   } else {
     OutFile = "-";
   }
-  
-  if (OutFile != "-") {
+
+  llvm::OwningPtr<llvm::raw_fd_ostream> OS;
+  std::string OSFile;
+
+  if (UseTemporary && OutFile != "-") {
     llvm::sys::Path OutPath(OutFile);
     // Only create the temporary if we can actually write to OutPath, otherwise
     // we want to fail early.
@@ -489,21 +495,26 @@ CompilerInstance::createOutputFile(StringRef OutputPath,
     if ((llvm::sys::fs::exists(OutPath.str(), Exists) || !Exists) ||
         (OutPath.isRegularFile() && OutPath.canWrite())) {
       // Create a temporary file.
-      llvm::sys::Path TempPath(OutFile);
-      if (!TempPath.makeUnique(/*reuse_current=*/false, /*ErrMsg*/0))
-        TempFile = TempPath.str();
+      llvm::SmallString<128> TempPath;
+      TempPath = OutFile;
+      TempPath += "-%%%%%%%%";
+      int fd;
+      if (llvm::sys::fs::unique_file(TempPath.str(), fd, TempPath,
+                               /*makeAbsolute=*/false) == llvm::errc::success) {
+        OS.reset(new llvm::raw_fd_ostream(fd, /*shouldClose=*/true));
+        OSFile = TempFile = TempPath.str();
+      }
     }
   }
 
-  std::string OSFile = OutFile;
-  if (!TempFile.empty())
-    OSFile = TempFile;
-
-  llvm::OwningPtr<llvm::raw_fd_ostream> OS(
-    new llvm::raw_fd_ostream(OSFile.c_str(), Error,
-                             (Binary ? llvm::raw_fd_ostream::F_Binary : 0)));
-  if (!Error.empty())
-    return 0;
+  if (!OS) {
+    OSFile = OutFile;
+    OS.reset(
+      new llvm::raw_fd_ostream(OSFile.c_str(), Error,
+                               (Binary ? llvm::raw_fd_ostream::F_Binary : 0)));
+    if (!Error.empty())
+      return 0;
+  }
 
   // Make sure the out stream file gets removed if we crash.
   if (RemoveFileOnSignal)

clang/lib/Frontend/FrontendActions.cpp

@@ -102,8 +102,10 @@ bool GeneratePCHAction::ComputeASTConsumerArguments(CompilerInstance &CI,
 
   // We use createOutputFile here because this is exposed via libclang, and we
   // must disable the RemoveFileOnSignal behavior.
+  // We use a temporary to avoid race conditions.
   OS = CI.createOutputFile(CI.getFrontendOpts().OutputFile, /*Binary=*/true,
-                           /*RemoveFileOnSignal=*/false, InFile);
+                           /*RemoveFileOnSignal=*/false, InFile,
+                           /*Extension=*/"", /*useTemporary=*/true);
   if (!OS)
     return true;