From 345223a7be3c3c93a10f8453d96287a3a03b6754 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Tue, 7 Dec 2021 12:42:13 -0800 Subject: [PATCH] Support: Extract sys::fs::readNativeFileToEOF() from MemoryBuffer Extract the `readNativeFile()` loop from `MemoryBuffer::getMemoryBufferForStream()` into `readNativeFileToEOF()` to allow reuse. The chunk size is configurable; the default of `4*4096` is exposed as `sys::fs::DefaultReadChunkSize` to allow sizing of SmallVectors. There's somewhere I'd like to read a usually-small file without overhead of a MemoryBuffer; extracting existing logic rather than duplicating it. Differential Revision: https://reviews.llvm.org/D115397 --- llvm/include/llvm/Support/FileSystem.h | 19 ++++++++++++++ llvm/lib/Support/MemoryBuffer.cpp | 20 +++------------ llvm/lib/Support/Path.cpp | 20 +++++++++++++++ llvm/unittests/Support/Path.cpp | 47 ++++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 17 deletions(-) diff --git a/llvm/include/llvm/Support/FileSystem.h b/llvm/include/llvm/Support/FileSystem.h index dabd384..0334829 100644 --- a/llvm/include/llvm/Support/FileSystem.h +++ b/llvm/include/llvm/Support/FileSystem.h @@ -1014,6 +1014,25 @@ file_t getStderrHandle(); /// @returns The number of bytes read, or error. Expected readNativeFile(file_t FileHandle, MutableArrayRef Buf); +/// Default chunk size for \a readNativeFileToEOF(). +enum : size_t { DefaultReadChunkSize = 4 * 4096 }; + +/// Reads from \p FileHandle until EOF, appending to \p Buffer in chunks of +/// size \p ChunkSize. +/// +/// This calls \a readNativeFile() in a loop. On Error, previous chunks that +/// were read successfully are left in \p Buffer and returned. +/// +/// Note: For reading the final chunk at EOF, \p Buffer's capacity needs extra +/// storage of \p ChunkSize. +/// +/// \param FileHandle File to read from. +/// \param Buffer Where to put the file content. +/// \param ChunkSize Size of chunks. +/// \returns The error if EOF was not found. +Error readNativeFileToEOF(file_t FileHandle, SmallVectorImpl &Buffer, + ssize_t ChunkSize = DefaultReadChunkSize); + /// Reads \p Buf.size() bytes from \p FileHandle at offset \p Offset into \p /// Buf. If 'pread' is available, this will use that, otherwise it will use /// 'lseek'. Returns the number of bytes actually read. Returns 0 when reaching diff --git a/llvm/lib/Support/MemoryBuffer.cpp b/llvm/lib/Support/MemoryBuffer.cpp index 345b0d4..1bbdafd 100644 --- a/llvm/lib/Support/MemoryBuffer.cpp +++ b/llvm/lib/Support/MemoryBuffer.cpp @@ -227,23 +227,9 @@ public: static ErrorOr> getMemoryBufferForStream(sys::fs::file_t FD, const Twine &BufferName) { - const ssize_t ChunkSize = 4096*4; - SmallString Buffer; - - // Read into Buffer until we hit EOF. - size_t Size = Buffer.size(); - for (;;) { - Buffer.resize_for_overwrite(Size + ChunkSize); - Expected ReadBytes = sys::fs::readNativeFile( - FD, makeMutableArrayRef(Buffer.begin() + Size, ChunkSize)); - if (!ReadBytes) - return errorToErrorCode(ReadBytes.takeError()); - if (*ReadBytes == 0) - break; - Size += *ReadBytes; - } - Buffer.truncate(Size); - + SmallString Buffer; + if (Error E = sys::fs::readNativeFileToEOF(FD, Buffer)) + return errorToErrorCode(std::move(E)); return getMemBufferCopyImpl(Buffer, BufferName); } diff --git a/llvm/lib/Support/Path.cpp b/llvm/lib/Support/Path.cpp index 7c99d08..2f80cdb 100644 --- a/llvm/lib/Support/Path.cpp +++ b/llvm/lib/Support/Path.cpp @@ -12,6 +12,7 @@ #include "llvm/Support/Path.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/Config/config.h" #include "llvm/Config/llvm-config.h" #include "llvm/Support/Endian.h" @@ -1167,6 +1168,25 @@ const char *mapped_file_region::const_data() const { return reinterpret_cast(Mapping); } +Error readNativeFileToEOF(file_t FileHandle, SmallVectorImpl &Buffer, + ssize_t ChunkSize) { + // Install a handler to truncate the buffer to the correct size on exit. + size_t Size = Buffer.size(); + auto TruncateOnExit = make_scope_exit([&]() { Buffer.truncate(Size); }); + + // Read into Buffer until we hit EOF. + for (;;) { + Buffer.resize_for_overwrite(Size + ChunkSize); + Expected ReadBytes = readNativeFile( + FileHandle, makeMutableArrayRef(Buffer.begin() + Size, ChunkSize)); + if (!ReadBytes) + return ReadBytes.takeError(); + if (*ReadBytes == 0) + return Error::success(); + Size += *ReadBytes; + } +} + } // end namespace fs } // end namespace sys } // end namespace llvm diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index 35c1de7..e3fa5d0 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -1949,6 +1949,53 @@ TEST_F(FileSystemTest, readNativeFile) { EXPECT_THAT_EXPECTED(Read(6), HasValue("01234")); } +TEST_F(FileSystemTest, readNativeFileToEOF) { + constexpr StringLiteral Content = "0123456789"; + createFileWithData(NonExistantFile, false, fs::CD_CreateNew, Content); + FileRemover Cleanup(NonExistantFile); + const auto &Read = [&](SmallVectorImpl &V, + Optional ChunkSize) { + Expected FD = fs::openNativeFileForRead(NonExistantFile); + if (!FD) + return FD.takeError(); + auto Close = make_scope_exit([&] { fs::closeFile(*FD); }); + if (ChunkSize) + return fs::readNativeFileToEOF(*FD, V, *ChunkSize); + return fs::readNativeFileToEOF(*FD, V); + }; + + // Check basic operation. + { + SmallString<0> NoSmall; + SmallString StaysSmall; + SmallVectorImpl *Vectors[] = { + static_cast *>(&NoSmall), + static_cast *>(&StaysSmall), + }; + for (SmallVectorImpl *V : Vectors) { + ASSERT_THAT_ERROR(Read(*V, None), Succeeded()); + ASSERT_EQ(Content, StringRef(V->begin(), V->size())); + } + ASSERT_EQ(fs::DefaultReadChunkSize + Content.size(), StaysSmall.capacity()); + + // Check appending. + { + constexpr StringLiteral Prefix = "prefix-"; + for (SmallVectorImpl *V : Vectors) { + V->assign(Prefix.begin(), Prefix.end()); + ASSERT_THAT_ERROR(Read(*V, None), Succeeded()); + ASSERT_EQ((Prefix + Content).str(), StringRef(V->begin(), V->size())); + } + } + } + + // Check that the chunk size (if specified) is respected. + SmallString SmallChunks; + ASSERT_THAT_ERROR(Read(SmallChunks, 5), Succeeded()); + ASSERT_EQ(SmallChunks, Content); + ASSERT_EQ(Content.size() + 5, SmallChunks.capacity()); +} + TEST_F(FileSystemTest, readNativeFileSlice) { createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "01234"); FileRemover Cleanup(NonExistantFile); -- 2.7.4