Page MenuHomePhabricator
Differential D19048

refactor: serialization simplifications
ClosedPublic
Actions

Authored by PiRK on Thu, Dec 4, 14:59.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCa241f4c0efc1: refactor: serialization simplifications
Summary

This is a code simplification and an optimization when serializing (pre)vectors of std::byte and int8_t

refactor: use "if constexpr" in (pre)vector's (Un)Serialize()

This gets rid of unnecessarily creating a temporary object T() to call the right function.

https://github.com/bitcoin/bitcoin/pull/28203/commits/c8839ec5cd81ba9ae88081747c49ecc758973dd1
https://github.com/bitcoin/bitcoin/pull/28203/commits/0fafaca4d3bbf0c0b5bfe1ec617ab15252ea51e6
https://github.com/bitcoin/bitcoin/pull/28203/commits/088caa68fb8efd8624709d643913b8a7e1218f8a
https://github.com/bitcoin/bitcoin/pull/28203/commits/f054bd072afb72d8dae7adc521ce15c13b236700

Faster std::byte (pre)vector (un)serialize

https://github.com/bitcoin/bitcoin/pull/29114/commits/facaa14785e006c1af5a8b17b10e2722af8d054e

Allow int8_t optimized vector serialization

int8_t serialization is allowed, but not the optimized vector
serialization. Fix that.

https://github.com/bitcoin/bitcoin/pull/29114/commits/fab41697a5448ef2861f65795bd63a4ccdda6a40

This concludes backport of core#28203 and core#29114

Test Plan

ninja all check-all

Benchmark with this patch:

diff --git a/src/bench/prevector.cpp b/src/bench/prevector.cpp
index d595101e3f..4240010ca2 100644
--- a/src/bench/prevector.cpp
+++ b/src/bench/prevector.cpp
@@ -28,7 +28,7 @@ struct nontrivial_t {
 static_assert(!IS_TRIVIALLY_CONSTRUCTIBLE<nontrivial_t>::value,
               "expected nontrivial_t to not be trivially constructible");

-typedef uint8_t trivial_t;
+typedef std::byte trivial_t;
 static_assert(IS_TRIVIALLY_CONSTRUCTIBLE<trivial_t>::value,
               "expected trivial_t to be trivially constructible");

Before:

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|               76.43 |       13,083,011.71 |    0.4% |      0.01 | `PrevectorDeserializeTrivial`

After:

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|                9.42 |      106,201,341.40 |    0.2% |      0.01 | `PrevectorDeserializeTrivial`

Diff Detail

Event Timeline

PiRK created this revision.Thu, Dec 4, 14:59
Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Dec 4, 14:59
PiRK requested review of this revision.Thu, Dec 4, 14:59
teamcity edited the summary of this revision. (Show Details)Thu, Dec 4, 14:59
PiRK added a comment.Thu, Dec 4, 15:01

This will prevent unexpected perf regressions as we continue transitionning stuff from uint8_t to std::byte

Harbormaster completed remote builds in B35232: Diff 56866.Thu, Dec 4, 15:19
Fabien accepted this revision.Thu, Dec 4, 15:28
This revision is now accepted and ready to land.Thu, Dec 4, 15:28
Closed by commit rABCa241f4c0efc1: refactor: serialization simplifications (authored by Martin Leitner-Ankerl <martin.ankerl@gmail.com>, committed by PiRK). · Explain WhyThu, Dec 4, 16:56
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABCa241f4c0efc1: refactor: serialization simplifications.

Revision Contents
Changeset List

PathSize
src/
155 lines
6 lines

Diff 56866

View Options

src/serialize.h

Loading...
View Options

src/span.h

Loading...