[ThinLTO/gold] Implement ThinLTO cache pruning support

[tejohnson]

Thanks for working on adding this missing feature! Please add a test and include an update to the info on where cache pruning is supported in ThinLTO.rst (for examples of both, see https://reviews.llvm.org/D37607, where it was added to lld).

[kongyi]

Tests added.

[kongyi]

In D37993#874181, @tejohnson wrote:

Thanks for working on adding this missing feature! Please add a test and include an update to the info on where cache pruning is supported in ThinLTO.rst (for examples of both, see https://reviews.llvm.org/D37607, where it was added to lld).

Done, thanks.

[tejohnson]

LGTM

[kongyi]

In D37993#874465, @tejohnson wrote:

LGTM

Thanks!

[mehdi_amini]

llvm/trunk/tools/gold/gold-plugin.cpp

229

Did you consider using the same option name as lld for consistency?
(i.e. why diverging when unnecessary)

[kongyi]

llvm/trunk/tools/gold/gold-plugin.cpp

229

I named this to be consistent with gold plugin's name for cache directory, "cache-dir".

[pcc]

Is it a good idea to support cache pruning in the gold plugin? As I mentioned in D31063 there are some unavoidable race conditions due to limitations in the plugin API.

llvm/trunk/tools/gold/gold-plugin.cpp

980

I think this should check cache_dir, not cache_policy.

[pcc]

In D37993#893466, @pcc wrote:

Is it a good idea to support cache pruning in the gold plugin? As I mentioned in D31063 there are some unavoidable race conditions due to limitations in the plugin API.

Ping... not sure whether it's a good idea to ship a known racy feature.

If you really want this feature in gold I'd suggest extending the gold plugin API so that it accepts file descriptors, and then have the plugin support this feature conditionally on whether the gold version is new enough.

[tejohnson]

In D37993#912428, @pcc wrote:

In D37993#893466, @pcc wrote:

Is it a good idea to support cache pruning in the gold plugin? As I mentioned in D31063 there are some unavoidable race conditions due to limitations in the plugin API.

Ping... not sure whether it's a good idea to ship a known racy feature.

If you really want this feature in gold I'd suggest extending the gold plugin API so that it accepts file descriptors, and then have the plugin support this feature conditionally on whether the gold version is new enough.

Hi Peter, thanks for pointing this out, and sorry for the delay in responding. Wondering whether instead of removing cache pruning support from gold-plugin for now, we could do something else: write the MemoryBuffer to another temp file and use that path (setting IsTemporary to true so it gets cleaned up). This would obviously require more space on disk, but maybe worth it to enable the pruning safely.

llvm/trunk/tools/gold/gold-plugin.cpp

980

It seems fine since pruneCache will return immediately if cache_dir is empty. But we could save some work if both were checked. I assume parseCachePruningPolicy will also work if cache_policy is empty, but it seems fine to have the checking.

[pcc]

In D37993#918918, @tejohnson wrote:

In D37993#912428, @pcc wrote:

In D37993#893466, @pcc wrote:

Is it a good idea to support cache pruning in the gold plugin? As I mentioned in D31063 there are some unavoidable race conditions due to limitations in the plugin API.

Ping... not sure whether it's a good idea to ship a known racy feature.

If you really want this feature in gold I'd suggest extending the gold plugin API so that it accepts file descriptors, and then have the plugin support this feature conditionally on whether the gold version is new enough.

Hi Peter, thanks for pointing this out, and sorry for the delay in responding. Wondering whether instead of removing cache pruning support from gold-plugin for now, we could do something else: write the MemoryBuffer to another temp file and use that path (setting IsTemporary to true so it gets cleaned up). This would obviously require more space on disk, but maybe worth it to enable the pruning safely.

Copying to a temporary file sounds fine to me. And if gold ever gets support for file descriptors we could always use the copying code path as a fallback for older versions.

llvm/trunk/tools/gold/gold-plugin.cpp

980

I was more thinking about the behaviour when a cache directory is specified and no cache policy is specified. In that case the behaviour of the other linkers is to prune with the default policy, so it's probably best to be consistent with that.

[tejohnson]

In D37993#918921, @pcc wrote:

In D37993#918918, @tejohnson wrote:

In D37993#912428, @pcc wrote:

In D37993#893466, @pcc wrote:

Is it a good idea to support cache pruning in the gold plugin? As I mentioned in D31063 there are some unavoidable race conditions due to limitations in the plugin API.

Ping... not sure whether it's a good idea to ship a known racy feature.

If you really want this feature in gold I'd suggest extending the gold plugin API so that it accepts file descriptors, and then have the plugin support this feature conditionally on whether the gold version is new enough.

Hi Peter, thanks for pointing this out, and sorry for the delay in responding. Wondering whether instead of removing cache pruning support from gold-plugin for now, we could do something else: write the MemoryBuffer to another temp file and use that path (setting IsTemporary to true so it gets cleaned up). This would obviously require more space on disk, but maybe worth it to enable the pruning safely.

Copying to a temporary file sounds fine to me. And if gold ever gets support for file descriptors we could always use the copying code path as a fallback for older versions.

Yi, can you address these suggestions?

llvm/trunk/tools/gold/gold-plugin.cpp

980

Oh, I see what you are saying. Yes, this makes sense.

[kongyi]

In D37993#893466, @pcc wrote:

Is it a good idea to support cache pruning in the gold plugin? As I mentioned in D31063 there are some unavoidable race conditions due to limitations in the plugin API.

Gold plugin

In D37993#920128, @tejohnson wrote:

In D37993#918921, @pcc wrote:

In D37993#918918, @tejohnson wrote:

In D37993#912428, @pcc wrote:

In D37993#893466, @pcc wrote:

Is it a good idea to support cache pruning in the gold plugin? As I mentioned in D31063 there are some unavoidable race conditions due to limitations in the plugin API.

Ping... not sure whether it's a good idea to ship a known racy feature.

If you really want this feature in gold I'd suggest extending the gold plugin API so that it accepts file descriptors, and then have the plugin support this feature conditionally on whether the gold version is new enough.

Hi Peter, thanks for pointing this out, and sorry for the delay in responding. Wondering whether instead of removing cache pruning support from gold-plugin for now, we could do something else: write the MemoryBuffer to another temp file and use that path (setting IsTemporary to true so it gets cleaned up). This would obviously require more space on disk, but maybe worth it to enable the pruning safely.

Copying to a temporary file sounds fine to me. And if gold ever gets support for file descriptors we could always use the copying code path as a fallback for older versions.

Yi, can you address these suggestions?

Sure, will fix these in a follow up change.

[tejohnson]

In D37993#921444, @kongyi wrote:

In D37993#893466, @pcc wrote:

Is it a good idea to support cache pruning in the gold plugin? As I mentioned in D31063 there are some unavoidable race conditions due to limitations in the plugin API.

Gold plugin

In D37993#920128, @tejohnson wrote:

In D37993#918921, @pcc wrote:

In D37993#918918, @tejohnson wrote:

In D37993#912428, @pcc wrote:

In D37993#893466, @pcc wrote:

Is it a good idea to support cache pruning in the gold plugin? As I mentioned in D31063 there are some unavoidable race conditions due to limitations in the plugin API.

Ping... not sure whether it's a good idea to ship a known racy feature.

If you really want this feature in gold I'd suggest extending the gold plugin API so that it accepts file descriptors, and then have the plugin support this feature conditionally on whether the gold version is new enough.

Hi Peter, thanks for pointing this out, and sorry for the delay in responding. Wondering whether instead of removing cache pruning support from gold-plugin for now, we could do something else: write the MemoryBuffer to another temp file and use that path (setting IsTemporary to true so it gets cleaned up). This would obviously require more space on disk, but maybe worth it to enable the pruning safely.

Copying to a temporary file sounds fine to me. And if gold ever gets support for file descriptors we could always use the copying code path as a fallback for older versions.

Yi, can you address these suggestions?

Sure, will fix these in a follow up change.

Ping - Yi, can you fix these?

[pcc]

Ping.

llvm/trunk/tools/gold/gold-plugin.cpp

980

Please at least address this.

[tejohnson]

llvm/trunk/tools/gold/gold-plugin.cpp

980

Since we haven't heard from Yi, I will go ahead and fix these. Just mailed you D43389 to fix this issue, I'll address the possible race separately.

llvm/trunk/test/tools/gold/X86/cache.ll

 ; RUN: rm -Rf %t.cache
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \
 ; RUN:     --plugin-opt=thinlto \
 ; RUN:     --plugin-opt=cache-dir=%t.cache \
 ; RUN:     -o %t3.o %t2.o %t.o
 
 ; RUN: ls %t.cache | count 2
 
+
+; Create two files that would be removed by cache pruning due to age.
+; We should only remove files matching the pattern "llvmcache-*".
+
+; RUN: touch -t 197001011200 %t.cache/llvmcache-foo %t.cache/foo
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:     --plugin-opt=thinlto \
+; RUN:     --plugin-opt=cache-dir=%t.cache \
+; RUN:     --plugin-opt=cache-policy=prune_after=1h \
+; RUN:     -o %t3.o %t2.o %t.o
+
+; Two cached objects, plus a timestamp file and "foo", minus the file we removed.
+; RUN: ls %t.cache | count 4
+
+
+; Create a file of size 64KB.
+; RUN: %python -c "print(' ' * 65536)" > %t.cache/llvmcache-foo
+
+; This should leave the file in place.
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:     --plugin-opt=thinlto \
+; RUN:     --plugin-opt=cache-dir=%t.cache \
+; RUN:     --plugin-opt=cache-policy=cache_size_bytes=128k \
+; RUN:     -o %t3.o %t2.o %t.o
+; RUN: ls %t.cache | count 5
+
+
+; This should remove it.
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:     --plugin-opt=thinlto \
+; RUN:     --plugin-opt=cache-dir=%t.cache \
+; RUN:     --plugin-opt=cache-policy=cache_size_bytes=32k \
+; RUN:     -o %t3.o %t2.o %t.o
+; RUN: ls %t.cache | count 4
+
+
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 define void @globalfunc() #0 {
 entry:
   ret void
 }

llvm/trunk/tools/gold/gold-plugin.cpp

 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/CodeGen/CommandFlags.h"
 #include "llvm/Config/config.h" // plugin-api.h requires HAVE_STDINT_H
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/LTO/Caching.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/Object/Error.h"
+#include "llvm/Support/CachePruning.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include <list>
#endif
   // Option to control the name of modules encoded in the individual index
   // files for a distributed backend. This enables the use of minimized
   // bitcode files for the thin link, assuming the name of the full bitcode
   // file used in the backend differs just in some part of the file suffix.
   // If specified, expects a string of the form "oldsuffix:newsuffix".
   static std::string thinlto_object_suffix_replace;
   // Optional path to a directory for caching ThinLTO objects.
   static std::string cache_dir;
+  // Optional pruning policy for ThinLTO caches.
+  static std::string cache_policy;
   // Additional options to pass into the code generator.
   // Note: This array will contain all plugin options which are not claimed
   // as plugin exclusive to pass to the code generator.
   static std::vector<const char *> extra;
   // Sample profile file path
   static std::string sample_profile;
 
   static void process_plugin_option(const char *opt_)
static void process_plugin_option(const char *opt_)
     } else if (opt.startswith("thinlto-object-suffix-replace=")) {
       thinlto_object_suffix_replace =
           opt.substr(strlen("thinlto-object-suffix-replace="));
       if (thinlto_object_suffix_replace.find(';') == std::string::npos)
         message(LDPL_FATAL,
                 "thinlto-object-suffix-replace expects 'old;new' format");
     } else if (opt.startswith("cache-dir=")) {
       cache_dir = opt.substr(strlen("cache-dir="));
+    } else if (opt.startswith("cache-policy=")) {
+      cache_policy = opt.substr(strlen("cache-policy="));
     } else if (opt.size() == 2 && opt[0] == 'O') {
       if (opt[1] < '0' || opt[1] > '3')
         message(LDPL_FATAL, "Optimization level must be between 0 and 3");
       OptLevel = opt[1] - '0';
     } else if (opt.startswith("jobs=")) {
       if (StringRef(opt_ + 5).getAsInteger(10, Parallelism))
         message(LDPL_FATAL, "Invalid parallelism level: %s", opt_ + 5);
     } else if (opt.startswith("lto-partitions=")) {

 static ld_plugin_status cleanup_hook(void) {
   for (std::string &Name : Cleanup) {
     std::error_code EC = sys::fs::remove(Name);
     if (EC)
       message(LDPL_ERROR, "Failed to delete '%s': %s", Name.c_str(),
               EC.message().c_str());
   }
 
+  // Prune cache
+  if (!options::cache_policy.empty()) {
+    CachePruningPolicy policy = check(parseCachePruningPolicy(options::cache_policy));
+    pruneCache(options::cache_dir, policy);
+  }
+
   return LDPS_OK;
 }