From 34160743dccd54c9e6270b844e21363ec7226cab Mon Sep 17 00:00:00 2001
From: Timothy Flynn <trflynn89@pm.me>
Date: Tue, 2 Jan 2024 20:27:29 -0500
Subject: [PATCH] LibIPC: Avoid redundant copy of every tranferred IPC message

For every IPC message sent, we currently prepend the message size to the
IPC message buffer. This incurs the cost of copying the entire message
to its newly allocated position. Instead, reserve the bytes for the size
at the front of the buffer upon creation. Prevent dangerous access to
the buffer with specific public methods.
---
 Userland/Libraries/LibIPC/Encoder.h   | 10 ++++----
 Userland/Libraries/LibIPC/Forward.h   |  2 +-
 Userland/Libraries/LibIPC/Message.cpp | 35 +++++++++++++++++++++++----
 Userland/Libraries/LibIPC/Message.h   | 15 +++++++++---
 4 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/Userland/Libraries/LibIPC/Encoder.h b/Userland/Libraries/LibIPC/Encoder.h
index 3b5341b78a..508d5db348 100644
--- a/Userland/Libraries/LibIPC/Encoder.h
+++ b/Userland/Libraries/LibIPC/Encoder.h
@@ -37,20 +37,20 @@ public:
 
     ErrorOr<void> extend_capacity(size_t capacity)
     {
-        return m_buffer.data.try_ensure_capacity(m_buffer.data.size() + capacity);
+        TRY(m_buffer.extend_data_capacity(capacity));
+        return {};
     }
 
     ErrorOr<void> append(u8 const* values, size_t count)
     {
-        TRY(extend_capacity(count));
-        m_buffer.data.unchecked_append(values, count);
+        TRY(m_buffer.append_data(values, count));
         return {};
     }
 
     ErrorOr<void> append_file_descriptor(int fd)
     {
-        auto auto_fd = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AutoCloseFileDescriptor(fd)));
-        return m_buffer.fds.try_append(move(auto_fd));
+        TRY(m_buffer.append_file_descriptor(fd));
+        return {};
     }
 
     ErrorOr<void> encode_size(size_t size);
diff --git a/Userland/Libraries/LibIPC/Forward.h b/Userland/Libraries/LibIPC/Forward.h
index 6be4a6197d..f1631faa93 100644
--- a/Userland/Libraries/LibIPC/Forward.h
+++ b/Userland/Libraries/LibIPC/Forward.h
@@ -13,7 +13,7 @@ namespace IPC {
 class Decoder;
 class Encoder;
 class Message;
-struct MessageBuffer;
+class MessageBuffer;
 class File;
 class Stub;
 
diff --git a/Userland/Libraries/LibIPC/Message.cpp b/Userland/Libraries/LibIPC/Message.cpp
index a6fe8c7212..ef96322b71 100644
--- a/Userland/Libraries/LibIPC/Message.cpp
+++ b/Userland/Libraries/LibIPC/Message.cpp
@@ -13,20 +13,45 @@ namespace IPC {
 
 using MessageSizeType = u32;
 
+MessageBuffer::MessageBuffer()
+{
+    m_data.resize(sizeof(MessageSizeType));
+}
+
+ErrorOr<void> MessageBuffer::extend_data_capacity(size_t capacity)
+{
+    TRY(m_data.try_ensure_capacity(m_data.size() + capacity));
+    return {};
+}
+
+ErrorOr<void> MessageBuffer::append_data(u8 const* values, size_t count)
+{
+    TRY(m_data.try_append(values, count));
+    return {};
+}
+
+ErrorOr<void> MessageBuffer::append_file_descriptor(int fd)
+{
+    auto auto_fd = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AutoCloseFileDescriptor(fd)));
+    TRY(m_fds.try_append(move(auto_fd)));
+    return {};
+}
+
 ErrorOr<void> MessageBuffer::transfer_message(Core::LocalSocket& fd_passing_socket, Core::LocalSocket& data_socket)
 {
-    Checked<MessageSizeType> checked_message_size { data.size() };
+    Checked<MessageSizeType> checked_message_size { m_data.size() };
+    checked_message_size -= sizeof(MessageSizeType);
 
     if (checked_message_size.has_overflow())
         return Error::from_string_literal("Message is too large for IPC encoding");
 
     auto message_size = checked_message_size.value();
-    TRY(data.try_prepend(reinterpret_cast<u8 const*>(&message_size), sizeof(message_size)));
+    m_data.span().overwrite(0, reinterpret_cast<u8 const*>(&message_size), sizeof(message_size));
 
-    for (auto const& fd : fds)
+    for (auto const& fd : m_fds)
         TRY(fd_passing_socket.send_fd(fd->value()));
 
-    ReadonlyBytes bytes_to_write { data.span() };
+    ReadonlyBytes bytes_to_write { m_data.span() };
     size_t writes_done = 0;
 
     while (!bytes_to_write.is_empty()) {
@@ -59,7 +84,7 @@ ErrorOr<void> MessageBuffer::transfer_message(Core::LocalSocket& fd_passing_sock
     }
 
     if (writes_done > 1) {
-        dbgln("LibIPC::transfer_message FIXME Warning, needed {} writes needed to send message of size {}B, this is pretty bad, as it spins on the EventLoop", writes_done, data.size());
+        dbgln("LibIPC::transfer_message FIXME Warning, needed {} writes needed to send message of size {}B, this is pretty bad, as it spins on the EventLoop", writes_done, m_data.size());
     }
 
     return {};
diff --git a/Userland/Libraries/LibIPC/Message.h b/Userland/Libraries/LibIPC/Message.h
index eea9471e27..cf07a3b109 100644
--- a/Userland/Libraries/LibIPC/Message.h
+++ b/Userland/Libraries/LibIPC/Message.h
@@ -35,11 +35,20 @@ private:
     int m_fd;
 };
 
-struct MessageBuffer {
+class MessageBuffer {
+public:
+    MessageBuffer();
+
+    ErrorOr<void> extend_data_capacity(size_t capacity);
+    ErrorOr<void> append_data(u8 const* values, size_t count);
+
+    ErrorOr<void> append_file_descriptor(int fd);
+
     ErrorOr<void> transfer_message(Core::LocalSocket& fd_passing_socket, Core::LocalSocket& data_socket);
 
-    Vector<u8, 1024> data;
-    Vector<NonnullRefPtr<AutoCloseFileDescriptor>, 1> fds;
+private:
+    Vector<u8, 1024> m_data;
+    Vector<NonnullRefPtr<AutoCloseFileDescriptor>, 1> m_fds;
 };
 
 enum class ErrorCode : u32 {