intel/llvm
1
0
Fork 0
mirror of https://github.com/intel/llvm.git synced 2026-01-17 06:40:01 +08:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Modify the localCache API to require an explicit commit on CachedFile… (#115331)"

Browse Source 
This reverts commit ce9e1d3c15.

The unittest added in this commit seems to be flaky causing random failure on buildbots:
 - https://lab.llvm.org/buildbot/#/builders/46/builds/13235
 - https://lab.llvm.org/buildbot/#/builders/46/builds/13232
 - https://lab.llvm.org/buildbot/#/builders/46/builds/13228
 - https://lab.llvm.org/buildbot/#/builders/46/builds/13224
 - https://lab.llvm.org/buildbot/#/builders/46/builds/13220
 - https://lab.llvm.org/buildbot/#/builders/46/builds/13210
 - https://lab.llvm.org/buildbot/#/builders/46/builds/13208
 - https://lab.llvm.org/buildbot/#/builders/46/builds/13207
 - https://lab.llvm.org/buildbot/#/builders/46/builds/13202
 - https://lab.llvm.org/buildbot/#/builders/46/builds/13196
and
 - https://lab.llvm.org/buildbot/#/builders/180/builds/14266
 - https://lab.llvm.org/buildbot/#/builders/180/builds/14254
 - https://lab.llvm.org/buildbot/#/builders/180/builds/14250
 - https://lab.llvm.org/buildbot/#/builders/180/builds/14245
 - https://lab.llvm.org/buildbot/#/builders/180/builds/14244
 - https://lab.llvm.org/buildbot/#/builders/180/builds/14226
This commit is contained in:
Douglas Yung
2025-03-08 23:54:57 +00:00
parent 0d209c6d90
commit 1d763f3833
10 changed files with 16 additions and 230 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
lldb/source/Core/DataFileCache.cpp
View File

