[llvm] 345223a - Support: Extract sys::fs::readNativeFileToEOF() from MemoryBuffer
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 11 18:06:01 PST 2022
Author: Duncan P. N. Exon Smith
Date: 2022-01-11T18:03:58-08:00
New Revision: 345223a7be3c3c93a10f8453d96287a3a03b6754
URL: https://github.com/llvm/llvm-project/commit/345223a7be3c3c93a10f8453d96287a3a03b6754
DIFF: https://github.com/llvm/llvm-project/commit/345223a7be3c3c93a10f8453d96287a3a03b6754.diff
LOG: 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
Added:
Modified:
llvm/include/llvm/Support/FileSystem.h
llvm/lib/Support/MemoryBuffer.cpp
llvm/lib/Support/Path.cpp
llvm/unittests/Support/Path.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Support/FileSystem.h b/llvm/include/llvm/Support/FileSystem.h
index dabd384b400b4..033482977a105 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<size_t> readNativeFile(file_t FileHandle, MutableArrayRef<char> 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<char> &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 345b0d4aede5d..1bbdafd082a45 100644
--- a/llvm/lib/Support/MemoryBuffer.cpp
+++ b/llvm/lib/Support/MemoryBuffer.cpp
@@ -227,23 +227,9 @@ class MemoryBufferMMapFile : public MB {
static ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
getMemoryBufferForStream(sys::fs::file_t FD, const Twine &BufferName) {
- const ssize_t ChunkSize = 4096*4;
- SmallString<ChunkSize> Buffer;
-
- // Read into Buffer until we hit EOF.
- size_t Size = Buffer.size();
- for (;;) {
- Buffer.resize_for_overwrite(Size + ChunkSize);
- Expected<size_t> 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<sys::fs::DefaultReadChunkSize> 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 7c99d088911c6..2f80cdba7e344 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<const char *>(Mapping);
}
+Error readNativeFileToEOF(file_t FileHandle, SmallVectorImpl<char> &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<size_t> 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 35c1de7202796..e3fa5d0b2c491 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<char> &V,
+ Optional<ssize_t> ChunkSize) {
+ Expected<fs::file_t> 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<fs::DefaultReadChunkSize + Content.size()> StaysSmall;
+ SmallVectorImpl<char> *Vectors[] = {
+ static_cast<SmallVectorImpl<char> *>(&NoSmall),
+ static_cast<SmallVectorImpl<char> *>(&StaysSmall),
+ };
+ for (SmallVectorImpl<char> *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<char> *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<Content.size() + 5> 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);
More information about the llvm-commits
mailing list