[Lldb-commits] [PATCH] D131531: [lldb] Allow DataFileCache to be constructed with a different policy

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/source/Core/DataFileCache.cpp:23-39
   // Prune the cache based off of the LLDB settings each time we create a cache
   // object.
   ModuleListProperties &properties =
       ModuleList::GetGlobalModuleListProperties();
   llvm::CachePruningPolicy policy;
   // Only scan once an hour. If we have lots of debug sessions we don't want
   // to scan this directory too often. A timestamp file is written to the
----------------
Would it make sense to extract this into a static function say `DefaultCachePruningPolicy` and then have one constructor with an optional policy argument? Something like this:

```
DataFileCache(llvm::StringRef path, llvm::CachePruningPolicy policy = DataFileCache::DefaultCachePruningPolicy);
```

In addition to having only one constructor, the new function also provides a natural place to document the policy.


================
Comment at: lldb/source/Core/DataFileCache.cpp:40
       std::chrono::hours(properties.GetLLDBIndexCacheExpirationDays() * 24);
   pruneCache(path, policy);
+  DataFileCache(path, policy);
----------------
Is it intentional that we now call this twice?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131531/new/

https://reviews.llvm.org/D131531