[ThinLTO] Add a couple of more knobs to C API to control cache size.

[kromanova]

Summary

Exposing a couple of C LTO API functions to control the size and amount of files in cache directory. These functions have been supported in C++ LTO API for a long time.

[kromanova]

Hello,
Will it possible to review this patch also? Thank you so much in advance.
Does someone knows who in Apple is responsible for ThinLTO? It might be helpful to add this person as a reviewer.
Katya.

[tejohnson]

include/llvm/LTO/legacy/ThinLTOCodeGenerator.h

169 ↗

(On Diff #131148)

extraneous new line

190 ↗

(On Diff #131148)

In CachePruning.h, a value of 0 for this field disables size based pruning. Why do something different here (which seems to go in the opposite direction from D42267)?

tools/llvm-lto/llvm-lto.cpp

163 ↗

(On Diff #131148)

line too long

166 ↗

(On Diff #131148)

line too long

166 ↗

(On Diff #131148)

default here is off by a factor of 10 from the default in CachePruning.h.

[kromanova]

Addressed code review comments.

include/llvm/LTO/legacy/ThinLTOCodeGenerator.h

190 ↗

(On Diff #131148)

In D42267 a value 0 for PruningInterval actually forces the scan to occur, so its meaning is somewhat different.

190 ↗

(On Diff #131148)

When writing the patch, I was hesitating for a whether to ignore value 0 here (for a time being) or use it for initializing CacheOptions.Policy.* (as a part of this patch).

I decided to ignore a value 0 for now and that's why :

the value 0 for two previously existing pruning control functions is currently ignored (while I believe it should be changed to have the same effect as in C++ AP!)

• setCacheEntryExpiration
• setMaxCacheSizeRelativeToAvailableSpace

I added these two knobs in this patch:

• setCacheMaxSizeBytes
• setCacheMaxSizeFiles

and decided to keep for now consistent with setCacheEntryExpiration and setMaxCacheSizeRelativeToAvailableSpace, since all of these 4 functions are related.

I could have changed all of these functions and accept value 0 as a part of this patch, but a lot of tests needs to be written, so I thought it's better to do it as a separate commit.

tools/llvm-lto/llvm-lto.cpp

166 ↗

(On Diff #131148)

Done.

However, I noticed that most of the functions from line 68 6o 215 in llvm-lto.cpp are formatted very inconsistently. I'd like to make the formatting uniformed. It could be done as a part of this or (preferably) different commit. If you think this "tidying up" needs to be done, which function should I use as a good example?
I personally prefer the formatting similar to the one used below, but I'm open to any other. as long as it's consistent. Or I could just run this part of the file through clang-format. Let me know.

static cl::list<std::string> ExportedSymbols(
    "exported-symbol",
    cl::desc("List of symbols to export from the resulting object file"),
    cl::ZeroOrMore);

166 ↗

(On Diff #131148)

Ah, good catch! Thanks. Can't count my zeros.

[kromanova]

include/llvm/LTO/legacy/ThinLTOCodeGenerator.h

190 ↗

(On Diff #131148)

Also, before making a subsequent commit to accept (instead of ignoring) 0 values for setCache* functions, I'd like to get ld64 developers option that they are OK with this. Who is responsible for ld64 or LTO project in Apple?

[kromanova]

Please let me know if more changes needed to this patch.

Based on the discussion in this code review, I'm planning to make two simple follow-up patches:
(1) formatting in llvm-lto.cpp (let me know an example of preferred formatting that I could use as a template or I could use plain clang-format).
(2) accepting 0 values for setCacheEntryExpiration, setMaxCacheSizeRelativeToAvailableSpace, setCacheMaxSizeBytes, setCacheMaxSizeFiles.

[kromanova]

Adding Apple's developers to the list of subscribers, since this patch is related to C LTO API .

[kromanova]

Teresa said that Peter Collingbourne might be the code owner for C LTO API, so I'm including him as a reviewer for this code review as well. This review was sitting for a while. Peter, could you please have a look and approve if everything looks OK to you.

[pcc]

I have no strong feelings about what happens in the legacy API, so probably someone at Apple should take a look first.

[kromanova]

Peter suggested that someone from Apple should have a look. I added Steven Wu as a reviewer.
Steven, could you (or someone else responsible for ld64 or ThinLTO) review and approve?

[steven_wu]

The API change is fine for ld64. But you need to do the following when you update LTO C API:

• You need to declare the interface in include/llvm-c/lto.h
• You need to bump LTO_API_VERSION in that header and document the new API is available since the new LTO_API_VERSION.
• You need to update tools/lto/lto.exports. This is the export list used by ld64 and only the interface listed in that file will be exported on darwin platform.

[kromanova]

Updated this patch to address Steven Wu's comments:

• Declared new interfaces in include/llvm-c/lto.h;
• Bumped LTO_API_VERSION;
• Documented the new APIs available since the new LTO_API_VERSION;
• Updated tools/lto/lto.exports (hopefully the order of the functions is not important, because I re-group them slightly).

[steven_wu]

LGTM. And yes, order doesn't matter.

llvm/trunk/include/llvm-c/lto.h

 
 /**
  * @defgroup LLVMCLTO LTO
  * @ingroup LLVMC
  *
  * @{
  */
 
-#define LTO_API_VERSION 21
+#define LTO_API_VERSION 22
 
 /**
  * \since prior to LTO_API_VERSION=3
  */
 typedef enum {
     LTO_SYMBOL_ALIGNMENT_MASK              = 0x0000001F, /* log2 of alignment */
     LTO_SYMBOL_PERMISSIONS_MASK            = 0x000000E0,
     LTO_SYMBOL_PERMISSIONS_CODE            = 0x000000A0,

  * default value will be applied. A value of 0 will be ignored.
  *
  * \since LTO_API_VERSION=18
  */
 extern void thinlto_codegen_set_cache_entry_expiration(thinlto_code_gen_t cg,
                                                        unsigned expiration);
 
 /**
+ * Sets the maximum size of the cache directory (in bytes). A value over the
+ * amount of available space on the disk will be reduced to the amount of
+ * available space. An unspecified default value will be applied. A value of 0
+ * will be ignored.
+ *
+ * \since LTO_API_VERSION=22
+ */
+extern void thinlto_codegen_set_cache_size_bytes(thinlto_code_gen_t cg,
+                                                 unsigned max_size_bytes);
+
+/**
+ * Sets the maximum number of files in the cache directory. An unspecified
+ * default value will be applied. A value of 0 will be ignored.
+ *
+ * \since LTO_API_VERSION=22
+ */
+extern void thinlto_codegen_set_cache_size_files(thinlto_code_gen_t cg,
+                                                 unsigned max_size_files);
+
+
+
+/**
  * @} // endgroup LLVMCTLTO_CACHING
  */
 
 #ifdef __cplusplus
 }
 #endif
 
 #endif /* LLVM_C_LTO_H */

llvm/trunk/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h

public:
    *  NewCacheSize = AvailableSpace * P/100
    *
    */
   void setMaxCacheSizeRelativeToAvailableSpace(unsigned Percentage) {
     if (Percentage)
       CacheOptions.Policy.MaxSizePercentageOfAvailableSpace = Percentage;
   }
 
+  /// Cache policy: the maximum size for the cache directory in bytes. A value
+  /// over the amount of available space on the disk will be reduced to the
+  /// amount of available space. A value of 0 will be ignored.
+  void setCacheMaxSizeBytes(unsigned MaxSizeBytes) {
+    if (MaxSizeBytes)
+      CacheOptions.Policy.MaxSizeBytes = MaxSizeBytes;
+  }
+
+  /// Cache policy: the maximum number of files in the cache directory. A value
+  /// of 0 will be ignored.
+  void setCacheMaxSizeFiles(unsigned MaxSizeFiles) {
+    if (MaxSizeFiles)
+      CacheOptions.Policy.MaxSizeFiles = MaxSizeFiles;
+  }
+
   /**@}*/
 
   /// Set the path to a directory where to save temporaries at various stages of
   /// the processing.
   void setSaveTempsDir(std::string Path) { SaveTempsDir = std::move(Path); }
 
   /// Set the path to a directory where to save generated object files. This
   /// path can be used by a linker to request on-disk files instead of in-memory

llvm/trunk/test/ThinLTO/X86/cache.ll

 ; llvmcache.timestamp is current,
 ; cache file is older than default cache expiration value.
 ; RUN: rm -Rf %t.cache && mkdir %t.cache
 ; RUN: touch -t 197001011200 %t.cache/llvmcache-foo
 ; RUN: touch %t.cache/llvmcache.timestamp
 ; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc %t.bc -thinlto-cache-dir %t.cache --thinlto-cache-pruning-interval 0
 ; RUN: not ls %t.cache/llvmcache-foo
 
+; Verify that specifying max size for the cache directory prunes it to this
+; size, removing the largest files first.
+; RUN: rm -Rf %t.cache && mkdir %t.cache
+; Create cache files with different sizes.
+; Only 8B, 16B and 76B files should stay after pruning.
+; RUN: %python -c "print(' ' * 1023)" > %t.cache/llvmcache-foo-1024
+; RUN: %python -c "print(' ' * 15)" > %t.cache/llvmcache-foo-16
+; RUN: %python -c "print(' ' * 7)" > %t.cache/llvmcache-foo-8
+; RUN: %python -c "print(' ' * 75)" > %t.cache/llvmcache-foo-76
+; RUN: %python -c "print(' ' * 76)" > %t.cache/llvmcache-foo-77
+; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc %t.bc -thinlto-cache-dir %t.cache --thinlto-cache-max-size-bytes 100
+; RUN: ls %t.cache/llvmcache-foo-16
+; RUN: ls %t.cache/llvmcache-foo-8
+; RUN: ls %t.cache/llvmcache-foo-76
+; RUN: not ls %t.cache/llvmcache-foo-1024
+; RUN: not ls %t.cache/llvmcache-foo-77
+
+; Verify that specifying max number of files in the cache directory prunes
+; it to this amount, removing the largest files first.
+; RUN: rm -Rf %t.cache && mkdir %t.cache
+; Create cache files with different sizes.
+; Only 8B and 16B files should stay after pruning.
+; RUN: %python -c "print(' ' * 1023)" > %t.cache/llvmcache-foo-1024
+; RUN: %python -c "print(' ' * 15)" > %t.cache/llvmcache-foo-16
+; RUN: %python -c "print(' ' * 7)" > %t.cache/llvmcache-foo-8
+; RUN: %python -c "print(' ' * 75)" > %t.cache/llvmcache-foo-76
+; RUN: %python -c "print(' ' * 76)" > %t.cache/llvmcache-foo-77
+; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc %t.bc -thinlto-cache-dir %t.cache --thinlto-cache-max-size-files 2
+; RUN: ls %t.cache/llvmcache-foo-16
+; RUN: ls %t.cache/llvmcache-foo-8
+; RUN: not ls %t.cache/llvmcache-foo-76
+; RUN: not ls %t.cache/llvmcache-foo-1024
+; RUN: not ls %t.cache/llvmcache-foo-77
+
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 define void @globalfunc() #0 {
 entry:
   ret void
 }

llvm/trunk/tools/llvm-lto/llvm-lto.cpp

 
 static cl::opt<std::string>
     ThinLTOCacheDir("thinlto-cache-dir", cl::desc("Enable ThinLTO caching."));
 
 static cl::opt<int>
     ThinLTOCachePruningInterval("thinlto-cache-pruning-interval",
     cl::init(1200), cl::desc("Set ThinLTO cache pruning interval."));
 
+static cl::opt<int>
+    ThinLTOCacheMaxSizeBytes("thinlto-cache-max-size-bytes",
+    cl::desc("Set ThinLTO cache pruning directory maximum size in bytes."));
+
+static cl::opt<int>
+    ThinLTOCacheMaxSizeFiles("thinlto-cache-max-size-files", cl::init(1000000),
+    cl::desc("Set ThinLTO cache pruning directory maximum number of files."));
+
 static cl::opt<std::string> ThinLTOSaveTempsPrefix(
     "thinlto-save-temps",
     cl::desc("Save ThinLTO temp files using filenames created by adding "
              "suffixes to the given file path prefix."));
 
 static cl::opt<std::string> ThinLTOGeneratedObjectsDir(
     "thinlto-save-objects",
     cl::desc("Save ThinLTO generated object files using filenames created in "

 public:
   ThinLTOCodeGenerator ThinGenerator;
 
   ThinLTOProcessing(const TargetOptions &Options) {
     ThinGenerator.setCodePICModel(getRelocModel());
     ThinGenerator.setTargetOptions(Options);
     ThinGenerator.setCacheDir(ThinLTOCacheDir);
     ThinGenerator.setCachePruningInterval(ThinLTOCachePruningInterval);
+    ThinGenerator.setCacheMaxSizeFiles(ThinLTOCacheMaxSizeFiles);
+    ThinGenerator.setCacheMaxSizeBytes(ThinLTOCacheMaxSizeBytes);
     ThinGenerator.setFreestanding(EnableFreestanding);
 
     // Add all the exported symbols to the table of symbols to preserve.
     for (unsigned i = 0; i < ExportedSymbols.size(); ++i)
       ThinGenerator.preserveSymbol(ExportedSymbols[i]);
   }
 
   void run() {

llvm/trunk/tools/lto/lto.cpp

void thinlto_codegen_set_cache_entry_expiration(thinlto_code_gen_t cg,
   return unwrap(cg)->setCacheEntryExpiration(expiration);
 }
 
 void thinlto_codegen_set_final_cache_size_relative_to_available_space(
     thinlto_code_gen_t cg, unsigned Percentage) {
   return unwrap(cg)->setMaxCacheSizeRelativeToAvailableSpace(Percentage);
 }
 
+void thinlto_codegen_set_cache_size_bytes(
+    thinlto_code_gen_t cg, unsigned MaxSizeBytes) {
+  return unwrap(cg)->setCacheMaxSizeBytes(MaxSizeBytes);
+}
+
+void thinlto_codegen_set_cache_size_files(
+    thinlto_code_gen_t cg, unsigned MaxSizeFiles) {
+  return unwrap(cg)->setCacheMaxSizeFiles(MaxSizeFiles);
+}
+
 void thinlto_codegen_set_savetemps_dir(thinlto_code_gen_t cg,
                                        const char *save_temps_dir) {
   return unwrap(cg)->setSaveTempsDir(save_temps_dir);
 }
 
 void thinlto_set_generated_objects_dir(thinlto_code_gen_t cg,
                                        const char *save_temps_dir) {
   unwrap(cg)->setGeneratedObjectsDirectory(save_temps_dir);

llvm/trunk/tools/lto/lto.exports

 thinlto_codegen_add_module
 thinlto_codegen_process
 thinlto_module_get_num_objects
 thinlto_module_get_object
 thinlto_codegen_set_pic_model
 thinlto_codegen_set_cache_dir
 thinlto_codegen_set_cache_pruning_interval
 thinlto_codegen_set_cache_entry_expiration
+thinlto_codegen_set_final_cache_size_relative_to_available_space
+thinlto_codegen_set_cache_size_bytes
+thinlto_codegen_set_cache_size_files
 thinlto_codegen_set_savetemps_dir
 thinlto_codegen_set_cpu
 thinlto_debug_options
 lto_module_is_thinlto
 thinlto_codegen_add_must_preserve_symbol
 thinlto_codegen_add_cross_referenced_symbol
-thinlto_codegen_set_final_cache_size_relative_to_available_space
 thinlto_codegen_set_codegen_only
 thinlto_codegen_disable_codegen
 thinlto_module_get_num_object_files
 thinlto_module_get_object_file
-thinlto_set_generated_objects_dir
+thinlto_set_generated_objects_dir