Skip to content

Commit

Permalink
Revert "[lldb] Switch debuginfod cache to use lldb index cache settin…
Browse files Browse the repository at this point in the history
…gs" (llvm#122816)

This reverts commit 7b808e7.

Previous commit which change default debuginfod cache path and pruning
policy settings is problematic. It broke multiple tests across lldb and
llvm. Reverting for now.

Co-authored-by: George Hu <[email protected]>
  • Loading branch information
GeorgeHuyubo and George Hu authored Jan 14, 2025
1 parent 2201164 commit 1908c41
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

#include "SymbolLocatorDebuginfod.h"

#include "lldb/Core/DataFileCache.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Interpreter/OptionValueString.h"
#include "lldb/Utility/Args.h"
Expand Down Expand Up @@ -173,14 +172,11 @@ GetFileForModule(const ModuleSpec &module_spec,
// Grab LLDB's Debuginfod overrides from the
// plugin.symbol-locator.debuginfod.* settings.
PluginProperties &plugin_props = GetGlobalPluginProperties();
// 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::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;
llvm::SmallVector<llvm::StringRef> debuginfod_urls =
llvm::getDefaultDebuginfodUrls();
std::chrono::milliseconds timeout = plugin_props.GetTimeout();
Expand All @@ -193,8 +189,7 @@ GetFileForModule(const ModuleSpec &module_spec,
if (!file_name.empty())
cache_file_name += "-" + file_name.str();
llvm::Expected<std::string> result = llvm::getCachedOrDownloadArtifact(
cache_file_name, url_path, cache_path, debuginfod_urls, timeout,
pruning_policy);
cache_file_name, url_path, cache_path, debuginfod_urls, timeout);
if (result)
return FileSpec(*result);

Expand Down
4 changes: 1 addition & 3 deletions llvm/include/llvm/Debuginfod/Debuginfod.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#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"
Expand Down Expand Up @@ -96,8 +95,7 @@ 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,
llvm::CachePruningPolicy policy);
ArrayRef<StringRef> DebuginfodUrls, std::chrono::milliseconds Timeout);

class ThreadPoolInterface;

Expand Down
29 changes: 9 additions & 20 deletions llvm/lib/Debuginfod/Debuginfod.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,6 @@ 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");
Expand Down Expand Up @@ -177,15 +169,9 @@ Expected<std::string> getCachedOrDownloadArtifact(StringRef UniqueKey,
return CacheDirOrErr.takeError();
CacheDir = *CacheDirOrErr;

Expected<llvm::CachePruningPolicy> PruningPolicyOrErr =
getDefaultDebuginfodCachePruningPolicy();
if (!PruningPolicyOrErr)
return PruningPolicyOrErr.takeError();
llvm::CachePruningPolicy PruningPolicy = *PruningPolicyOrErr;

return getCachedOrDownloadArtifact(
UniqueKey, UrlPath, CacheDir, getDefaultDebuginfodUrls(),
getDefaultDebuginfodTimeout(), PruningPolicy);
return getCachedOrDownloadArtifact(UniqueKey, UrlPath, CacheDir,
getDefaultDebuginfodUrls(),
getDefaultDebuginfodTimeout());
}

namespace {
Expand Down Expand Up @@ -264,8 +250,7 @@ static SmallVector<std::string, 0> getHeaders() {

Expected<std::string> getCachedOrDownloadArtifact(
StringRef UniqueKey, StringRef UrlPath, StringRef CacheDirectoryPath,
ArrayRef<StringRef> DebuginfodUrls, std::chrono::milliseconds Timeout,
llvm::CachePruningPolicy policy) {
ArrayRef<StringRef> DebuginfodUrls, std::chrono::milliseconds Timeout) {
SmallString<64> AbsCachedArtifactPath;
sys::path::append(AbsCachedArtifactPath, CacheDirectoryPath,
"llvmcache-" + UniqueKey);
Expand Down Expand Up @@ -319,7 +304,11 @@ Expected<std::string> getCachedOrDownloadArtifact(
continue;
}

pruneCache(CacheDirectoryPath, policy);
Expected<CachePruningPolicy> PruningPolicyOrErr =
parseCachePruningPolicy(std::getenv("DEBUGINFOD_CACHE_POLICY"));
if (!PruningPolicyOrErr)
return PruningPolicyOrErr.takeError();
pruneCache(CacheDirectoryPath, *PruningPolicyOrErr);

// Return the path to the artifact on disk.
return std::string(AbsCachedArtifactPath);
Expand Down
3 changes: 1 addition & 2 deletions llvm/unittests/Debuginfod/DebuginfodTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,14 @@ 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";
OF << CacheDir << "\n";
OF.close();
Expected<std::string> PathOrErr = getCachedOrDownloadArtifact(
UniqueKey, /*UrlPath=*/"/null", CacheDir,
/*DebuginfodUrls=*/{}, /*Timeout=*/std::chrono::milliseconds(1), policy);
/*DebuginfodUrls=*/{}, /*Timeout=*/std::chrono::milliseconds(1));
EXPECT_THAT_EXPECTED(PathOrErr, HasValue(CachedFilePath));
}

Expand Down

0 comments on commit 1908c41

Please sign in to comment.