Debuginfod cache use index cache settings and include real file name #120814
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-debuginfo Author: None (GeorgeHuyubo) ChangesThis PR include two changes:
Full diff: https://github.com/llvm/llvm-project/pull/120814.diff 4 Files Affected:
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));
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
GeorgeHuyubo
force-pushed
the
debuginfod
branch
from
5813472 to
7b808e7
Compare
GeorgeHuyubo
changed the title
clayborg
approved these changes
Jan 11, 2025
lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
Show resolved
Hide resolved
lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
Outdated
Show resolved
Hide resolved
|
This is causing test failures for me: |
|
BaiXilin
pushed a commit
to BaiXilin/llvm-fix-vnni-instr-types
that referenced
this pull request
Jan 12, 2025
…lvm#120814) 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. --------- Co-authored-by: George Hu <[email protected]>
|
Same here, and the lldb debuginfod test is failing as well: |
|
Working on fixing the tests broke by this diff. |
|
@GeorgeHuyubo I am part of the Fuchsia toolchain team and we are seeing the same |
|
PR to revert the problematic commit: #122816 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR include two changes:
llvmcache-13267c5f5d2e3df472c133c8efa45fb3331ef1ea-liblzma.so.5.2.2.debuginfo.dwp
So it will provide more information in image list instead of a plain llvmcache-123