[PATCH] D41497: [ThinLTO][CachePruning] explicitly disable pruning

[ben via Phabricator via llvm-commits]

bd1976llvm created this revision.
bd1976llvm added reviewers: tejohnson, labath, pcc, mehdi_amini.
Herald added a reviewer: deadalnix.
Herald added subscribers: eraman, inglorion.

In https://reviews.llvm.org/rL321077 and https://reviews.llvm.org/D41231 I fixed a regression in the c-api which prevented the pruning from being *effectively* disabled.

However this approach, helpfully recommended by @labath, is cleaner.
It is also nice to remove the weasel words about effectively disabling from the api comments.


https://reviews.llvm.org/D41497

Files:
  include/llvm-c/lto.h
  include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
  include/llvm/Support/CachePruning.h
  lib/Support/CachePruning.cpp


Index: lib/Support/CachePruning.cpp
===================================================================
--- lib/Support/CachePruning.cpp
+++ lib/Support/CachePruning.cpp
@@ -155,8 +155,7 @@
   SmallString<128> TimestampFile(Path);
   sys::path::append(TimestampFile, "llvmcache.timestamp");
   sys::fs::file_status FileStatus;
-  const auto CurrentTime =
-      time_point_cast<decltype(Policy.Interval)>(system_clock::now());
+  const auto CurrentTime = system_clock::now();
   if (auto EC = sys::fs::status(TimestampFile, FileStatus)) {
     if (EC == errc::no_such_file_or_directory) {
       // If the timestamp file wasn't there, create one now.
@@ -166,13 +165,14 @@
       return false;
     }
   } else {
+    if (!Policy.Interval)
+      return false;
     if (Policy.Interval != seconds(0)) {
       // Check whether the time stamp is older than our pruning interval.
       // If not, do nothing.
-      const auto TimeStampModTime = time_point_cast<decltype(Policy.Interval)>(
-          FileStatus.getLastModificationTime());
+      const auto TimeStampModTime = FileStatus.getLastModificationTime();
       auto TimeStampAge = CurrentTime - TimeStampModTime;
-      if (TimeStampAge <= Policy.Interval) {
+      if (TimeStampAge <= *Policy.Interval) {
         DEBUG(dbgs() << "Timestamp file too recent ("
                      << duration_cast<seconds>(TimeStampAge).count()
                      << "s old), do not prune.\n");
Index: include/llvm/Support/CachePruning.h
===================================================================
--- include/llvm/Support/CachePruning.h
+++ include/llvm/Support/CachePruning.h
@@ -28,7 +28,7 @@
   /// The pruning interval. This is intended to be used to avoid scanning the
   /// directory too often. It does not impact the decision of which file to
   /// prune. A value of 0 forces the scan to occur.
-  std::chrono::seconds Interval = std::chrono::seconds(1200);
+  llvm::Optional<std::chrono::seconds> Interval = std::chrono::seconds(1200);
 
   /// The expiration for a file. When a file hasn't been accessed for Expiration
   /// seconds, it is removed from the cache. A value of 0 disables the
Index: include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
===================================================================
--- include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
+++ include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
@@ -148,16 +148,15 @@
   /// incremental build.
   void setCacheDir(std::string Path) { CacheOptions.Path = std::move(Path); }
 
-  /// Cache policy: interval (seconds) between two prunes of the cache. A
-  /// negative value sets the maximum possible pruning interval. A value
-  /// of 0 will be ignored.
+  /// Cache policy: interval (seconds) between two prunes of the cache. Set to a
+  /// negative value to disable pruning. A value of 0 will be ignored.
   void setCachePruningInterval(int Interval) {
-    static_assert(std::is_same<decltype(CacheOptions.Policy.Interval),
-                               std::chrono::seconds>::value,
-                  "ensure same types to avoid risk of overflow");
-    if (Interval)
-      CacheOptions.Policy.Interval = Interval > 0 ? std::chrono::seconds(Interval)
-                                                  : std::chrono::seconds::max();
+    if (Interval == 0)
+      return;
+    if(Interval < 0)
+      CacheOptions.Policy.Interval.reset();
+    else
+      CacheOptions.Policy.Interval = std::chrono::seconds(Interval);
   }
 
   /// Cache policy: expiration (in seconds) for an entry.
Index: include/llvm-c/lto.h
===================================================================
--- include/llvm-c/lto.h
+++ include/llvm-c/lto.h
@@ -764,7 +764,7 @@
  * To avoid filling the disk space, a few knobs are provided:
  *  - The pruning interval limits the frequency at which the garbage collector
  *    will try to scan the cache directory to prune expired entries.
- *    Setting to a negative number applies the maximum interval.
+ *    Setting to a negative number disables the pruning.
  *  - The pruning expiration time indicates to the garbage collector how old an
  *    entry needs to be to be removed.
  *  - Finally, the garbage collector can be instructed to prune the cache until
@@ -782,9 +782,9 @@
                                           const char *cache_dir);
 
 /**
- * Sets the cache pruning interval (in seconds). A negative value sets the
- * maximum possible pruning interval. An unspecified default value will be
- * applied, and a value of 0 will be ignored.
+ * Sets the cache pruning interval (in seconds). A negative value disables the
+ * pruning. An unspecified default value will be applied, and a value of 0 will
+ * be ignored.
  *
  * \since LTO_API_VERSION=18
  */


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41497.127897.patch
Type: text/x-patch
Size: 4745 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171221/0fb808c7/attachment.bin>