@@ -132,11 +132,6 @@ bool DataFileCache::SetCachedData(llvm::StringRef key,
if (file_or_err) {
llvm::CachedFileStream *cfs = file_or_err->get();
cfs->OS->write((const char *)data.data(), data.size());
if (llvm::Error err = cfs->commit()) {
Log *log = GetLog(LLDBLog::Modules);
LLDB_LOG_ERROR(log, std::move(err),
"failed to commit to the cache for key: {0}");
}
return true;
} else {
Log *log = GetLog(LLDBLog::Modules);

21
llvm/include/llvm/Support/Caching.h
View File

@@ -24,32 +24,15 @@ class MemoryBuffer;
/// This class wraps an output stream for a file. Most clients should just be
/// able to return an instance of this base class from the stream callback, but
/// if a client needs to perform some action after the stream is written to,
/// that can be done by deriving from this class and overriding the destructor
/// or the commit() method.
/// that can be done by deriving from this class and overriding the destructor.
class CachedFileStream {
public:
CachedFileStream(std::unique_ptr<raw_pwrite_stream> OS,
std::string OSPath = "")
: OS(std::move(OS)), ObjectPathName(OSPath) {}
/// Must be called exactly once after the writes to OS have been completed
/// but before the CachedFileStream object is destroyed.
virtual Error commit() {
if (Committed)
return createStringError(make_error_code(std::errc::invalid_argument),
Twine("CacheStream already committed."));
Committed = true;
return Error::success();
}
bool Committed = false;
std::unique_ptr<raw_pwrite_stream> OS;
std::string ObjectPathName;
virtual ~CachedFileStream() {
if (!Committed)
report_fatal_error("CachedFileStream was not committed.\n");
}
virtual ~CachedFileStream() = default;
};
/// This type defines the callback to add a file that is generated on the fly.

3
llvm/lib/CGData/CodeGenData.cpp
View File

@@ -233,9 +233,6 @@ void saveModuleForTwoRounds(const Module &TheModule, unsigned Task,
WriteBitcodeToFile(TheModule, *Stream->OS,
/*ShouldPreserveUseListOrder=*/true);
if (Error Err = Stream->commit())
report_fatal_error(std::move(Err));
}
std::unique_ptr<Module> loadModuleForTwoRounds(BitcodeModule &OrigModule,

13
llvm/lib/Debuginfod/Debuginfod.cpp
View File

@@ -188,11 +188,6 @@ class StreamedHTTPResponseHandler : public HTTPResponseHandler {
public:
StreamedHTTPResponseHandler(CreateStreamFn CreateStream, HTTPClient &Client)
: CreateStream(CreateStream), Client(Client) {}
/// Must be called exactly once after the writes have been completed
/// but before the StreamedHTTPResponseHandler object is destroyed.
Error commit();
virtual ~StreamedHTTPResponseHandler() = default;
Error handleBodyChunk(StringRef BodyChunk) override;
@@ -215,12 +210,6 @@ Error StreamedHTTPResponseHandler::handleBodyChunk(StringRef BodyChunk) {
return Error::success();
}
Error StreamedHTTPResponseHandler::commit() {
if (FileStream)
return FileStream->commit();
return Error::success();
}
// An over-accepting simplification of the HTTP RFC 7230 spec.
static bool isHeader(StringRef S) {
StringRef Name;
@@ -309,8 +298,6 @@ Expected<std::string> getCachedOrDownloadArtifact(
Error Err = Client.perform(Request, Handler);
if (Err)
return std::move(Err);
if (Err = Handler.commit())
return std::move(Err);
unsigned Code = Client.responseCode();
if (Code && Code != 200)

3
llvm/lib/LTO/LTOBackend.cpp
View File

@@ -460,9 +460,6 @@ static void codegen(const Config &Conf, TargetMachine *TM,
if (DwoOut)
DwoOut->keep();
if (Error Err = Stream->commit())
report_fatal_error(std::move(Err));
}
static void splitCodeGen(const Config &C, TargetMachine *TM,

29
llvm/lib/Support/Caching.cpp
View File

@@ -88,10 +88,9 @@ Expected<FileCache> llvm::localCache(const Twine &CacheNameRef,
AddBuffer(std::move(AddBuffer)), TempFile(std::move(TempFile)),
ModuleName(ModuleName), Task(Task) {}
Error commit() override {
Error E = CachedFileStream::commit();
if (E)
return E;
~CacheStream() {
// TODO: Manually commit rather than using non-trivial destructor,
// allowing to replace report_fatal_errors with a return Error.
// Make sure the stream is closed before committing it.
OS.reset();
@@ -101,12 +100,10 @@ Expected<FileCache> llvm::localCache(const Twine &CacheNameRef,
MemoryBuffer::getOpenFile(
sys::fs::convertFDToNativeFile(TempFile.FD), ObjectPathName,
/*FileSize=*/-1, /*RequiresNullTerminator=*/false);
if (!MBOrErr) {
std::error_code EC = MBOrErr.getError();
return createStringError(EC, Twine("Failed to open new cache file ") +
TempFile.TmpName + ": " +
EC.message() + "\n");
}
if (!MBOrErr)
report_fatal_error(Twine("Failed to open new cache file ") +
TempFile.TmpName + ": " +
MBOrErr.getError().message() + "\n");
// On POSIX systems, this will atomically replace the destination if
// it already exists. We try to emulate this on Windows, but this may
@@ -117,14 +114,11 @@ Expected<FileCache> llvm::localCache(const Twine &CacheNameRef,
// AddBuffer a copy of the bytes we wrote in that case. We do this
// instead of just using the existing file, because the pruner might
// delete the file before we get a chance to use it.
E = TempFile.keep(ObjectPathName);
Error E = TempFile.keep(ObjectPathName);
E = handleErrors(std::move(E), [&](const ECError &E) -> Error {
std::error_code EC = E.convertToErrorCode();
if (EC != errc::permission_denied)
return createStringError(
EC, Twine("Failed to rename temporary file ") +
TempFile.TmpName + " to " + ObjectPathName + ": " +
EC.message() + "\n");
return errorCodeToError(EC);
auto MBCopy = MemoryBuffer::getMemBufferCopy((*MBOrErr)->getBuffer(),
ObjectPathName);
@@ -137,10 +131,11 @@ Expected<FileCache> llvm::localCache(const Twine &CacheNameRef,
});
if (E)
return E;
report_fatal_error(Twine("Failed to rename temporary file ") +
TempFile.TmpName + " to " + ObjectPathName + ": " +
toString(std::move(E)) + "\n");
AddBuffer(Task, ModuleName, std::move(*MBOrErr));
return Error::success();
}
};

4
llvm/tools/gold/gold-plugin.cpp
View File

@@ -1119,9 +1119,7 @@ static std::vector<std::pair<SmallString<128>, bool>> runLTO() {
auto AddBuffer = [&](size_t Task, const Twine &moduleName,
std::unique_ptr<MemoryBuffer> MB) {
auto Stream = *AddStream(Task, ModuleName);
Stream->OS << MB->getBuffer();
check(Stream->commit(), "Failed to commit cache");
*AddStream(Task, moduleName)->OS << MB->getBuffer();
};
FileCache Cache;

4
llvm/tools/llvm-lto2/llvm-lto2.cpp
View File

@@ -448,9 +448,7 @@ static int run(int argc, char **argv) {
auto AddBuffer = [&](size_t Task, const Twine &ModuleName,
std::unique_ptr<MemoryBuffer> MB) {
auto Stream = AddStream(Task, ModuleName);
*Stream->OS << MB->getBuffer();
check(Stream->commit(), "Failed to commit cache");
*AddStream(Task, ModuleName)->OS << MB->getBuffer();
};
FileCache Cache;

1
llvm/unittests/Support/CMakeLists.txt
View File

@@ -18,7 +18,6 @@ add_llvm_unittest(SupportTests
BranchProbabilityTest.cpp
CachePruningTest.cpp
CrashRecoveryTest.cpp
Caching.cpp
Casting.cpp
CheckedArithmeticTest.cpp
Chrono.cpp

163
llvm/unittests/Support/Caching.cpp
View File

@@ -1,163 +0,0 @@
//===- Caching.cpp --------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
#include "llvm/Support/Caching.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Testing/Support/Error.h"
#include "gtest/gtest.h"
using namespace llvm;
#define ASSERT_NO_ERROR(x) \
if (std::error_code ASSERT_NO_ERROR_ec = x) { \
SmallString<128> MessageStorage; \
raw_svector_ostream Message(MessageStorage); \
Message << #x ": did not return errc::success.\n" \
<< "error number: " << ASSERT_NO_ERROR_ec.value() << "\n" \
<< "error message: " << ASSERT_NO_ERROR_ec.message() << "\n"; \
GTEST_FATAL_FAILURE_(MessageStorage.c_str()); \
} else { \
}
char data[] = "some data";
TEST(Caching, Normal) {
SmallString<256> TempDir;
sys::path::system_temp_directory(true, TempDir);
SmallString<256> CacheDir;
sys::path::append(CacheDir, TempDir, "llvm_test_cache");
sys::fs::remove_directories(CacheDir.str());
std::unique_ptr<MemoryBuffer> CachedBuffer;
auto AddBuffer = [&CachedBuffer](unsigned Task, const Twine &ModuleName,
std::unique_ptr<MemoryBuffer> M) {
CachedBuffer = std::move(M);
};
auto CacheOrErr =
localCache("LLVMTestCache", "LLVMTest", CacheDir, AddBuffer);
ASSERT_TRUE(bool(CacheOrErr));
FileCache &Cache = *CacheOrErr;
{
auto AddStreamOrErr = Cache(1, "foo", "");
ASSERT_TRUE(bool(AddStreamOrErr));
AddStreamFn &AddStream = *AddStreamOrErr;
ASSERT_TRUE(AddStream);
auto FileOrErr = AddStream(1, "");
ASSERT_TRUE(bool(FileOrErr));
CachedFileStream *CFS = FileOrErr->get();
(*CFS->OS).write(data, sizeof(data));
ASSERT_THAT_ERROR(CFS->commit(), Succeeded());
}
{
auto AddStreamOrErr = Cache(1, "foo", "");
ASSERT_TRUE(bool(AddStreamOrErr));
AddStreamFn &AddStream = *AddStreamOrErr;
ASSERT_FALSE(AddStream);
ASSERT_TRUE(CachedBuffer->getBufferSize() == sizeof(data));
StringRef readData = CachedBuffer->getBuffer();
ASSERT_TRUE(memcmp(data, readData.data(), sizeof(data)) == 0);
}
ASSERT_NO_ERROR(sys::fs::remove_directories(CacheDir.str()));
}
TEST(Caching, WriteAfterCommit) {
SmallString<256> TempDir;
sys::path::system_temp_directory(true, TempDir);
SmallString<256> CacheDir;
sys::path::append(CacheDir, TempDir, "llvm_test_cache");
sys::fs::remove_directories(CacheDir.str());
std::unique_ptr<MemoryBuffer> CachedBuffer;
auto AddBuffer = [&CachedBuffer](unsigned Task, const Twine &ModuleName,
std::unique_ptr<MemoryBuffer> M) {
CachedBuffer = std::move(M);
};
auto CacheOrErr =
localCache("LLVMTestCache", "LLVMTest", CacheDir, AddBuffer);
ASSERT_TRUE(bool(CacheOrErr));
FileCache &Cache = *CacheOrErr;
auto AddStreamOrErr = Cache(1, "foo", "");
ASSERT_TRUE(bool(AddStreamOrErr));
AddStreamFn &AddStream = *AddStreamOrErr;
ASSERT_TRUE(AddStream);
auto FileOrErr = AddStream(1, "");
ASSERT_TRUE(bool(FileOrErr));
CachedFileStream *CFS = FileOrErr->get();
(*CFS->OS).write(data, sizeof(data));
ASSERT_THAT_ERROR(CFS->commit(), Succeeded());
EXPECT_DEATH(
{ (*CFS->OS).write(data, sizeof(data)); }, "")
<< "Write after commit did not cause abort";
ASSERT_NO_ERROR(sys::fs::remove_directories(CacheDir.str()));
}
TEST(Caching, NoCommit) {
SmallString<256> TempDir;
sys::path::system_temp_directory(true, TempDir);
SmallString<256> CacheDir;
sys::path::append(CacheDir, TempDir, "llvm_test_cache");
sys::fs::remove_directories(CacheDir.str());
std::unique_ptr<MemoryBuffer> CachedBuffer;
auto AddBuffer = [&CachedBuffer](unsigned Task, const Twine &ModuleName,
std::unique_ptr<MemoryBuffer> M) {
CachedBuffer = std::move(M);
};
auto CacheOrErr =
localCache("LLVMTestCache", "LLVMTest", CacheDir, AddBuffer);
ASSERT_TRUE(bool(CacheOrErr));
FileCache &Cache = *CacheOrErr;
auto AddStreamOrErr = Cache(1, "foo", "");
ASSERT_TRUE(bool(AddStreamOrErr));
AddStreamFn &AddStream = *AddStreamOrErr;
ASSERT_TRUE(AddStream);
auto FileOrErr = AddStream(1, "");
ASSERT_TRUE(bool(FileOrErr));
CachedFileStream *CFS = FileOrErr->get();
(*CFS->OS).write(data, sizeof(data));
ASSERT_THAT_ERROR(CFS->commit(), Succeeded());
EXPECT_DEATH(
{
auto FileOrErr = AddStream(1, "");
ASSERT_TRUE(bool(FileOrErr));
CachedFileStream *CFS = FileOrErr->get();
(*CFS->OS).write(data, sizeof(data));
},
"")
<< "destruction without commit did not cause error";
ASSERT_NO_ERROR(sys::fs::remove_directories(CacheDir.str()));
}