From 63ce1708532b7f884541d536ecb1caf3c11dd9ad Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Tue, 2 Jan 2024 07:02:20 -0800 Subject: [PATCH] Replace virtual dispatch with normal functions in buffers --- include/fmt/core.h | 76 ++++++++++++++++++++++---------------------- include/fmt/format.h | 25 ++++++++------- include/fmt/os.h | 3 +- src/os.cc | 11 +++---- test/core-test.cc | 12 ++++--- test/ostream-test.cc | 7 ++-- 6 files changed, 69 insertions(+), 65 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index cb0ec968..19beae14 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -8,12 +8,12 @@ #ifndef FMT_CORE_H_ #define FMT_CORE_H_ -#include // std::byte -#include // std::FILE -#include // std::strlen -#include -#include -#include // std::addressof +#include // std::byte +#include // std::FILE +#include // std::strlen +#include // std::back_insert_iterator +#include // std::numeric_limits +#include // std::addressof #include #include @@ -821,13 +821,18 @@ template class buffer { size_t size_; size_t capacity_; + using grow_fun = void (*)(buffer& buf, size_t capacity); + grow_fun grow_; + protected: // Don't initialize ptr_ since it is not accessed to save a few cycles. FMT_MSC_WARNING(suppress : 26495) - FMT_CONSTEXPR buffer(size_t sz) noexcept : size_(sz), capacity_(sz) {} + FMT_CONSTEXPR buffer(grow_fun grow, size_t sz) noexcept + : size_(sz), capacity_(sz), grow_(grow) {} - FMT_CONSTEXPR20 buffer(T* p = nullptr, size_t sz = 0, size_t cap = 0) noexcept - : ptr_(p), size_(sz), capacity_(cap) {} + FMT_CONSTEXPR20 buffer(grow_fun grow, T* p = nullptr, size_t sz = 0, + size_t cap = 0) noexcept + : ptr_(p), size_(sz), capacity_(cap), grow_(grow) {} FMT_CONSTEXPR20 ~buffer() = default; buffer(buffer&&) = default; @@ -838,10 +843,6 @@ template class buffer { capacity_ = buf_capacity; } - /** Increases the buffer capacity to hold at least *capacity* elements. */ - // DEPRECATED! - virtual FMT_CONSTEXPR20 void grow(size_t capacity) = 0; - public: using value_type = T; using const_reference = const T&; @@ -880,7 +881,7 @@ template class buffer { // for at least one additional element either by increasing the capacity or by // flushing the buffer if it is full. FMT_CONSTEXPR20 void try_reserve(size_t new_capacity) { - if (new_capacity > capacity_) grow(new_capacity); + if (new_capacity > capacity_) grow_(*this, new_capacity); } FMT_CONSTEXPR20 void push_back(const T& value) { @@ -929,9 +930,8 @@ class iterator_buffer final : public Traits, public buffer { enum { buffer_size = 256 }; T data_[buffer_size]; - protected: - FMT_CONSTEXPR20 void grow(size_t) override { - if (this->size() == buffer_size) flush(); + static FMT_CONSTEXPR20 void grow(buffer& buf, size_t) { + if (buf.size() == buffer_size) static_cast(buf).flush(); } void flush() { @@ -942,9 +942,11 @@ class iterator_buffer final : public Traits, public buffer { public: explicit iterator_buffer(OutputIt out, size_t n = buffer_size) - : Traits(n), buffer(data_, 0, buffer_size), out_(out) {} + : Traits(n), buffer(grow, data_, 0, buffer_size), out_(out) {} iterator_buffer(iterator_buffer&& other) - : Traits(other), buffer(data_, 0, buffer_size), out_(other.out_) {} + : Traits(other), + buffer(grow, data_, 0, buffer_size), + out_(other.out_) {} ~iterator_buffer() { flush(); } auto out() -> OutputIt { @@ -963,9 +965,9 @@ class iterator_buffer final enum { buffer_size = 256 }; T data_[buffer_size]; - protected: - FMT_CONSTEXPR20 void grow(size_t) override { - if (this->size() == this->capacity()) flush(); + static FMT_CONSTEXPR20 void grow(buffer& buf, size_t) { + if (buf.size() == buf.capacity()) + static_cast(buf).flush(); } void flush() { @@ -979,7 +981,7 @@ class iterator_buffer final public: explicit iterator_buffer(T* out, size_t n = buffer_size) - : fixed_buffer_traits(n), buffer(out, 0, n), out_(out) {} + : fixed_buffer_traits(n), buffer(grow, out, 0, n), out_(out) {} iterator_buffer(iterator_buffer&& other) : fixed_buffer_traits(other), buffer(std::move(other)), @@ -1001,11 +1003,9 @@ class iterator_buffer final }; template class iterator_buffer final : public buffer { - protected: - FMT_CONSTEXPR20 void grow(size_t) override {} - public: - explicit iterator_buffer(T* out, size_t = 0) : buffer(out, 0, ~size_t()) {} + explicit iterator_buffer(T* out, size_t = 0) + : buffer([](buffer&, size_t) {}, out, 0, ~size_t()) {} auto out() -> T* { return &*this->end(); } }; @@ -1017,17 +1017,18 @@ class iterator_buffer, typename Container::value_type>> final : public buffer { private: + using value_type = typename Container::value_type; Container& container_; - protected: - FMT_CONSTEXPR20 void grow(size_t capacity) override { - container_.resize(capacity); - this->set(&container_[0], capacity); + static FMT_CONSTEXPR20 void grow(buffer& buf, size_t capacity) { + auto& self = static_cast(buf); + self.container_.resize(capacity); + self.set(&self.container_[0], capacity); } public: explicit iterator_buffer(Container& c) - : buffer(c.size()), container_(c) {} + : buffer(grow, c.size()), container_(c) {} explicit iterator_buffer(std::back_insert_iterator out, size_t = 0) : iterator_buffer(get_container(out)) {} @@ -1043,15 +1044,14 @@ template class counting_buffer final : public buffer { T data_[buffer_size]; size_t count_ = 0; - protected: - FMT_CONSTEXPR20 void grow(size_t) override { - if (this->size() != buffer_size) return; - count_ += this->size(); - this->clear(); + static FMT_CONSTEXPR20 void grow(buffer& buf, size_t) { + if (buf.size() != buffer_size) return; + static_cast(buf).count_ += buf.size(); + buf.clear(); } public: - counting_buffer() : buffer(data_, 0, buffer_size) {} + counting_buffer() : buffer(grow, data_, 0, buffer_size) {} auto count() -> size_t { return count_ + this->size(); } }; diff --git a/include/fmt/format.h b/include/fmt/format.h index ce06f6ed..997020b9 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -880,27 +880,29 @@ class basic_memory_buffer final : public detail::buffer { } protected: - FMT_CONSTEXPR20 void grow(size_t size) override { + static FMT_CONSTEXPR20 void grow(detail::buffer& buf, size_t size) { detail::abort_fuzzing_if(size > 5000); - const size_t max_size = std::allocator_traits::max_size(alloc_); - size_t old_capacity = this->capacity(); + auto& self = static_cast(buf); + const size_t max_size = + std::allocator_traits::max_size(self.alloc_); + size_t old_capacity = buf.capacity(); size_t new_capacity = old_capacity + old_capacity / 2; if (size > new_capacity) new_capacity = size; else if (new_capacity > max_size) new_capacity = size > max_size ? size : max_size; - T* old_data = this->data(); + T* old_data = buf.data(); T* new_data = - std::allocator_traits::allocate(alloc_, new_capacity); + std::allocator_traits::allocate(self.alloc_, new_capacity); // Suppress a bogus -Wstringop-overflow in gcc 13.1 (#3481). - detail::assume(this->size() <= new_capacity); + detail::assume(buf.size() <= new_capacity); // The following code doesn't throw, so the raw pointer above doesn't leak. - std::uninitialized_copy_n(old_data, this->size(), new_data); - this->set(new_data, new_capacity); + std::uninitialized_copy_n(old_data, buf.size(), new_data); + self.set(new_data, new_capacity); // deallocate must not throw according to the standard, but even if it does, // the buffer already uses the new storage and will deallocate it in // destructor. - if (old_data != store_) alloc_.deallocate(old_data, old_capacity); + if (old_data != self.store_) self.alloc_.deallocate(old_data, old_capacity); } public: @@ -909,7 +911,7 @@ class basic_memory_buffer final : public detail::buffer { FMT_CONSTEXPR20 explicit basic_memory_buffer( const Allocator& alloc = Allocator()) - : alloc_(alloc) { + : detail::buffer(grow), alloc_(alloc) { this->set(store_, SIZE); if (detail::is_constant_evaluated()) detail::fill_n(store_, SIZE, T()); } @@ -941,7 +943,8 @@ class basic_memory_buffer final : public detail::buffer { of the other object to it. \endrst */ - FMT_CONSTEXPR20 basic_memory_buffer(basic_memory_buffer&& other) noexcept { + FMT_CONSTEXPR20 basic_memory_buffer(basic_memory_buffer&& other) noexcept + : detail::buffer(grow) { move(other); } diff --git a/include/fmt/os.h b/include/fmt/os.h index 4ddcd9ce..6009ccc1 100644 --- a/include/fmt/os.h +++ b/include/fmt/os.h @@ -377,9 +377,10 @@ struct ostream_params { }; class file_buffer final : public buffer { + private: file file_; - FMT_API void grow(size_t) override; + FMT_API static void grow(buffer& buf, size_t); public: FMT_API file_buffer(cstring_view path, const ostream_params& params); diff --git a/src/os.cc b/src/os.cc index 4b277a1d..7813c433 100644 --- a/src/os.cc +++ b/src/os.cc @@ -370,18 +370,17 @@ long getpagesize() { namespace detail { -void file_buffer::grow(size_t) { - if (this->size() == this->capacity()) flush(); +void file_buffer::grow(buffer& buf, size_t) { + if (buf.size() == buf.capacity()) static_cast(buf).flush(); } -file_buffer::file_buffer(cstring_view path, - const detail::ostream_params& params) - : file_(path, params.oflag) { +file_buffer::file_buffer(cstring_view path, const ostream_params& params) + : buffer(grow), file_(path, params.oflag) { set(new char[params.buffer_size], params.buffer_size); } file_buffer::file_buffer(file_buffer&& other) - : detail::buffer(other.data(), other.size(), other.capacity()), + : buffer(grow, other.data(), other.size(), other.capacity()), file_(std::move(other.file_)) { other.clear(); other.set(nullptr, 0); diff --git a/test/core-test.cc b/test/core-test.cc index b505e2ab..db2cbc39 100644 --- a/test/core-test.cc +++ b/test/core-test.cc @@ -143,11 +143,12 @@ TEST(buffer_test, indestructible) { template struct mock_buffer final : buffer { MOCK_METHOD(size_t, do_grow, (size_t)); - void grow(size_t capacity) override { - this->set(this->data(), do_grow(capacity)); + static void grow(buffer& buf, size_t capacity) { + auto& self = static_cast(buf); + self.set(buf.data(), self.do_grow(capacity)); } - mock_buffer(T* data = nullptr, size_t buf_capacity = 0) { + mock_buffer(T* data = nullptr, size_t buf_capacity = 0) : buffer(grow) { this->set(data, buf_capacity); ON_CALL(*this, do_grow(_)).WillByDefault(Invoke([](size_t capacity) { return capacity; @@ -443,8 +444,9 @@ struct check_custom { -> test_result { struct test_buffer final : fmt::detail::buffer { char data[10]; - test_buffer() : fmt::detail::buffer(data, 0, 10) {} - void grow(size_t) override {} + test_buffer() + : fmt::detail::buffer([](buffer&, size_t) {}, data, 0, + 10) {} } buffer; auto parse_ctx = fmt::format_parse_context(""); auto ctx = fmt::format_context(fmt::detail::buffer_appender(buffer), diff --git a/test/ostream-test.cc b/test/ostream-test.cc index 98ee0757..fd02a656 100644 --- a/test/ostream-test.cc +++ b/test/ostream-test.cc @@ -135,8 +135,8 @@ TEST(ostream_test, write_to_ostream_max_size) { struct test_buffer final : fmt::detail::buffer { explicit test_buffer(size_t size) - : fmt::detail::buffer(nullptr, size, size) {} - void grow(size_t) override {} + : fmt::detail::buffer([](buffer&, size_t) {}, nullptr, size, + size) {} } buffer(max_size); struct mock_streambuf : std::streambuf { @@ -292,8 +292,7 @@ TEST(ostream_test, closed_ofstream) { struct unlocalized {}; -auto operator<<(std::ostream& os, unlocalized) - -> std::ostream& { +auto operator<<(std::ostream& os, unlocalized) -> std::ostream& { return os << 12345; }