[lldb] [llvm] Debuginfod cache use index cache settings and include real file name. (PR #120814)

via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 20 17:54:05 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-debuginfo

Author: None (GeorgeHuyubo)

<details>
<summary>Changes</summary>

This PR include two changes:
1. Change debuginfod cache file name to include origin file name, the new file name would be something like:
llvmcache-13267c5f5d2e3df472c133c8efa45fb3331ef1ea-liblzma.so.5.2.2.debuginfo.dwp
So it will provide more information in image list instead of a plain llvmcache-123
2. Switch debuginfod cache to use lldb index cache settings. Currently we don't have proper settings for setting the cache path or the cache expiration time for debuginfod cache. We want to use the lldb index cache settings, as they make sense to be in the same place and have the same TTL.

---
Full diff: https://github.com/llvm/llvm-project/pull/120814.diff


4 Files Affected:

- (modified) lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp (+34-7) 
- (modified) llvm/include/llvm/Debuginfod/Debuginfod.h (+3-1) 
- (modified) llvm/lib/Debuginfod/Debuginfod.cpp (+20-9) 
- (modified) llvm/unittests/Debuginfod/DebuginfodTests.cpp (+2-1) 


``````````diff
diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index 2cd7bbbb244902..f459825a61a562 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -8,6 +8,7 @@
 
 #include "SymbolLocatorDebuginfod.h"
 
+#include "lldb/Core/DataFileCache.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Interpreter/OptionValueString.h"
 #include "lldb/Utility/Args.h"
@@ -141,6 +142,25 @@ SymbolLocator *SymbolLocatorDebuginfod::CreateInstance() {
   return new SymbolLocatorDebuginfod();
 }
 
+static llvm::StringRef getFileName(const ModuleSpec &module_spec,
+                                   std::string url_path) {
+  // Check if the URL path requests an executable file or a symbol file
+  bool is_executable = url_path.find("debuginfo") == std::string::npos;
+  if (is_executable) {
+    return module_spec.GetFileSpec().GetFilename().GetStringRef();
+  }
+  llvm::StringRef symbol_file =
+      module_spec.GetSymbolFileSpec().GetFilename().GetStringRef();
+  // Remove llvmcache- prefix and hash, keep origin file name
+  if (symbol_file.starts_with("llvmcache-")) {
+    size_t pos = symbol_file.rfind('-');
+    if (pos != llvm::StringRef::npos) {
+      symbol_file = symbol_file.substr(pos + 1);
+    }
+  }
+  return symbol_file;
+}
+
 static std::optional<FileSpec>
 GetFileForModule(const ModuleSpec &module_spec,
                  std::function<std::string(llvm::object::BuildID)> UrlBuilder) {
@@ -154,11 +174,14 @@ GetFileForModule(const ModuleSpec &module_spec,
   // Grab LLDB's Debuginfod overrides from the
   // plugin.symbol-locator.debuginfod.* settings.
   PluginProperties &plugin_props = GetGlobalPluginProperties();
-  llvm::Expected<std::string> cache_path_or_err = plugin_props.GetCachePath();
-  // A cache location is *required*.
-  if (!cache_path_or_err)
-    return {};
-  std::string cache_path = *cache_path_or_err;
+  // Grab the lldb index cache settings from the global module list properties.
+  ModuleListProperties &properties =
+      ModuleList::GetGlobalModuleListProperties();
+  std::string cache_path = properties.GetLLDBIndexCachePath().GetPath();
+
+  llvm::CachePruningPolicy pruning_policy =
+      DataFileCache::GetLLDBIndexCachePolicy();
+
   llvm::SmallVector<llvm::StringRef> debuginfod_urls =
       llvm::getDefaultDebuginfodUrls();
   std::chrono::milliseconds timeout = plugin_props.GetTimeout();
@@ -166,9 +189,13 @@ GetFileForModule(const ModuleSpec &module_spec,
   // We're ready to ask the Debuginfod library to find our file.
   llvm::object::BuildID build_id(module_uuid.GetBytes());
   std::string url_path = UrlBuilder(build_id);
-  std::string cache_key = llvm::getDebuginfodCacheKey(url_path);
+  llvm::StringRef file_name = getFileName(module_spec, url_path);
+  std::string cache_file_name = llvm::toHex(build_id, true);
+  if (!file_name.empty()) {
+    cache_file_name += "-" + file_name.str();
+  }
   llvm::Expected<std::string> result = llvm::getCachedOrDownloadArtifact(
-      cache_key, url_path, cache_path, debuginfod_urls, timeout);
+      cache_file_name, url_path, cache_path, debuginfod_urls, timeout, pruning_policy);
   if (result)
     return FileSpec(*result);
 
diff --git a/llvm/include/llvm/Debuginfod/Debuginfod.h b/llvm/include/llvm/Debuginfod/Debuginfod.h
index 99fe15ad859794..aebcf31cd48227 100644
--- a/llvm/include/llvm/Debuginfod/Debuginfod.h
+++ b/llvm/include/llvm/Debuginfod/Debuginfod.h
@@ -25,6 +25,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/BuildID.h"
+#include "llvm/Support/CachePruning.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Mutex.h"
@@ -95,7 +96,8 @@ Expected<std::string> getCachedOrDownloadArtifact(StringRef UniqueKey,
 /// found, uses the UniqueKey for the local cache file.
 Expected<std::string> getCachedOrDownloadArtifact(
     StringRef UniqueKey, StringRef UrlPath, StringRef CacheDirectoryPath,
-    ArrayRef<StringRef> DebuginfodUrls, std::chrono::milliseconds Timeout);
+    ArrayRef<StringRef> DebuginfodUrls, std::chrono::milliseconds Timeout,
+    llvm::CachePruningPolicy policy);
 
 class ThreadPoolInterface;
 
diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp
index 4c785117ae8ef7..17efaea892c669 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -106,6 +106,14 @@ Expected<std::string> getDefaultDebuginfodCacheDirectory() {
   return std::string(CacheDirectory);
 }
 
+Expected<llvm::CachePruningPolicy> getDefaultDebuginfodCachePruningPolicy() {
+  Expected<CachePruningPolicy> PruningPolicyOrErr =
+      parseCachePruningPolicy(std::getenv("DEBUGINFOD_CACHE_POLICY"));
+  if (!PruningPolicyOrErr)
+    return PruningPolicyOrErr.takeError();
+  return *PruningPolicyOrErr;
+}
+
 std::chrono::milliseconds getDefaultDebuginfodTimeout() {
   long Timeout;
   const char *DebuginfodTimeoutEnv = std::getenv("DEBUGINFOD_TIMEOUT");
@@ -169,9 +177,15 @@ Expected<std::string> getCachedOrDownloadArtifact(StringRef UniqueKey,
     return CacheDirOrErr.takeError();
   CacheDir = *CacheDirOrErr;
 
-  return getCachedOrDownloadArtifact(UniqueKey, UrlPath, CacheDir,
-                                     getDefaultDebuginfodUrls(),
-                                     getDefaultDebuginfodTimeout());
+  Expected<llvm::CachePruningPolicy> PruningPolicyOrErr =
+      getDefaultDebuginfodCachePruningPolicy();
+  if (!PruningPolicyOrErr)
+    return PruningPolicyOrErr.takeError();
+  llvm::CachePruningPolicy PruningPolicy = *PruningPolicyOrErr;
+
+  return getCachedOrDownloadArtifact(
+      UniqueKey, UrlPath, CacheDir, getDefaultDebuginfodUrls(),
+      getDefaultDebuginfodTimeout(), PruningPolicy);
 }
 
 namespace {
@@ -250,7 +264,8 @@ static SmallVector<std::string, 0> getHeaders() {
 
 Expected<std::string> getCachedOrDownloadArtifact(
     StringRef UniqueKey, StringRef UrlPath, StringRef CacheDirectoryPath,
-    ArrayRef<StringRef> DebuginfodUrls, std::chrono::milliseconds Timeout) {
+    ArrayRef<StringRef> DebuginfodUrls, std::chrono::milliseconds Timeout,
+    llvm::CachePruningPolicy policy) {
   SmallString<64> AbsCachedArtifactPath;
   sys::path::append(AbsCachedArtifactPath, CacheDirectoryPath,
                     "llvmcache-" + UniqueKey);
@@ -304,11 +319,7 @@ Expected<std::string> getCachedOrDownloadArtifact(
         continue;
     }
 
-    Expected<CachePruningPolicy> PruningPolicyOrErr =
-        parseCachePruningPolicy(std::getenv("DEBUGINFOD_CACHE_POLICY"));
-    if (!PruningPolicyOrErr)
-      return PruningPolicyOrErr.takeError();
-    pruneCache(CacheDirectoryPath, *PruningPolicyOrErr);
+    pruneCache(CacheDirectoryPath, policy);
 
     // Return the path to the artifact on disk.
     return std::string(AbsCachedArtifactPath);
diff --git a/llvm/unittests/Debuginfod/DebuginfodTests.cpp b/llvm/unittests/Debuginfod/DebuginfodTests.cpp
index 5312912599e933..8dacf2ae5b3f8a 100644
--- a/llvm/unittests/Debuginfod/DebuginfodTests.cpp
+++ b/llvm/unittests/Debuginfod/DebuginfodTests.cpp
@@ -37,6 +37,7 @@ TEST(DebuginfodClient, CacheHit) {
   sys::fs::createTemporaryFile("llvmcache-key", "temp", FD, CachedFilePath);
   StringRef CacheDir = sys::path::parent_path(CachedFilePath);
   StringRef UniqueKey = sys::path::filename(CachedFilePath);
+  llvm::CachePruningPolicy policy;
   EXPECT_TRUE(UniqueKey.consume_front("llvmcache-"));
   raw_fd_ostream OF(FD, true, /*unbuffered=*/true);
   OF << "contents\n";
@@ -44,7 +45,7 @@ TEST(DebuginfodClient, CacheHit) {
   OF.close();
   Expected<std::string> PathOrErr = getCachedOrDownloadArtifact(
       UniqueKey, /*UrlPath=*/"/null", CacheDir,
-      /*DebuginfodUrls=*/{}, /*Timeout=*/std::chrono::milliseconds(1));
+      /*DebuginfodUrls=*/{}, /*Timeout=*/std::chrono::milliseconds(1), policy);
   EXPECT_THAT_EXPECTED(PathOrErr, HasValue(CachedFilePath));
 }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/120814


More information about the llvm-commits mailing list