From e6741d080dc5a694f58ec1c14bfea331a756fc1a Mon Sep 17 00:00:00 2001 From: random-zebra Date: Sat, 19 Dec 2020 18:25:37 +0100 Subject: [PATCH 1/4] Add new prevector benchmarks. >>> backports bitcoin/bitcoin@f0e7aa702095b22ba57a763c5c093e15d41586d1 This prepares for a series of two additional commits which optimize prevector performance. --- src/Makefile.bench.include | 2 +- src/bench/prevector.cpp | 77 ++++++++++++++++++++++++++++++ src/bench/prevector_destructor.cpp | 36 -------------- src/compat.h | 10 ++++ 4 files changed, 88 insertions(+), 37 deletions(-) create mode 100644 src/bench/prevector.cpp delete mode 100644 src/bench/prevector_destructor.cpp diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 2ea719861a19..480aa5c8d92c 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -13,7 +13,7 @@ bench_bench_pivx_SOURCES = \ bench/crypto_hash.cpp \ bench/perf.cpp \ bench/perf.h \ - bench/prevector_destructor.cpp + bench/prevector.cpp bench_bench_pivx_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/ bench_bench_pivx_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) diff --git a/src/bench/prevector.cpp b/src/bench/prevector.cpp new file mode 100644 index 000000000000..48d63172b026 --- /dev/null +++ b/src/bench/prevector.cpp @@ -0,0 +1,77 @@ +// Copyright (c) 2015-2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "bench/bench.h" + +#include "compat.h" +#include "prevector.h" + +struct nontrivial_t { + int x; + nontrivial_t() :x(-1) {} +}; +static_assert(!IS_TRIVIALLY_CONSTRUCTIBLE::value, + "expected nontrivial_t to not be trivially constructible"); + +typedef unsigned char trivial_t; +static_assert(IS_TRIVIALLY_CONSTRUCTIBLE::value, + "expected trivial_t to be trivially constructible"); + +template +static void PrevectorDestructor(benchmark::State& state) +{ + while (state.KeepRunning()) { + for (auto x = 0; x < 1000; ++x) { + prevector<28, T> t0; + prevector<28, T> t1; + t0.resize(28); + t1.resize(29); + } + } +} + +template +static void PrevectorClear(benchmark::State& state) +{ + + while (state.KeepRunning()) { + for (auto x = 0; x < 1000; ++x) { + prevector<28, T> t0; + prevector<28, T> t1; + t0.resize(28); + t0.clear(); + t1.resize(29); + t0.clear(); + } + } +} + +template +void PrevectorResize(benchmark::State& state) +{ + while (state.KeepRunning()) { + prevector<28, T> t0; + prevector<28, T> t1; + for (auto x = 0; x < 1000; ++x) { + t0.resize(28); + t0.resize(0); + t1.resize(29); + t1.resize(0); + } + } +} + +#define PREVECTOR_TEST(name) \ + static void Prevector ## name ## Nontrivial(benchmark::State& state) { \ + PrevectorResize(state); \ + } \ + BENCHMARK(Prevector ## name ## Nontrivial); \ + static void Prevector ## name ## Trivial(benchmark::State& state) { \ + PrevectorResize(state); \ + } \ + BENCHMARK(Prevector ## name ## Trivial); + +PREVECTOR_TEST(Clear) +PREVECTOR_TEST(Destructor) +PREVECTOR_TEST(Resize) diff --git a/src/bench/prevector_destructor.cpp b/src/bench/prevector_destructor.cpp deleted file mode 100644 index aa178e6ad404..000000000000 --- a/src/bench/prevector_destructor.cpp +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright (c) 2015-2017 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include "bench.h" -#include "prevector.h" - -static void PrevectorDestructor(benchmark::State& state) -{ - while (state.KeepRunning()) { - for (auto x = 0; x < 1000; ++x) { - prevector<28, unsigned char> t0; - prevector<28, unsigned char> t1; - t0.resize(28); - t1.resize(29); - } - } -} - -static void PrevectorClear(benchmark::State& state) -{ - - while (state.KeepRunning()) { - for (auto x = 0; x < 1000; ++x) { - prevector<28, unsigned char> t0; - prevector<28, unsigned char> t1; - t0.resize(28); - t0.clear(); - t1.resize(29); - t0.clear(); - } - } -} - -BENCHMARK(PrevectorDestructor); -BENCHMARK(PrevectorClear); \ No newline at end of file diff --git a/src/compat.h b/src/compat.h index f51f9df96520..57e7dd2d0f6d 100644 --- a/src/compat.h +++ b/src/compat.h @@ -11,6 +11,16 @@ #include "config/pivx-config.h" #endif +#include + +// GCC 4.8 is missing some C++11 type_traits, +// https://www.gnu.org/software/gcc/gcc-5/changes.html +#if defined(__GNUC__) && __GNUC__ < 5 +#define IS_TRIVIALLY_CONSTRUCTIBLE std::is_trivial +#else +#define IS_TRIVIALLY_CONSTRUCTIBLE std::is_trivially_constructible +#endif + #ifdef WIN32 #ifdef _WIN32_WINNT #undef _WIN32_WINNT From c412d5fa6cc4a91a314d124350b5d1256962f98f Mon Sep 17 00:00:00 2001 From: random-zebra Date: Sat, 19 Dec 2020 18:51:36 +0100 Subject: [PATCH 2/4] Reduce redundant code of prevector and speed it up >>> based on bitcoin/bitcoin@e46be25f0e19d574157752a5c0b674907a1578e6 (updated with more recent changes) In prevector.h, the code which like item_ptr(size()) apears in the loop. Both item_ptr() and size() judge whether values are held directly or indirectly, but in most cases it is sufficient to make that judgement once outside the loop. This PR adds 2 private function fill() which has the loop to initialize by specified value (or iterator of the other prevector's element), but don't call item_ptr() in their loop. Other functions(assign(), constructor, operator=(), insert()) that has similar loop, call fill() instead of original loop. Also, resize() was changed like fill(), but it calls the default constructor for that element each time. --- src/prevector.h | 87 ++++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 45 deletions(-) diff --git a/src/prevector.h b/src/prevector.h index 14aa1ea8c9a2..a02afd37a7f0 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -194,16 +194,27 @@ class prevector { T* item_ptr(difference_type pos) { return is_direct() ? direct_ptr(pos) : indirect_ptr(pos); } const T* item_ptr(difference_type pos) const { return is_direct() ? direct_ptr(pos) : indirect_ptr(pos); } + void fill(T* dst, ptrdiff_t count, const T& value = T{}) { + std::fill_n(dst, count, value); + } + + template + void fill(T* dst, InputIterator first, InputIterator last) { + while (first != last) { + new(static_cast(dst)) T(*first); + ++dst; + ++first; + } + } + public: void assign(size_type n, const T& val) { clear(); if (capacity() < n) { change_capacity(n); } - while (size() < n) { - _size++; - new(static_cast(item_ptr(size() - 1))) T(val); - } + _size += n; + fill(item_ptr(0), n, val); } template @@ -213,11 +224,8 @@ class prevector { if (capacity() < n) { change_capacity(n); } - while (first != last) { - _size++; - new(static_cast(item_ptr(size() - 1))) T(*first); - ++first; - } + _size += n; + fill(item_ptr(0), first, last); } prevector() {} @@ -228,31 +236,23 @@ class prevector { explicit prevector(size_type n, const T& val = T()) { change_capacity(n); - while (size() < n) { - _size++; - new(static_cast(item_ptr(size() - 1))) T(val); - } + _size += n; + fill(item_ptr(0), n, val); } template prevector(InputIterator first, InputIterator last) { size_type n = last - first; change_capacity(n); - while (first != last) { - _size++; - new(static_cast(item_ptr(size() - 1))) T(*first); - ++first; - } + _size += n; + fill(item_ptr(0), first, last); } prevector(const prevector& other) { - change_capacity(other.size()); - const_iterator it = other.begin(); - while (it != other.end()) { - _size++; - new(static_cast(item_ptr(size() - 1))) T(*it); - ++it; - } + size_type n = other.size(); + change_capacity(n); + _size += n; + fill(item_ptr(0), other.begin(), other.end()); } prevector(prevector&& other) { @@ -263,14 +263,7 @@ class prevector { if (&other == this) { return *this; } - resize(0); - change_capacity(other.size()); - const_iterator it = other.begin(); - while (it != other.end()) { - _size++; - new(static_cast(item_ptr(size() - 1))) T(*it); - ++it; - } + assign(other.begin(), other.end()); return *this; } @@ -314,16 +307,20 @@ class prevector { } void resize(size_type new_size) { - if (size() > new_size) { + size_type cur_size = size(); + if (cur_size == new_size) { + return; + } + if (cur_size > new_size) { erase(item_ptr(new_size), end()); + return; } if (new_size > capacity()) { change_capacity(new_size); } - while (size() < new_size) { - _size++; - new(static_cast(item_ptr(size() - 1))) T(); - } + ptrdiff_t increase = new_size - cur_size; + fill(item_ptr(cur_size), increase); + _size += increase; } void reserve(size_type new_capacity) { @@ -346,10 +343,11 @@ class prevector { if (capacity() < new_size) { change_capacity(new_size + (new_size >> 1)); } - memmove(item_ptr(p + 1), item_ptr(p), (size() - p) * sizeof(T)); + T* ptr = item_ptr(p); + memmove(ptr + 1, ptr, (size() - p) * sizeof(T)); _size++; - new(static_cast(item_ptr(p))) T(value); - return iterator(item_ptr(p)); + new(static_cast(ptr)) T(value); + return iterator(ptr); } void insert(iterator pos, size_type count, const T& value) { @@ -358,11 +356,10 @@ class prevector { if (capacity() < new_size) { change_capacity(new_size + (new_size >> 1)); } - memmove(item_ptr(p + count), item_ptr(p), (size() - p) * sizeof(T)); + T* ptr = item_ptr(p); + memmove(ptr + count, ptr, (size() - p) * sizeof(T)); _size += count; - for (size_type i = 0; i < count; i++) { - new(static_cast(item_ptr(p + i))) T(value); - } + fill(item_ptr(p), count, value); } template From 4cb188ecb8b359aac888f1719fae0ece0345a5a7 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Tue, 13 Nov 2018 04:15:09 -0500 Subject: [PATCH 3/4] Drop defunct IS_TRIVIALLY_CONSTRUCTIBLE handling from prevector.h It's now only referenced from the bench, so leave it there. This allows us to drop the associated includes as well. --- src/bench/prevector.cpp | 12 ++++++++++++ src/compat.h | 10 ---------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/bench/prevector.cpp b/src/bench/prevector.cpp index 48d63172b026..f192fc370ff6 100644 --- a/src/bench/prevector.cpp +++ b/src/bench/prevector.cpp @@ -6,6 +6,18 @@ #include "compat.h" #include "prevector.h" +#include "serialize.h" +#include "streams.h" + +#include + +// GCC 4.8 is missing some C++11 type_traits, +// https://www.gnu.org/software/gcc/gcc-5/changes.html +#if defined(__GNUC__) && !defined(__clang__) && __GNUC__ < 5 +#define IS_TRIVIALLY_CONSTRUCTIBLE std::has_trivial_default_constructor +#else +#define IS_TRIVIALLY_CONSTRUCTIBLE std::is_trivially_default_constructible +#endif struct nontrivial_t { int x; diff --git a/src/compat.h b/src/compat.h index 57e7dd2d0f6d..f51f9df96520 100644 --- a/src/compat.h +++ b/src/compat.h @@ -11,16 +11,6 @@ #include "config/pivx-config.h" #endif -#include - -// GCC 4.8 is missing some C++11 type_traits, -// https://www.gnu.org/software/gcc/gcc-5/changes.html -#if defined(__GNUC__) && __GNUC__ < 5 -#define IS_TRIVIALLY_CONSTRUCTIBLE std::is_trivial -#else -#define IS_TRIVIALLY_CONSTRUCTIBLE std::is_trivially_constructible -#endif - #ifdef WIN32 #ifdef _WIN32_WINNT #undef _WIN32_WINNT From d856989513cafc9bb2292cbffc3a109c530de7de Mon Sep 17 00:00:00 2001 From: Lenny Maiorani Date: Sat, 3 Nov 2018 21:15:05 -0600 Subject: [PATCH 4/4] warnings: Compiler warning on memset usage for non-trivial type Problem: - IS_TRIVIALLY_CONSTRUCTIBLE macro does not work correctly resulting in `memset()` usage to set a non-trivial type to 0 when `nontrivial_t` is passed in from the tests. - Warning reported by GCC when compiling with `--enable-werror`. Solution: - Use the standard algorithm `std::fill_n()` and let the compiler determine the optimal way of looping or using `memset()`. --- src/prevector.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/prevector.h b/src/prevector.h index a02afd37a7f0..f3294870b46f 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -10,6 +10,8 @@ #include #include +#include +#include #include #include