Skip to content

Commit 5cce4d2

Browse files
committed
Merge bitcoin/bitcoin#27334: util: implement noexcept move assignment & move ctor for prevector
bfb9291 util: implement prevector's move ctor & move assignment (Martin Leitner-Ankerl) fffc86f test: CScriptCheck is used a lot in std::vector, make sure that's efficient (Martin Leitner-Ankerl) 81f6797 util: prevector's move ctor and move assignment is `noexcept` (Martin Leitner-Ankerl) d380d28 bench: Add benchmark for prevector usage in std::vector (Martin Leitner-Ankerl) Pull request description: `prevector`'s move assignment and move constructor were not `noexcept`, which makes it inefficient to use inside STL containers like `std::vector`. That's the case e.g. for `CScriptCheck`. This PR adds `noexcept`, and also implements the move assignment & ctor, which makes it quite a bit more efficient to use prevector in an std::vector. The PR also adds a benchmark which grows an `std::vector` by adding `prevector` objects to it. merge-base: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 6,440.29 | 155,272.42 | 0.2% | 40,713.01 | 20,473.84 | 1.989 | 7,132.01 | 0.2% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 3,213.19 | 311,217.35 | 0.7% | 35,373.01 | 10,214.07 | 3.463 | 6,945.00 | 0.2% | 0.43 | `PrevectorFillVectorDirectTrivial` | 34,749.70 | 28,777.23 | 0.1% | 364,396.05 | 110,521.94 | 3.297 | 78,568.37 | 0.1% | 0.43 | `PrevectorFillVectorIndirectNontrivial` | 32,535.05 | 30,736.09 | 0.4% | 353,823.31 | 103,464.53 | 3.420 | 79,871.80 | 0.2% | 0.40 | `PrevectorFillVectorIndirectTrivial` util: prevector's move ctor and move assignment is `noexcept`: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 6,603.87 | 151,426.40 | 0.2% | 23,734.01 | 21,009.63 | 1.130 | 2,445.01 | 0.3% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 1,980.93 | 504,813.15 | 0.1% | 13,784.00 | 6,304.32 | 2.186 | 2,258.00 | 0.3% | 0.44 | `PrevectorFillVectorDirectTrivial` | 19,110.54 | 52,327.15 | 0.1% | 139,816.41 | 51,987.72 | 2.689 | 28,512.18 | 0.1% | 0.43 | `PrevectorFillVectorIndirectNontrivial` | 12,334.37 | 81,074.27 | 0.7% | 125,655.12 | 39,253.58 | 3.201 | 27,854.46 | 0.2% | 0.44 | `PrevectorFillVectorIndirectTrivial` util: implement prevector's move ctor & move assignment | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 5,262.66 | 190,018.01 | 0.2% | 20,157.01 | 16,745.26 | 1.204 | 2,445.01 | 0.3% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 1,687.07 | 592,744.35 | 0.2% | 12,742.00 | 5,368.02 | 2.374 | 2,258.00 | 0.3% | 0.44 | `PrevectorFillVectorDirectTrivial` | 17,930.80 | 55,769.95 | 0.1% | 136,237.69 | 47,903.31 | 2.844 | 28,512.02 | 0.2% | 0.42 | `PrevectorFillVectorIndirectNontrivial` | 11,893.75 | 84,077.78 | 0.2% | 126,182.02 | 37,852.91 | 3.333 | 28,152.01 | 0.1% | 0.44 | `PrevectorFillVectorIndirectTrivial` As can be seen, mostly thanks to just `noexcept` the benchmark becomes about 2 times faster because `std::vector` can now use move operations instead of having to fall back to copy everything I had a look at how this change affects the other benchmarks, and they are all pretty much the same, the only noticable difference is `CCheckQueueSpeedPrevectorJob` goes from 364.56ns down to 346.21ns. ACKs for top commit: achow101: ACK bfb9291 jonatack: > Tested Approach ACK [bfb9291](bitcoin/bitcoin@bfb9291), ~ACK modulo re-verifying the implementation of the last commit. john-moffett: Approach ACK bfb9291 theStack: Tested and light code-review ACK bfb9291 Tree-SHA512: 242995d7cb2f8ebfa73177aa690a505f189d91edeb8cea3e34a41ca507c8cb17c65fe2a4e196fdafc5c6e89b2b2222627bfc9f5c316517de0857b7b5e9c58225
2 parents 7ee4121 + bfb9291 commit 5cce4d2

