Simplify unlinkAsync

[• rafael]

Summary

I started to take a look at our use of temporary files, with the eventual goal of never leaving a temporary file in case of crash.

The first thing I am looking at is where we use the version of createUniqueFile that doesn't open the file. In general that can race. In lld I think there is only one use and it is simple to avoid.

It is used to create a temp name to pass to rename, but there are simpler ways of keeping a reference to a file than renaming it. In fact, just opening the file will do it.

I tested that the close is indeed where the time is spent by wrapping both in std::chrono::system_clock::now. The remove timed at about 5.065000e-06 seconds, the close at 3.852581e-01 seconds.

[ruiu]

ELF/Filesystem.cpp

20

Does all system have unistd.h?

61–65

Is this safe on Windows? On Windows, you can't remove opened files.

[ruiu]

Do you want to replace runBackground with std::thread([=] { ::close(FD) }).detach()?

[rnk]

Check out https://bugs.llvm.org/show_bug.cgi?id=34070, I wanted to push as much of this down into FileOutputBuffer and ToolOutputFile as possible and reduce our reliance on RemoveFileOnSignal, but I'm not actively working on it.

[ruiu]

LGTM

ELF/Filesystem.cpp

47

I think it is better to replace this with #else to avoid a possible "unreachable code after return" warning on Windows.

[ruiu]

Wait, I think we should remove a given file on Windows as well to make this function's behavior consistent on all platforms.

[• rafael]

Also unlink on windows.

[ruiu]

Please go ahead and submit.

[mati865]

ELF/Filesystem.cpp

17

It broke lld stand-alone builds:

../ELF/Filesystem.cpp:17:32: fatal error: llvm/Config/config.h: No such file or directory
 #include "llvm/Config/config.h"
                                ^
compilation terminated.

[• espindola]

317631

ELF/Filesystem.cpp

 //===- Filesystem.cpp -----------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains a few utility functions to handle files.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Filesystem.h"
 #include "Config.h"
 #include "lld/Common/Threads.h"
+#include "llvm/Config/config.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/FileSystem.h"
+#if defined(HAVE_UNISTD_H)
+#include <unistd.h>
+#endif
 
 using namespace llvm;
 
 using namespace lld;
 using namespace lld::elf;
 
 // Removes a given file asynchronously. This is a performance hack,
 // so remove this when operating systems are improved.
 //
 // On Linux (and probably on other Unix-like systems), unlink(2) is a
 // noticeably slow system call. As of 2016, unlink takes 250
 // milliseconds to remove a 1 GB file on ext4 filesystem on my machine.
 //
 // To create a new result file, we first remove existing file. So, if
 // you repeatedly link a 1 GB program in a regular compile-link-debug
 // cycle, every cycle wastes 250 milliseconds only to remove a file.
 // Since LLD can link a 1 GB binary in about 5 seconds, that waste
 // actually counts.
 //
-// This function spawns a background thread to call unlink.
+// This function spawns a background thread to remove the file.
 // The calling thread returns almost immediately.
 void elf::unlinkAsync(StringRef Path) {
+// Removing a file is async on windows.
+#if defined(LLVM_ON_WIN32)
+  sys::fs::remove(Path);
+#else
   if (!ThreadsEnabled || !sys::fs::exists(Path) ||
       !sys::fs::is_regular_file(Path))
     return;
 
-  // First, rename Path to avoid race condition. We cannot remove
-  // Path from a different thread because we are now going to create
-  // Path as a new file. If we do that in a different thread, the new
-  // thread can remove the new file.
-  SmallString<128> TempPath;
-  if (sys::fs::createUniqueFile(Path + "tmp%%%%%%%%", TempPath))
+  // We cannot just remove path from a different thread because we are now going
+  // to create path as a new file.
+  // Instead we open the file and unlink it on this thread. The unlink is fast
+  // since the open fd guarantees that it is not removing the last reference.
+  int FD;
+  if (std::error_code EC = sys::fs::openFileForRead(Path, FD))
     return;
-  if (sys::fs::rename(Path, TempPath)) {
-    sys::fs::remove(TempPath);
-    return;
-  }
 
-  // Remove TempPath in background.
-  runBackground([=] { ::remove(TempPath.str().str().c_str()); });
+  sys::fs::remove(Path);
+
+  // close and therefore remove TempPath in background.
+  runBackground([=] { ::close(FD); });
+#endif
 }
 
 // Simulate file creation to see if Path is writable.
 //
 // Determining whether a file is writable or not is amazingly hard,
 // and after all the only reliable way of doing that is to actually
 // create a file. But we don't want to do that in this function
 // because LLD shouldn't update any file if it will end in a failure.