From e432a284d7db4ff3719e0f1e4cda7e9a5aa453e1 Mon Sep 17 00:00:00 2001
From: Lucas CHOLLET <lucas.chollet@free.fr>
Date: Sun, 27 Feb 2022 12:57:31 +0100
Subject: [PATCH] LibCore: Avoid some successive allocations in
 Stream::read_all()

For the general case, allocations will always have the size of a block.
In case of a smaller read a block will be filled entirely before another
allocation appends.

It also adds a specialization for Stream::File::read_all() that tries to
detect the size of the file with fstat to perform a single allocation.
---
 Userland/Libraries/LibCore/Stream.cpp | 43 ++++++++++++++++++++-------
 Userland/Libraries/LibCore/Stream.h   |  6 +++-
 2 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/Userland/Libraries/LibCore/Stream.cpp b/Userland/Libraries/LibCore/Stream.cpp
index 8e18320c618..d7bc40c38a3 100644
--- a/Userland/Libraries/LibCore/Stream.cpp
+++ b/Userland/Libraries/LibCore/Stream.cpp
@@ -47,20 +47,35 @@ bool Stream::read_or_error(Bytes buffer)
     return true;
 }
 
-ErrorOr<ByteBuffer> Stream::read_all()
+ErrorOr<ByteBuffer> Stream::read_all(size_t block_size)
 {
-    ByteBuffer output;
-    u8 buffer_raw[4096];
-    Bytes buffer { buffer_raw, 4096 };
+    return read_all_impl(block_size);
+}
 
-    while (true) {
-        Bytes read_bytes = TRY(read(buffer));
-        if (read_bytes.is_empty())
-            break;
-        output.append(read_bytes);
+ErrorOr<ByteBuffer> Stream::read_all_impl(size_t block_size, size_t file_size)
+{
+    ByteBuffer data;
+    data.ensure_capacity(file_size);
+
+    size_t total_read {};
+    size_t next_reading_size { block_size };
+    for (Span<u8> chunk; !is_eof();) {
+        if (next_reading_size == block_size)
+            chunk = TRY(data.get_bytes_for_writing(next_reading_size));
+        auto const nread = TRY(read(chunk)).size();
+
+        next_reading_size -= nread;
+
+        if (next_reading_size == 0)
+            next_reading_size = block_size;
+
+        total_read += nread;
+
+        if (nread < block_size)
+            data.resize(total_read);
     }
 
-    return output;
+    return data;
 }
 
 bool Stream::write_or_error(ReadonlyBytes buffer)
@@ -191,6 +206,14 @@ ErrorOr<Bytes> File::read(Bytes buffer)
     return buffer.trim(nread);
 }
 
+ErrorOr<ByteBuffer> File::read_all(size_t block_size)
+{
+    // Note: This is used as a heuristic, it's not valid for devices or virtual files.
+    auto const potential_file_size = TRY(System::fstat(m_fd)).st_size;
+
+    return read_all_impl(block_size, potential_file_size);
+}
+
 ErrorOr<size_t> File::write(ReadonlyBytes buffer)
 {
     if (!has_flag(m_mode, OpenMode::Write)) {
diff --git a/Userland/Libraries/LibCore/Stream.h b/Userland/Libraries/LibCore/Stream.h
index ae7aff18da2..f21dd8a109a 100644
--- a/Userland/Libraries/LibCore/Stream.h
+++ b/Userland/Libraries/LibCore/Stream.h
@@ -37,7 +37,7 @@ public:
     /// Tries to fill the entire buffer through reading. Returns whether the
     /// buffer was filled without an error.
     virtual bool read_or_error(Bytes);
-    ErrorOr<ByteBuffer> read_all();
+    virtual ErrorOr<ByteBuffer> read_all(size_t block_size = 4096);
 
     virtual bool is_writable() const { return false; }
     /// Tries to write the entire contents of the buffer. It is possible for
@@ -62,6 +62,9 @@ public:
     virtual ~Stream()
     {
     }
+
+protected:
+    ErrorOr<ByteBuffer> read_all_impl(size_t block_size, size_t file_size = 0);
 };
 
 enum class SeekMode {
@@ -197,6 +200,7 @@ public:
 
     virtual bool is_readable() const override;
     virtual ErrorOr<Bytes> read(Bytes) override;
+    virtual ErrorOr<ByteBuffer> read_all(size_t block_size = 4096) override;
     virtual bool is_writable() const override;
     virtual ErrorOr<size_t> write(ReadonlyBytes) override;
     virtual bool is_eof() const override;