File tree

3 files changed

+43
-4
lines changed

3 files changed

+43
-4
lines changed

src/bench/prevector.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,30 @@ static void PrevectorDeserialize(benchmark::Bench& bench)
8080
});
8181
}
8282

83+
template <typename T>
84+
static void PrevectorFillVectorDirect(benchmark::Bench& bench)
85+
{
86+
bench.run([&] {
87+
std::vector<prevector<28, T>> vec;
88+
for (size_t i = 0; i < 260; ++i) {
89+
vec.emplace_back();
90+
}
91+
});
92+
}
93+
94+
95+
template <typename T>
96+
static void PrevectorFillVectorIndirect(benchmark::Bench& bench)
97+
{
98+
bench.run([&] {
99+
std::vector<prevector<28, T>> vec;
100+
for (size_t i = 0; i < 260; ++i) {
101+
// force allocation
102+
vec.emplace_back(29, T{});
103+
}
104+
});
105+
}
106+
83107
#define PREVECTOR_TEST(name) \
84108
static void Prevector##name##Nontrivial(benchmark::Bench& bench) \
85109
{ \
@@ -96,3 +120,5 @@ PREVECTOR_TEST(Clear)
96120
PREVECTOR_TEST(Destructor)
97121
PREVECTOR_TEST(Resize)
98122
PREVECTOR_TEST(Deserialize)
123+
PREVECTOR_TEST(FillVectorDirect)
124+
PREVECTOR_TEST(FillVectorIndirect)

src/prevector.h

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -264,8 +264,10 @@ class prevector {
264264
fill(item_ptr(0), other.begin(), other.end());
265265
}
266266

267-
prevector(prevector<N, T, Size, Diff>&& other) {
268-
swap(other);
267+
prevector(prevector<N, T, Size, Diff>&& other) noexcept
268+
: _union(std::move(other._union)), _size(other._size)
269+
{
270+
other._size = 0;
269271
}
270272

271273
prevector& operator=(const prevector<N, T, Size, Diff>& other) {
@@ -276,8 +278,13 @@ class prevector {
276278
return *this;
277279
}
278280

279-
prevector& operator=(prevector<N, T, Size, Diff>&& other) {
280-
swap(other);
281+
prevector& operator=(prevector<N, T, Size, Diff>&& other) noexcept {
282+
if (!is_direct()) {
283+
free(_union.indirect_contents.indirect);
284+
}
285+
_union = std::move(other._union);
286+
_size = other._size;
287+
other._size = 0;
281288
return *this;
282289
}
283290

src/validation.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
#include <stdint.h>
4444
#include <string>
4545
#include <thread>
46+
#include <type_traits>
4647
#include <utility>
4748
#include <vector>
4849

@@ -330,6 +331,11 @@ class CScriptCheck
330331
ScriptError GetScriptError() const { return error; }
331332
};
332333

334+
// CScriptCheck is used a lot in std::vector, make sure that's efficient
335+
static_assert(std::is_nothrow_move_assignable_v<CScriptCheck>);
336+
static_assert(std::is_nothrow_move_constructible_v<CScriptCheck>);
337+
static_assert(std::is_nothrow_destructible_v<CScriptCheck>);
338+
333339
/** Initializes the script-execution cache */
334340
[[nodiscard]] bool InitScriptExecutionCache(size_t max_size_bytes);
335341

0 commit comments

Comments
 (0)