diff --git a/AK/ByteBuffer.cpp b/AK/ByteBuffer.cpp deleted file mode 100644 index ab6b5a21b2..0000000000 --- a/AK/ByteBuffer.cpp +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright (c) 2020, the SerenityOS developers. - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#include - -namespace AK { - -bool ByteBuffer::operator==(const ByteBuffer& other) const -{ - if (is_empty() != other.is_empty()) - return false; - if (is_empty()) - return true; - if (size() != other.size()) - return false; - - // So they both have data, and the same length. - return !__builtin_memcmp(data(), other.data(), size()); -} - -} diff --git a/AK/ByteBuffer.h b/AK/ByteBuffer.h index ff6114af26..3b188695b3 100644 --- a/AK/ByteBuffer.h +++ b/AK/ByteBuffer.h @@ -1,212 +1,180 @@ /* * Copyright (c) 2018-2020, Andreas Kling + * Copyright (c) 2021, Gunnar Beutner * * SPDX-License-Identifier: BSD-2-Clause */ #pragma once -#include -#include -#include #include #include #include namespace AK { +namespace Detail { -class ByteBufferImpl : public RefCounted { -public: - static NonnullRefPtr create_uninitialized(size_t size); - static NonnullRefPtr create_zeroed(size_t); - static NonnullRefPtr copy(const void*, size_t); - - void operator delete(void* ptr) - { - kfree(ptr); - } - - ByteBufferImpl() = delete; - - u8& operator[](size_t i) - { - VERIFY(i < m_size); - return m_data[i]; - } - const u8& operator[](size_t i) const - { - VERIFY(i < m_size); - return m_data[i]; - } - bool is_empty() const { return !m_size; } - size_t size() const { return m_size; } - - u8* data() { return m_data; } - const u8* data() const { return m_data; } - - Bytes bytes() { return { data(), size() }; } - ReadonlyBytes bytes() const { return { data(), size() }; } - - Span span() { return { data(), size() }; } - Span span() const { return { data(), size() }; } - - u8* offset_pointer(int offset) { return m_data + offset; } - const u8* offset_pointer(int offset) const { return m_data + offset; } - - void* end_pointer() { return m_data + m_size; } - const void* end_pointer() const { return m_data + m_size; } - - // NOTE: trim() does not reallocate. - void trim(size_t size) - { - VERIFY(size <= m_size); - m_size = size; - } - - void zero_fill(); - -private: - explicit ByteBufferImpl(size_t); - - size_t m_size { 0 }; - u8 m_data[]; -}; - +template class ByteBuffer { public: ByteBuffer() = default; - ByteBuffer(const ByteBuffer& other) - : m_impl(other.m_impl) + + ~ByteBuffer() { + clear(); } + + ByteBuffer(ByteBuffer const& other) + { + grow(other.size()); + VERIFY(m_size == other.size()); + VERIFY(!m_is_null); + __builtin_memcpy(data(), other.data(), other.size()); + } + ByteBuffer(ByteBuffer&& other) - : m_impl(move(other.m_impl)) { + move_from(move(other)); } + ByteBuffer& operator=(ByteBuffer&& other) { - if (this != &other) - m_impl = move(other.m_impl); + if (this != &other) { + if (!is_inline()) + kfree(m_outline_buffer); + move_from(move(other)); + } return *this; } - ByteBuffer& operator=(const ByteBuffer& other) + + ByteBuffer& operator=(ByteBuffer const& other) { - if (this != &other) - m_impl = other.m_impl; + if (this != &other) { + if (m_size > other.size()) + internal_trim(other.size(), true); + else + grow(other.size()); + __builtin_memcpy(data(), other.data(), other.size()); + } return *this; } - [[nodiscard]] static ByteBuffer create_uninitialized(size_t size) { return ByteBuffer(ByteBufferImpl::create_uninitialized(size)); } - [[nodiscard]] static ByteBuffer create_zeroed(size_t size) { return ByteBuffer(ByteBufferImpl::create_zeroed(size)); } - [[nodiscard]] static ByteBuffer copy(const void* data, size_t size) { return ByteBuffer(ByteBufferImpl::copy(data, size)); } - [[nodiscard]] static ByteBuffer copy(ReadonlyBytes bytes) { return ByteBuffer(ByteBufferImpl::copy(bytes.data(), bytes.size())); } + [[nodiscard]] static ByteBuffer create_uninitialized(size_t size) + { + return ByteBuffer(size); + } - ~ByteBuffer() { clear(); } - void clear() { m_impl = nullptr; } + [[nodiscard]] static ByteBuffer create_zeroed(size_t size) + { + auto buffer = create_uninitialized(size); + buffer.zero_fill(); + VERIFY(size == 0 || (buffer[0] == 0 && buffer[size - 1] == 0)); + return buffer; + } + + [[nodiscard]] static ByteBuffer copy(void const* data, size_t size) + { + auto buffer = create_uninitialized(size); + __builtin_memcpy(buffer.data(), data, size); + return buffer; + } + + [[nodiscard]] static ByteBuffer copy(ReadonlyBytes bytes) + { + return copy(bytes.data(), bytes.size()); + } + + template + bool operator==(ByteBuffer const& other) const + { + if (size() != other.size()) + return false; + + // So they both have data, and the same length. + return !__builtin_memcmp(data(), other.data(), size()); + } + + bool operator!=(ByteBuffer const& other) const { return !(*this == other); } operator bool() const { return !is_null(); } bool operator!() const { return is_null(); } - [[nodiscard]] bool is_null() const { return m_impl == nullptr; } - - // Disable default implementations that would use surprising integer promotion. - bool operator==(const ByteBuffer& other) const; - bool operator!=(const ByteBuffer& other) const { return !(*this == other); } - bool operator<=(const ByteBuffer& other) const = delete; - bool operator>=(const ByteBuffer& other) const = delete; - bool operator<(const ByteBuffer& other) const = delete; - bool operator>(const ByteBuffer& other) const = delete; + [[nodiscard]] bool is_null() const { return m_is_null; } [[nodiscard]] u8& operator[](size_t i) { - VERIFY(m_impl); - return (*m_impl)[i]; + VERIFY(i < m_size); + return data()[i]; } - [[nodiscard]] u8 operator[](size_t i) const + + [[nodiscard]] u8 const& operator[](size_t i) const { - VERIFY(m_impl); - return (*m_impl)[i]; - } - [[nodiscard]] bool is_empty() const { return !m_impl || m_impl->is_empty(); } - [[nodiscard]] size_t size() const { return m_impl ? m_impl->size() : 0; } - - [[nodiscard]] u8* data() { return m_impl ? m_impl->data() : nullptr; } - [[nodiscard]] const u8* data() const { return m_impl ? m_impl->data() : nullptr; } - - [[nodiscard]] Bytes bytes() - { - if (m_impl) { - return m_impl->bytes(); - } - return {}; - } - [[nodiscard]] ReadonlyBytes bytes() const - { - if (m_impl) { - return m_impl->bytes(); - } - return {}; + VERIFY(i < m_size); + return data()[i]; } - [[nodiscard]] Span span() - { - if (m_impl) { - return m_impl->span(); - } - return {}; - } - [[nodiscard]] Span span() const - { - if (m_impl) { - return m_impl->span(); - } - return {}; - } + [[nodiscard]] bool is_empty() const { return !m_size; } + [[nodiscard]] size_t size() const { return m_size; } - [[nodiscard]] u8* offset_pointer(int offset) { return m_impl ? m_impl->offset_pointer(offset) : nullptr; } - [[nodiscard]] const u8* offset_pointer(int offset) const { return m_impl ? m_impl->offset_pointer(offset) : nullptr; } + [[nodiscard]] u8* data() { return is_inline() ? m_inline_buffer : m_outline_buffer; } + [[nodiscard]] u8 const* data() const { return is_inline() ? m_inline_buffer : m_outline_buffer; } - [[nodiscard]] void* end_pointer() { return m_impl ? m_impl->end_pointer() : nullptr; } - [[nodiscard]] const void* end_pointer() const { return m_impl ? m_impl->end_pointer() : nullptr; } + [[nodiscard]] Bytes bytes() { return { data(), size() }; } + [[nodiscard]] ReadonlyBytes bytes() const { return { data(), size() }; } - [[nodiscard]] ByteBuffer isolated_copy() const - { - if (!m_impl) - return {}; - return copy(m_impl->data(), m_impl->size()); - } + [[nodiscard]] AK::Span span() { return { data(), size() }; } + [[nodiscard]] AK::Span span() const { return { data(), size() }; } + + [[nodiscard]] u8* offset_pointer(int offset) { return data() + offset; } + [[nodiscard]] u8 const* offset_pointer(int offset) const { return data() + offset; } + + [[nodiscard]] void* end_pointer() { return data() + m_size; } + [[nodiscard]] void const* end_pointer() const { return data() + m_size; } - // NOTE: trim() does not reallocate. void trim(size_t size) { - if (m_impl) - m_impl->trim(size); + internal_trim(size, false); } [[nodiscard]] ByteBuffer slice(size_t offset, size_t size) const { - if (is_null()) - return {}; - - if (offset == 0 && size == this->size()) - return *this; - // I cannot hand you a slice I don't have VERIFY(offset + size <= this->size()); return copy(offset_pointer(offset), size); } - void grow(size_t size) + void clear() { - if (m_impl && size < m_impl->size()) - return; - auto new_impl = ByteBufferImpl::create_uninitialized(size); - if (m_impl) - __builtin_memcpy(new_impl->data(), m_impl->data(), m_impl->size()); - m_impl = new_impl; + if (!is_inline()) + kfree(m_outline_buffer); + m_size = 0; } - void append(const void* data, size_t data_size) + void grow(size_t new_size) + { + m_is_null = false; + if (new_size <= m_size) + return; + if (new_size <= capacity()) { + m_size = new_size; + return; + } + u8* new_buffer; + auto new_capacity = kmalloc_good_size(new_size); + if (!is_inline()) { + new_buffer = (u8*)krealloc(m_outline_buffer, new_capacity); + VERIFY(new_buffer); + } else { + new_buffer = (u8*)kmalloc(new_capacity); + VERIFY(new_buffer); + __builtin_memcpy(new_buffer, data(), m_size); + } + m_outline_buffer = new_buffer; + m_outline_capacity = new_capacity; + m_size = new_size; + } + + void append(void const* data, size_t data_size) { if (data_size == 0) return; @@ -216,12 +184,12 @@ public: __builtin_memcpy(this->data() + old_size, data, data_size); } - void operator+=(const ByteBuffer& other) + void operator+=(ByteBuffer const& other) { append(other.data(), other.size()); } - void overwrite(size_t offset, const void* data, size_t data_size) + void overwrite(size_t offset, void const* data, size_t data_size) { // make sure we're not told to write past the end VERIFY(offset + data_size <= size()); @@ -230,53 +198,59 @@ public: void zero_fill() { - m_impl->zero_fill(); + __builtin_memset(data(), 0, m_size); } operator Bytes() { return bytes(); } operator ReadonlyBytes() const { return bytes(); } private: - explicit ByteBuffer(RefPtr&& impl) - : m_impl(move(impl)) + ByteBuffer(size_t size) { + grow(size); + VERIFY(!m_is_null); + VERIFY(m_size == size); } - RefPtr m_impl; + void move_from(ByteBuffer&& other) + { + m_is_null = other.m_is_null; + m_size = other.m_size; + if (other.m_size > inline_capacity) { + m_outline_buffer = other.m_outline_buffer; + m_outline_capacity = other.m_outline_capacity; + } else + __builtin_memcpy(m_inline_buffer, other.m_inline_buffer, other.m_size); + other.m_is_null = true; + other.m_size = 0; + } + + void internal_trim(size_t size, bool may_discard_existing_data) + { + VERIFY(size <= m_size); + if (!is_inline() && size <= inline_capacity) { + // m_inline_buffer and m_outline_buffer are part of a union, so save the pointer + auto outline_buffer = m_outline_buffer; + if (!may_discard_existing_data) + __builtin_memcpy(m_inline_buffer, outline_buffer, size); + kfree(outline_buffer); + } + m_size = size; + } + + bool is_inline() const { return m_size <= inline_capacity; } + size_t capacity() const { return is_inline() ? inline_capacity : m_outline_capacity; } + + size_t m_size { 0 }; + bool m_is_null { true }; + union { + u8 m_inline_buffer[inline_capacity]; + struct { + u8* m_outline_buffer; + size_t m_outline_capacity; + }; + }; }; -inline ByteBufferImpl::ByteBufferImpl(size_t size) - : m_size(size) -{ } - -inline void ByteBufferImpl::zero_fill() -{ - __builtin_memset(m_data, 0, m_size); } - -inline NonnullRefPtr ByteBufferImpl::create_uninitialized(size_t size) -{ - auto* buffer = kmalloc(sizeof(ByteBufferImpl) + size); - VERIFY(buffer); - return ::adopt_ref(*new (buffer) ByteBufferImpl(size)); -} - -inline NonnullRefPtr ByteBufferImpl::create_zeroed(size_t size) -{ - auto buffer = create_uninitialized(size); - if (size != 0) - __builtin_memset(buffer->data(), 0, size); - return buffer; -} - -inline NonnullRefPtr ByteBufferImpl::copy(const void* data, size_t size) -{ - auto buffer = create_uninitialized(size); - __builtin_memcpy(buffer->data(), data, size); - return buffer; -} - -} - -using AK::ByteBuffer; diff --git a/AK/Forward.h b/AK/Forward.h index 1e44690091..aa9234d835 100644 --- a/AK/Forward.h +++ b/AK/Forward.h @@ -10,8 +10,13 @@ namespace AK { -class Bitmap; +namespace Detail { +template class ByteBuffer; +} + +class Bitmap; +using ByteBuffer = AK::Detail::ByteBuffer<32>; class IPv4Address; class JsonArray; class JsonObject; diff --git a/AK/StringBuilder.cpp b/AK/StringBuilder.cpp index aa0329a947..68d5343578 100644 --- a/AK/StringBuilder.cpp +++ b/AK/StringBuilder.cpp @@ -21,23 +21,12 @@ inline void StringBuilder::will_append(size_t size) Checked needed_capacity = m_length; needed_capacity += size; VERIFY(!needed_capacity.has_overflow()); - if (needed_capacity < inline_capacity) - return; - Checked expanded_capacity = needed_capacity; - expanded_capacity *= 2; - VERIFY(!expanded_capacity.has_overflow()); - if (m_buffer.is_null()) { - m_buffer.grow(expanded_capacity.value()); - memcpy(m_buffer.data(), m_inline_buffer, m_length); - } else if (needed_capacity.value() > m_buffer.size()) { - m_buffer.grow(expanded_capacity.value()); - } + m_buffer.grow(needed_capacity.value()); } StringBuilder::StringBuilder(size_t initial_capacity) + : m_buffer(decltype(m_buffer)::create_uninitialized(initial_capacity)) { - if (initial_capacity > inline_capacity) - m_buffer.grow(initial_capacity); } void StringBuilder::append(const StringView& str) @@ -94,7 +83,6 @@ StringView StringBuilder::string_view() const void StringBuilder::clear() { m_buffer.clear(); - m_inline_buffer[0] = '\0'; m_length = 0; } diff --git a/AK/StringBuilder.h b/AK/StringBuilder.h index e97b76b6af..1ad64d4f39 100644 --- a/AK/StringBuilder.h +++ b/AK/StringBuilder.h @@ -62,13 +62,11 @@ public: private: void will_append(size_t); - u8* data() { return m_buffer.is_null() ? m_inline_buffer : m_buffer.data(); } - const u8* data() const { return m_buffer.is_null() ? m_inline_buffer : m_buffer.data(); } - bool using_inline_buffer() const { return m_buffer.is_null(); } + u8* data() { return m_buffer.data(); } + const u8* data() const { return m_buffer.data(); } static constexpr size_t inline_capacity = 128; - u8 m_inline_buffer[inline_capacity]; - ByteBuffer m_buffer; + AK::Detail::ByteBuffer m_buffer; size_t m_length { 0 }; }; diff --git a/Kernel/CMakeLists.txt b/Kernel/CMakeLists.txt index c2c3a4cc34..103ba80609 100644 --- a/Kernel/CMakeLists.txt +++ b/Kernel/CMakeLists.txt @@ -256,7 +256,6 @@ set(KERNEL_SOURCES ) set(AK_SOURCES - ../AK/ByteBuffer.cpp ../AK/FlyString.cpp ../AK/GenericLexer.cpp ../AK/Hex.cpp diff --git a/Tests/LibC/snprintf-correctness.cpp b/Tests/LibC/snprintf-correctness.cpp index 0df2a5cc30..a3de8da7b5 100644 --- a/Tests/LibC/snprintf-correctness.cpp +++ b/Tests/LibC/snprintf-correctness.cpp @@ -55,7 +55,7 @@ static bool test_single(const Testcase& testcase) // Setup ByteBuffer actual = ByteBuffer::create_uninitialized(SANDBOX_CANARY_SIZE + testcase.dest_n + SANDBOX_CANARY_SIZE); fill_with_random(actual.data(), actual.size()); - ByteBuffer expected = actual.isolated_copy(); + ByteBuffer expected = actual; VERIFY(actual.offset_pointer(0) != expected.offset_pointer(0)); actual.overwrite(SANDBOX_CANARY_SIZE, testcase.dest, testcase.dest_n); expected.overwrite(SANDBOX_CANARY_SIZE, testcase.dest_expected, testcase.dest_expected_n); diff --git a/Tests/LibC/strlcpy-correctness.cpp b/Tests/LibC/strlcpy-correctness.cpp index e23765cd29..c9dfc24128 100644 --- a/Tests/LibC/strlcpy-correctness.cpp +++ b/Tests/LibC/strlcpy-correctness.cpp @@ -57,7 +57,7 @@ static bool test_single(const Testcase& testcase) // Setup ByteBuffer actual = ByteBuffer::create_uninitialized(SANDBOX_CANARY_SIZE + testcase.dest_n + SANDBOX_CANARY_SIZE); fill_with_random(actual.data(), actual.size()); - ByteBuffer expected = actual.isolated_copy(); + ByteBuffer expected = actual; VERIFY(actual.offset_pointer(0) != expected.offset_pointer(0)); actual.overwrite(SANDBOX_CANARY_SIZE, testcase.dest, testcase.dest_n); expected.overwrite(SANDBOX_CANARY_SIZE, testcase.dest_expected, testcase.dest_expected_n); diff --git a/Userland/Applications/HexEditor/HexEditorWidget.cpp b/Userland/Applications/HexEditor/HexEditorWidget.cpp index d5589f690e..b328e8533c 100644 --- a/Userland/Applications/HexEditor/HexEditorWidget.cpp +++ b/Userland/Applications/HexEditor/HexEditorWidget.cpp @@ -169,7 +169,7 @@ void HexEditorWidget::initialize_menubar(GUI::Menubar& menubar) })); edit_menu.add_separator(); edit_menu.add_action(GUI::Action::create("&Find", { Mod_Ctrl, Key_F }, Gfx::Bitmap::load_from_file("/res/icons/16x16/find.png"), [&](const GUI::Action&) { - auto old_buffer = m_search_buffer.isolated_copy(); + auto old_buffer = m_search_buffer; if (FindDialog::show(window(), m_search_text, m_search_buffer) == GUI::InputBox::ExecOK) { bool same_buffers = false;