diff options
author | Andreas Kling <awesomekling@gmail.com> | 2019-10-18 14:55:04 +0200 |
---|---|---|
committer | Andreas Kling <awesomekling@gmail.com> | 2019-10-18 14:55:04 +0200 |
commit | 1cca5142afbd76833deedfdb238230ac53424855 (patch) | |
tree | f7efa21ad7fd5e0fd9cb5bc5bf0e9e66d3afa924 | |
parent | 4027a64fc59253c85ca3da358fa85f15942480c0 (diff) | |
download | serenity-1cca5142afbd76833deedfdb238230ac53424855.zip |
Kernel: Make DoubleBuffer use a KBuffer instead of kmalloc()ing
Background: DoubleBuffer is a handy buffer class in the kernel that
allows you to keep writing to it from the "outside" while the "inside"
reads from it. It's used for things like LocalSocket and PTY's.
Internally, it has a read buffer and a write buffer, but the two will
swap places when the read buffer is exhausted (by reading from it.)
Before this patch, it was internally implemented as two Vector<u8>
that we would swap between when the reader side had exhausted the data
in the read buffer. Now instead we preallocate a large KBuffer (64KB*2)
on DoubleBuffer construction and use that throughout its lifetime.
This removes all the kmalloc heap traffic caused by DoubleBuffers :^)
-rw-r--r-- | Kernel/DoubleBuffer.cpp | 45 | ||||
-rw-r--r-- | Kernel/DoubleBuffer.h | 34 | ||||
-rw-r--r-- | Kernel/FileSystem/FIFO.cpp | 2 | ||||
-rw-r--r-- | Kernel/Net/LocalSocket.cpp | 4 | ||||
-rw-r--r-- | Kernel/TTY/MasterPTY.cpp | 2 |
5 files changed, 53 insertions, 34 deletions
diff --git a/Kernel/DoubleBuffer.cpp b/Kernel/DoubleBuffer.cpp index d4015f12e5..512c506407 100644 --- a/Kernel/DoubleBuffer.cpp +++ b/Kernel/DoubleBuffer.cpp @@ -1,20 +1,32 @@ #include <Kernel/DoubleBuffer.h> -inline void DoubleBuffer::compute_emptiness() +inline void DoubleBuffer::compute_lockfree_metadata() { - m_empty = m_read_buffer_index >= m_read_buffer->size() && m_write_buffer->is_empty(); + InterruptDisabler disabler; + m_empty = m_read_buffer_index >= m_read_buffer->size && m_write_buffer->size == 0; + m_space_for_writing = m_capacity - m_write_buffer->size; +} + +DoubleBuffer::DoubleBuffer(size_t capacity) + : m_write_buffer(&m_buffer1) + , m_read_buffer(&m_buffer2) + , m_storage(KBuffer::create_with_size(capacity * 2)) + , m_capacity(capacity) +{ + m_buffer1.data = m_storage.data(); + m_buffer1.size = 0; + m_buffer2.data = m_storage.data() + capacity; + m_buffer2.size = 0; + m_space_for_writing = capacity; } void DoubleBuffer::flip() { - ASSERT(m_read_buffer_index == m_read_buffer->size()); + ASSERT(m_read_buffer_index == m_read_buffer->size); swap(m_read_buffer, m_write_buffer); - if (m_write_buffer->capacity() < 32) - m_write_buffer->clear_with_capacity(); - else - m_write_buffer->clear(); + m_write_buffer->size = 0; m_read_buffer_index = 0; - compute_emptiness(); + compute_lockfree_metadata(); } ssize_t DoubleBuffer::write(const u8* data, ssize_t size) @@ -22,8 +34,11 @@ ssize_t DoubleBuffer::write(const u8* data, ssize_t size) if (!size) return 0; LOCKER(m_lock); - m_write_buffer->append(data, size); - compute_emptiness(); + ASSERT(size <= (ssize_t)space_for_writing()); + u8* write_ptr = m_write_buffer->data + m_write_buffer->size; + m_write_buffer->size += size; + compute_lockfree_metadata(); + memcpy(write_ptr, data, size); return size; } @@ -32,13 +47,13 @@ ssize_t DoubleBuffer::read(u8* data, ssize_t size) if (!size) return 0; LOCKER(m_lock); - if (m_read_buffer_index >= m_read_buffer->size() && !m_write_buffer->is_empty()) + if (m_read_buffer_index >= m_read_buffer->size && m_write_buffer->size != 0) flip(); - if (m_read_buffer_index >= m_read_buffer->size()) + if (m_read_buffer_index >= m_read_buffer->size) return 0; - ssize_t nread = min((ssize_t)m_read_buffer->size() - m_read_buffer_index, size); - memcpy(data, m_read_buffer->data() + m_read_buffer_index, nread); + ssize_t nread = min((ssize_t)m_read_buffer->size - (ssize_t)m_read_buffer_index, size); + memcpy(data, m_read_buffer->data + m_read_buffer_index, nread); m_read_buffer_index += nread; - compute_emptiness(); + compute_lockfree_metadata(); return nread; } diff --git a/Kernel/DoubleBuffer.h b/Kernel/DoubleBuffer.h index 44094829c9..c2f0f9cab4 100644 --- a/Kernel/DoubleBuffer.h +++ b/Kernel/DoubleBuffer.h @@ -1,34 +1,38 @@ #pragma once #include <AK/Types.h> -#include <AK/Vector.h> +#include <Kernel/KBuffer.h> #include <Kernel/Lock.h> class DoubleBuffer { public: - DoubleBuffer() - : m_write_buffer(&m_buffer1) - , m_read_buffer(&m_buffer2) - { - } + explicit DoubleBuffer(size_t capacity = 65536); ssize_t write(const u8*, ssize_t); ssize_t read(u8*, ssize_t); bool is_empty() const { return m_empty; } - // FIXME: Isn't this racy? What if we get interrupted between getting the buffer pointer and dereferencing it? - ssize_t bytes_in_write_buffer() const { return (ssize_t)m_write_buffer->size(); } + size_t space_for_writing() const { return m_space_for_writing; } private: void flip(); - void compute_emptiness(); + void compute_lockfree_metadata(); - Vector<u8>* m_write_buffer { nullptr }; - Vector<u8>* m_read_buffer { nullptr }; - Vector<u8> m_buffer1; - Vector<u8> m_buffer2; - ssize_t m_read_buffer_index { 0 }; + struct InnerBuffer { + u8* data { nullptr }; + size_t size; + }; + + InnerBuffer* m_write_buffer { nullptr }; + InnerBuffer* m_read_buffer { nullptr }; + InnerBuffer m_buffer1; + InnerBuffer m_buffer2; + + KBuffer m_storage; + size_t m_capacity { 0 }; + size_t m_read_buffer_index { 0 }; + size_t m_space_for_writing { 0 }; bool m_empty { true }; - Lock m_lock { "DoubleBuffer" }; + mutable Lock m_lock { "DoubleBuffer" }; }; diff --git a/Kernel/FileSystem/FIFO.cpp b/Kernel/FileSystem/FIFO.cpp index 6fc77a7b2b..1e2982f519 100644 --- a/Kernel/FileSystem/FIFO.cpp +++ b/Kernel/FileSystem/FIFO.cpp @@ -90,7 +90,7 @@ bool FIFO::can_read(FileDescription&) const bool FIFO::can_write(FileDescription&) const { - return m_buffer.bytes_in_write_buffer() < 4096 || !m_readers; + return m_buffer.space_for_writing() || !m_readers; } ssize_t FIFO::read(FileDescription&, u8* buffer, ssize_t size) diff --git a/Kernel/Net/LocalSocket.cpp b/Kernel/Net/LocalSocket.cpp index c59ab67699..e8fe103a95 100644 --- a/Kernel/Net/LocalSocket.cpp +++ b/Kernel/Net/LocalSocket.cpp @@ -210,9 +210,9 @@ bool LocalSocket::can_write(FileDescription& description) const { auto role = this->role(description); if (role == Role::Accepted) - return !has_attached_peer(description) || m_for_client.bytes_in_write_buffer() < 16384; + return !has_attached_peer(description) || m_for_client.space_for_writing(); if (role == Role::Connected) - return !has_attached_peer(description) || m_for_server.bytes_in_write_buffer() < 16384; + return !has_attached_peer(description) || m_for_server.space_for_writing(); ASSERT_NOT_REACHED(); } diff --git a/Kernel/TTY/MasterPTY.cpp b/Kernel/TTY/MasterPTY.cpp index a3caf1b17b..5afeec5355 100644 --- a/Kernel/TTY/MasterPTY.cpp +++ b/Kernel/TTY/MasterPTY.cpp @@ -81,7 +81,7 @@ bool MasterPTY::can_write_from_slave() const { if (m_closed) return true; - return m_buffer.bytes_in_write_buffer() < 4096; + return m_buffer.space_for_writing(); } void MasterPTY::close() |