[lld-macho] Hardlink -object_path_lto files to cache when possible

[lgrey]

Summary

This is a follow-up to https://reviews.llvm.org/D131624 (specifically to https://reviews.llvm.org/D131624#3716584)

[thakis]

Cool, thanks!

Should we try to test this? Say, with a REQUIRES: shell line (to rule out Windows which can't hardlink files without admin) and ls -l somefile | cut -f 3 -d ' ' and check that this link count is 2?

lld/MachO/LTO.cpp

60

nit: To make it more clear that this isn't a function that returns a success bool, I'd write this as:

autor err = fs::create_hard_link(*originalPath, path);
if (!err)
  return;

(I assumed create_hard_link returns true on success and false on failure and the wondered why we'd return on failure here.)

[smeenai]

(to rule out Windows which can't hardlink files without admin)

IIRC Windows can create hard links without admin. It's symbolic links which require admin (until developer mode on Windows 10).

[lgrey]

Add test, clarify control flow

[lgrey]

Added test. For some reason, I thought it was less practical than it ended up being, thanks for the suggestion.

I guess the Windows thing is a moot point either way since this test already has`UNSUPPORTED: system-windows`

[steven_wu]

LGTM. Small comment inline.

lld/test/MachO/lto-object-path.ll

26

Would be good to add a short comment about what this is testing. It is not obvious that you are reading the number of links field from ls long format.

[lgrey]

Comment on test, make test work on Linux

lld/MachO/LTO.cpp

static lto::Config createConfig() {
   c.OptLevel = config->ltoo;
   c.CGOptLevel = args::getCGOptLevel(config->ltoo);
   if (config->saveTemps)
     checkError(c.addSaveTemps(config->outputFile.str() + ".",
                               /*UseInputModulePath=*/true));
   return c;
 }
 
+// If `originalPath` exists, hardlinks `path` to `originalPath`. If that fails,
+// or `originalPath` is not set, saves `buffer` to `path`.
+static void saveOrHardlinkBuffer(StringRef buffer, const Twine &path,
+                                 Optional<StringRef> originalPath) {
+  if (originalPath) {
+    auto err = fs::create_hard_link(*originalPath, path);
+    if (!err)
+      return;
+  }
+  saveBuffer(buffer, path);
+}
+
 BitcodeCompiler::BitcodeCompiler() {
   lto::ThinBackend backend = lto::createInProcessThinBackend(
       heavyweight_hardware_concurrency(config->thinLTOJobs));
   ltoObj = std::make_unique<lto::LTO>(createConfig(), backend);
 }
 
 void BitcodeCompiler::add(BitcodeFile &f) {
   ArrayRef<lto::InputFile::Symbol> objSyms = f.obj->symbols();
std::vector<ObjFile *> BitcodeCompiler::compile() {
   }
 
   std::vector<ObjFile *> ret;
   for (unsigned i = 0; i < maxTasks; ++i) {
     // Get the native object contents either from the cache or from memory.  Do
     // not use the cached MemoryBuffer directly to ensure dsymutil does not
     // race with the cache pruner.
     StringRef objBuf;
-    if (files[i])
+    Optional<StringRef> cachePath = llvm::None;
+    if (files[i]) {
       objBuf = files[i]->getBuffer();
-    else
+      cachePath = files[i]->getBufferIdentifier();
+    } else {
       objBuf = buf[i];
+    }
     if (objBuf.empty())
       continue;
 
     // FIXME: should `saveTemps` and `ltoObjPath` use the same file name?
     if (config->saveTemps)
       saveBuffer(objBuf,
                  config->outputFile + ((i == 0) ? "" : Twine(i)) + ".lto.o");
 
     SmallString<261> filePath("/tmp/lto.tmp");
     uint32_t modTime = 0;
     if (!config->ltoObjPath.empty()) {
       filePath = config->ltoObjPath;
       if (objPathIsDir)
         path::append(filePath, Twine(i) + "." +
                                    getArchitectureName(config->arch()) +
                                    ".lto.o");
-      saveBuffer(objBuf, filePath);
+      saveOrHardlinkBuffer(objBuf, filePath, cachePath);
       modTime = getModTime(filePath);
     }
     ret.push_back(make<ObjFile>(
         MemoryBufferRef(objBuf, saver().save(filePath.str())), modTime, ""));
   }
 
   return ret;
 }

lld/test/MachO/lto-object-path.ll

 ; RUN: llvm-nm -pa %t/test | FileCheck %s --check-prefixes CHECK,OBJPATH-DIR -DDIR=%t/lto-temps
 
 ;; Ensure object path is still used if the cache is used
 ; RUN: %lld %t/test.o -o %t/test -object_path_lto %t/lto-temps -prune_interval_lto 20 -cache_path_lto %t/cache --thinlto-cache-policy=cache_size_files=1:cache_size_bytes=10
 ; RUN: llvm-nm -pa %t/test | FileCheck %s --check-prefixes CHECK,OBJPATH-DIR -DDIR=%t/lto-temps
 ;; And that dsymutil can read the result
 ; RUN: dsymutil -f -o - %t/test | llvm-dwarfdump - | FileCheck %s --check-prefix=DSYM
 
+;; Test that the object path hardlinks to the cache when possible
+;; NB: 
+;; 1) Need to delete current object path contents or hardlink will fail.
+;; 2) Note the cache policy is different from the test above (which is crafted
+;;    to ensure that the cache is pruned immediately; we want the opposite here).
+; RUN: rm -rf %t/lto-temps
+; RUN: %lld %t/test.o -o %t/test -object_path_lto %t/lto-temps -prune_interval_lto 20 -cache_path_lto %t/cache --thinlto-cache-policy=cache_size_files=10:cache_size_bytes=10000
+;; Get the link count.
+; RUN: ls -l %t/lto-temps/1.x86_64.lto.o | tr -s ' ' | cut -f 2 -d ' ' | FileCheck %s --check-prefix=HARDLINK
+
 ;; check that the object path can be an existing file
 ; RUN: touch %t/lto-tmp.o
 ; RUN: %lld %t/test.o -o %t/test -object_path_lto %t/lto-tmp.o
 ; RUN: llvm-nm -pa %t/test | FileCheck %s --check-prefixes CHECK,OBJPATH-FILE -DFILE=%t/lto-tmp.o
 
 
 ; CHECK:             0000000000000000                - 00 0000    SO /tmp/test.cpp
 ; NOOBJPATH-NEXT:    0000000000000000                - 03 0001   OSO /tmp/lto.tmp
 ;; check that modTime is nonzero when `-object_path_lto` is provided
 ; OBJPATH-DIR-NEXT:  {{[0-9a-f]*[1-9a-f]+[0-9a-f]*}} - 03 0001   OSO [[DIR]]/1.x86_64.lto.o
 ; OBJPATH-FILE-NEXT: {{[0-9a-f]*[1-9a-f]+[0-9a-f]*}} - 03 0001   OSO [[FILE]]
 ; CHECK-NEXT:        {{[0-9a-f]+}}                   - 01 0000   FUN _main
 ; CHECK-NEXT:        0000000000000001                - 00 0000   FUN
 ; CHECK-NEXT:        0000000000000000                - 01 0000    SO
 ; CHECK-NEXT:        {{[0-9a-f]+}}                   T _main
 ; DSYM: DW_AT_name ("test.cpp")
+; HARDLINK: 2
 
 target triple = "x86_64-apple-macosx10.15.0"
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
 define void @main() #0 !dbg !4 {
   ret void
 }