From 1ed5a2ba82f1d9f6c135cf3feca8427cb2e39e39 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Mon, 23 Jan 2023 11:00:47 -0800 Subject: [PATCH 1/3] [VirtualOutput] Add option to only overwrite output if content is different Add new output configuration that checks if Output.keep() will change the file content and skip overwriting when the content are the same. This avoids updating file status for the output file like timestamps. --- .../llvm/Support/VirtualOutputConfig.def | 2 + llvm/lib/Support/VirtualOutputBackends.cpp | 102 ++++++++++++++++++ .../Support/VirtualOutputBackendsTest.cpp | 41 +++++++ .../Support/VirtualOutputConfigTest.cpp | 2 + 4 files changed, 147 insertions(+) diff --git a/llvm/include/llvm/Support/VirtualOutputConfig.def b/llvm/include/llvm/Support/VirtualOutputConfig.def index bf9159ec22eb8..9b88448887e88 100644 --- a/llvm/include/llvm/Support/VirtualOutputConfig.def +++ b/llvm/include/llvm/Support/VirtualOutputConfig.def @@ -19,5 +19,7 @@ HANDLE_OUTPUT_CONFIG_FLAG(CRLF, false) // OF_CRLF. HANDLE_OUTPUT_CONFIG_FLAG(DiscardOnSignal, true) // E.g., RemoveFileOnSignal. HANDLE_OUTPUT_CONFIG_FLAG(AtomicWrite, true) // E.g., use temporaries. HANDLE_OUTPUT_CONFIG_FLAG(ImplyCreateDirectories, true) +// Skip atomic write if existing file content is the same +HANDLE_OUTPUT_CONFIG_FLAG(OnlyIfDifferent, false) #undef HANDLE_OUTPUT_CONFIG_FLAG diff --git a/llvm/lib/Support/VirtualOutputBackends.cpp b/llvm/lib/Support/VirtualOutputBackends.cpp index 6b90c4db9d5bb..e57282a2973df 100644 --- a/llvm/lib/Support/VirtualOutputBackends.cpp +++ b/llvm/lib/Support/VirtualOutputBackends.cpp @@ -337,6 +337,89 @@ Error OnDiskOutputFile::initializeStream() { return Error::success(); } +namespace { +class OpenFileRAII { + static const int InvalidFd = -1; + +public: + int Fd = InvalidFd; + + ~OpenFileRAII() { + if (Fd != InvalidFd) + llvm::sys::Process::SafelyCloseFileDescriptor(Fd); + } +}; + +enum class FileDifference : uint8_t { + /// The source and destination paths refer to the exact same file. + IdenticalFile, + /// The source and destination paths refer to separate files with identical + /// contents. + SameContents, + /// The source and destination paths refer to separate files with different + /// contents. + DifferentContents +}; +} // end anonymous namespace + +static Expected +areFilesDifferent(const llvm::Twine &Source, const llvm::Twine &Destination) { + if (sys::fs::equivalent(Source, Destination)) + return FileDifference::IdenticalFile; + + OpenFileRAII SourceFile; + sys::fs::file_status SourceStatus; + // If we can't open the source file, fail. + if (std::error_code EC = sys::fs::openFileForRead(Source, SourceFile.Fd)) + return convertToOutputError(Source, EC); + + // If we can't stat the source file, fail. + if (std::error_code EC = sys::fs::status(SourceFile.Fd, SourceStatus)) + return convertToOutputError(Source, EC); + + OpenFileRAII DestFile; + sys::fs::file_status DestStatus; + // If we can't open the destination file, report different. + if (std::error_code Error = + sys::fs::openFileForRead(Destination, DestFile.Fd)) + return FileDifference::DifferentContents; + + // If we can't open the destination file, report different. + if (std::error_code Error = sys::fs::status(DestFile.Fd, DestStatus)) + return FileDifference::DifferentContents; + + // If the files are different sizes, they must be different. + uint64_t Size = SourceStatus.getSize(); + if (Size != DestStatus.getSize()) + return FileDifference::DifferentContents; + + // If both files are zero size, they must be the same. + if (Size == 0) + return FileDifference::SameContents; + + // The two files match in size, so we have to compare the bytes to determine + // if they're the same. + std::error_code SourceRegionErr; + sys::fs::mapped_file_region SourceRegion( + sys::fs::convertFDToNativeFile(SourceFile.Fd), + sys::fs::mapped_file_region::readonly, Size, 0, SourceRegionErr); + if (SourceRegionErr) + return convertToOutputError(Source, SourceRegionErr); + + std::error_code DestRegionErr; + sys::fs::mapped_file_region DestRegion( + sys::fs::convertFDToNativeFile(DestFile.Fd), + sys::fs::mapped_file_region::readonly, Size, 0, DestRegionErr); + + if (DestRegionErr) + return FileDifference::DifferentContents; + + if (memcmp(SourceRegion.const_data(), DestRegion.const_data(), Size) != 0) + return FileDifference::DifferentContents; + + return FileDifference::SameContents; +} + Error OnDiskOutputFile::keep() { // Destroy the streams to flush them. BufferOS.reset(); @@ -351,6 +434,25 @@ Error OnDiskOutputFile::keep() { if (!TempPath) return Error::success(); + if (Config.getOnlyIfDifferent()) { + auto Result = areFilesDifferent(*TempPath, OutputPath); + if (!Result) + return Result.takeError(); + switch (*Result) { + case FileDifference::IdenticalFile: + // Do nothing for a self-move. + return Error::success(); + + case FileDifference::SameContents: + // Files are identical; remove the source file. + (void) sys::fs::remove(*TempPath); + return Error::success(); + + case FileDifference::DifferentContents: + break; // Rename the file. + } + } + // Move temporary to the final output path and remove it if that fails. std::error_code RenameEC = sys::fs::rename(*TempPath, OutputPath); if (!RenameEC) diff --git a/llvm/unittests/Support/VirtualOutputBackendsTest.cpp b/llvm/unittests/Support/VirtualOutputBackendsTest.cpp index 1055f448a349c..2369a5937e413 100644 --- a/llvm/unittests/Support/VirtualOutputBackendsTest.cpp +++ b/llvm/unittests/Support/VirtualOutputBackendsTest.cpp @@ -765,4 +765,45 @@ TEST(VirtualOutputBackendAdaptors, makeMirroringOutputBackendCreateError) { FailedWithMessage(Error1->Msg)); } +TEST(OnDiskBackendTest, OnlyIfDifferent) { + OnDiskOutputBackendProvider Provider; + auto Backend = Provider.createBackend(); + std::string FilePath = Provider.getFilePathToCreate(); + StringRef Data = "some data"; + OutputConfig Config = OutputConfig().setOnlyIfDifferent(); + + OutputFile O1, O2, O3; + sys::fs::file_status Status1, Status2, Status3; + // Write first file. + EXPECT_THAT_ERROR(Backend->createFile(FilePath, Config).moveInto(O1), + Succeeded()); + O1 << Data; + EXPECT_THAT_ERROR(O1.keep(), Succeeded()); + EXPECT_FALSE(O1.isOpen()); + EXPECT_FALSE(sys::fs::status(FilePath, Status1, /*follow=*/false)); + + // Write second with same content. + EXPECT_THAT_ERROR(Backend->createFile(FilePath, Config).moveInto(O2), + Succeeded()); + O2 << Data; + EXPECT_THAT_ERROR(O2.keep(), Succeeded()); + EXPECT_FALSE(O2.isOpen()); + EXPECT_FALSE(sys::fs::status(FilePath, Status2, /*follow=*/false)); + + // Make sure the output path file is not modified with same content. + EXPECT_EQ(Status1.getUniqueID(), Status2.getUniqueID()); + + // Write third with different content. + EXPECT_THAT_ERROR(Backend->createFile(FilePath, Config).moveInto(O3), + Succeeded()); + O3 << Data << "\n"; + EXPECT_THAT_ERROR(O3.keep(), Succeeded()); + EXPECT_FALSE(O3.isOpen()); + EXPECT_FALSE(sys::fs::status(FilePath, Status3, /*follow=*/false)); + + // This should overwrite the file and create a different UniqueID. + EXPECT_NE(Status1.getUniqueID(), Status3.getUniqueID()); +} + + } // end namespace diff --git a/llvm/unittests/Support/VirtualOutputConfigTest.cpp b/llvm/unittests/Support/VirtualOutputConfigTest.cpp index 11e0935e9e25a..f5545c10ec752 100644 --- a/llvm/unittests/Support/VirtualOutputConfigTest.cpp +++ b/llvm/unittests/Support/VirtualOutputConfigTest.cpp @@ -22,6 +22,7 @@ TEST(VirtualOutputConfigTest, construct) { EXPECT_TRUE(OutputConfig().getDiscardOnSignal()); EXPECT_TRUE(OutputConfig().getAtomicWrite()); EXPECT_TRUE(OutputConfig().getImplyCreateDirectories()); + EXPECT_FALSE(OutputConfig().getOnlyIfDifferent()); // Test inverted defaults. EXPECT_TRUE(OutputConfig().getNoText()); @@ -29,6 +30,7 @@ TEST(VirtualOutputConfigTest, construct) { EXPECT_FALSE(OutputConfig().getNoDiscardOnSignal()); EXPECT_FALSE(OutputConfig().getNoAtomicWrite()); EXPECT_FALSE(OutputConfig().getNoImplyCreateDirectories()); + EXPECT_TRUE(OutputConfig().getNoOnlyIfDifferent()); } TEST(VirtualOutputConfigTest, set) { From 6984b3480df812949a521be61c64d66b29176490 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 24 Jan 2023 11:11:52 -0800 Subject: [PATCH 2/3] [VirtualOutputBackend] Add HashingOutputBackend Add a hashing outputbackend that can hash the outputs for sanity check. --- .../llvm/Support/HashingOutputBackend.h | 114 ++++++++++++++++++ .../Support/VirtualOutputBackendsTest.cpp | 34 ++++++ 2 files changed, 148 insertions(+) create mode 100644 llvm/include/llvm/Support/HashingOutputBackend.h diff --git a/llvm/include/llvm/Support/HashingOutputBackend.h b/llvm/include/llvm/Support/HashingOutputBackend.h new file mode 100644 index 0000000000000..2c760515e99be --- /dev/null +++ b/llvm/include/llvm/Support/HashingOutputBackend.h @@ -0,0 +1,114 @@ +//===- HashingOutputBackends.h - Hashing output backends --------*- C++ -*-===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_SUPPORT_HASHINGOUTPUTBACKEND_H +#define LLVM_SUPPORT_HASHINGOUTPUTBACKEND_H + +#include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringMap.h" +#include "llvm/Support/Endian.h" +#include "llvm/Support/HashBuilder.h" +#include "llvm/Support/VirtualOutputBackend.h" +#include "llvm/Support/VirtualOutputConfig.h" +#include "llvm/Support/raw_ostream.h" + +namespace llvm { +namespace vfs { + +/// raw_pwrite_stream that writes to a hasher. +template +class HashingStream : public llvm::raw_pwrite_stream { +private: + SmallVector Buffer; + raw_svector_ostream OS; + + using HashBuilderT = HashBuilder; + HashBuilderT Builder; + + void write_impl(const char *Ptr, size_t Size) override { + OS.write(Ptr, Size); + } + + void pwrite_impl(const char *Ptr, size_t Size, uint64_t Offset) override { + OS.pwrite(Ptr, Size, Offset); + } + + uint64_t current_pos() const override { return OS.str().size(); } + +public: + HashingStream() : OS(Buffer) { SetUnbuffered(); } + + auto final() { + Builder.update(OS.str()); + return Builder.final(); + } +}; + +template class HashingOutputFile; + +/// An output backend that only generates the hash for outputs. +template class HashingOutputBackend : public OutputBackend { +private: + friend class HashingOutputFile; + void addOutputFile(StringRef Path, StringRef Hash) { + OutputHashes[Path] = std::string(Hash); + } + +protected: + IntrusiveRefCntPtr cloneImpl() const override { + return const_cast *>(this); + } + + Expected> + createFileImpl(StringRef Path, Optional Config) override { + return std::make_unique>(Path, *this); + } + +public: + /// Iterator for all the output file names. + auto outputFiles() const { return OutputHashes.keys(); } + + /// Get hash value for the output files in hex representation. + /// Return None if the requested path is not generated. + Optional getHashValueForFile(StringRef Path) { + auto F = OutputHashes.find(Path); + if (F == OutputHashes.end()) + return None; + return toHex(F->second); + } + +private: + StringMap OutputHashes; +}; + +/// HashingOutputFile. +template +class HashingOutputFile final : public OutputFileImpl { +public: + Error keep() override { + auto Result = OS.final(); + Backend.addOutputFile(OutputPath, toStringRef(Result)); + return Error::success(); + } + Error discard() override { return Error::success(); } + raw_pwrite_stream &getOS() override { return OS; } + + HashingOutputFile(StringRef OutputPath, + HashingOutputBackend &Backend) + : OutputPath(OutputPath.str()), Backend(Backend) {} + +private: + const std::string OutputPath; + HashingStream OS; + HashingOutputBackend &Backend; +}; + +} // namespace vfs +} // namespace llvm + +#endif // LLVM_SUPPORT_HASHINGOUTPUTBACKEND_H diff --git a/llvm/unittests/Support/VirtualOutputBackendsTest.cpp b/llvm/unittests/Support/VirtualOutputBackendsTest.cpp index 2369a5937e413..60c6c458c15fc 100644 --- a/llvm/unittests/Support/VirtualOutputBackendsTest.cpp +++ b/llvm/unittests/Support/VirtualOutputBackendsTest.cpp @@ -8,6 +8,8 @@ #include "llvm/Support/VirtualOutputBackends.h" #include "llvm/ADT/StringMap.h" +#include "llvm/Support/BLAKE3.h" +#include "llvm/Support/HashingOutputBackend.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" @@ -805,5 +807,37 @@ TEST(OnDiskBackendTest, OnlyIfDifferent) { EXPECT_NE(Status1.getUniqueID(), Status3.getUniqueID()); } +TEST(HashingBackendTest, HashOutput) { + HashingOutputBackend Backend; + OutputFile O1, O2, O3, O4, O5; + EXPECT_THAT_ERROR(Backend.createFile("file1").moveInto(O1), Succeeded()); + O1 << "some data"; + EXPECT_THAT_ERROR(O1.keep(), Succeeded()); + EXPECT_THAT_ERROR(Backend.createFile("file2").moveInto(O2), Succeeded()); + O2 << "some data"; + EXPECT_THAT_ERROR(O2.keep(), Succeeded()); + EXPECT_EQ(Backend.getHashValueForFile("file1"), + Backend.getHashValueForFile("file2")); + + EXPECT_THAT_ERROR(Backend.createFile("file3").moveInto(O3), Succeeded()); + O3 << "some "; + O3 << "data"; + EXPECT_THAT_ERROR(O3.keep(), Succeeded()); + EXPECT_EQ(Backend.getHashValueForFile("file1"), + Backend.getHashValueForFile("file3")); + + EXPECT_THAT_ERROR(Backend.createFile("file4").moveInto(O4), Succeeded()); + O4 << "same data"; + O4.getOS().pwrite("o", 1, 1); + EXPECT_THAT_ERROR(O4.keep(), Succeeded()); + EXPECT_EQ(Backend.getHashValueForFile("file1"), + Backend.getHashValueForFile("file4")); + + EXPECT_THAT_ERROR(Backend.createFile("file5").moveInto(O5), Succeeded()); + O5 << "different data"; + EXPECT_THAT_ERROR(O5.keep(), Succeeded()); + EXPECT_NE(Backend.getHashValueForFile("file1"), + Backend.getHashValueForFile("file5")); +} } // end namespace From 9db2c355f2ec3a9fb82629f70375aa0b3e4c1f0a Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Wed, 25 Jan 2023 18:02:42 -0800 Subject: [PATCH 3/3] [lldb] Fix lldb build after swift ASTContext change --- lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp index 4862848250398..dbe661052ecf2 100644 --- a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp +++ b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp @@ -2907,7 +2907,7 @@ swift::ASTContext *SwiftASTContext::GetASTContext() { GetLanguageOptions(), GetTypeCheckerOptions(), GetSILOptions(), GetSearchPathOptions(), GetClangImporterOptions(), GetSymbolGraphOptions(), GetSourceManager(), GetDiagnosticEngine(), - ReportModuleLoadingProgress)); + /*OutputBackend=*/nullptr, ReportModuleLoadingProgress)); m_diagnostic_consumer_ap.reset(new StoringDiagnosticConsumer(*this)); if (getenv("LLDB_SWIFT_DUMP_DIAGS")) {