Page MenuHomePhabricator
Differential D19046

refactor: use fold expressions instead of recursive calls in (Un)SerializeMany()
Needs ReviewPublic
Actions

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

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Summary

Instead of recursively calling SerializeMany and peeling off one
argument at a time, use a fold expression. This simplifies the code,
makes it most likely faster because it reduces the number of function
calls, and compiles faster because there are fewer template
instantiations.

This is a partial backport of core#28203
https://github.com/bitcoin/bitcoin/pull/28203/commits/bd08a008b42dac921bd9c031637e378899c1cd1d
https://github.com/bitcoin/bitcoin/pull/28203/commits/1403d181c106bc271ad2522adebde07c7850069b

Test Plan

ninja all check-all

Diff Detail

Event Timeline

PiRK created this revision.Thu, Dec 4, 14:12
Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Dec 4, 14:12
PiRK requested review of this revision.Thu, Dec 4, 14:12
teamcity edited the summary of this revision. (Show Details)Thu, Dec 4, 14:12
Harbormaster completed remote builds in B35230: Diff 56864.Thu, Dec 4, 14:23
teamcity added a comment.Thu, Dec 4, 14:23
Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Tail of the build log:

PASSED                         [ 20%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.048' duration='10' flowId='tests.test_iguana.test_invalid_format' name='tests.test_iguana.test_invalid_format']

tests/test_iguana.py::test_invalid_inputindex ##teamcity[testStarted timestamp='2025-12-04T14:20:14.048' captureStandardOutput='false' flowId='tests.test_iguana.test_invalid_inputindex' metainfo='test_invalid_inputindex' name='tests.test_iguana.test_invalid_inputindex']
PASSED                     [ 25%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.060' duration='10' flowId='tests.test_iguana.test_invalid_inputindex' name='tests.test_iguana.test_invalid_inputindex']

tests/test_iguana.py::test_sig_push_only ##teamcity[testStarted timestamp='2025-12-04T14:20:14.060' captureStandardOutput='false' flowId='tests.test_iguana.test_sig_push_only' metainfo='test_sig_push_only' name='tests.test_iguana.test_sig_push_only']
PASSED                          [ 30%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.117' duration='55' flowId='tests.test_iguana.test_sig_push_only' name='tests.test_iguana.test_sig_push_only']

tests/test_iguana.py::test_script_sig_success ##teamcity[testStarted timestamp='2025-12-04T14:20:14.117' captureStandardOutput='false' flowId='tests.test_iguana.test_script_sig_success' metainfo='test_script_sig_success' name='tests.test_iguana.test_script_sig_success']
PASSED                     [ 35%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.164' duration='46' flowId='tests.test_iguana.test_script_sig_success' name='tests.test_iguana.test_script_sig_success']

tests/test_iguana.py::test_script_sig_invalid_opcode_encoding ##teamcity[testStarted timestamp='2025-12-04T14:20:14.165' captureStandardOutput='false' flowId='tests.test_iguana.test_script_sig_invalid_opcode_encoding' metainfo='test_script_sig_invalid_opcode_encoding' name='tests.test_iguana.test_script_sig_invalid_opcode_encoding']
PASSED     [ 40%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.229' duration='63' flowId='tests.test_iguana.test_script_sig_invalid_opcode_encoding' name='tests.test_iguana.test_script_sig_invalid_opcode_encoding']

tests/test_iguana.py::test_script_pub_key_success ##teamcity[testStarted timestamp='2025-12-04T14:20:14.230' captureStandardOutput='false' flowId='tests.test_iguana.test_script_pub_key_success' metainfo='test_script_pub_key_success' name='tests.test_iguana.test_script_pub_key_success']
PASSED                 [ 45%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.296' duration='65' flowId='tests.test_iguana.test_script_pub_key_success' name='tests.test_iguana.test_script_pub_key_success']

tests/test_iguana.py::test_script_pub_key_failure ##teamcity[testStarted timestamp='2025-12-04T14:20:14.297' captureStandardOutput='false' flowId='tests.test_iguana.test_script_pub_key_failure' metainfo='test_script_pub_key_failure' name='tests.test_iguana.test_script_pub_key_failure']
PASSED                 [ 50%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.330' duration='32' flowId='tests.test_iguana.test_script_pub_key_failure' name='tests.test_iguana.test_script_pub_key_failure']

tests/test_iguana.py::test_script_pub_key_empty_stack ##teamcity[testStarted timestamp='2025-12-04T14:20:14.330' captureStandardOutput='false' flowId='tests.test_iguana.test_script_pub_key_empty_stack' metainfo='test_script_pub_key_empty_stack' name='tests.test_iguana.test_script_pub_key_empty_stack']
PASSED             [ 55%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.352' duration='21' flowId='tests.test_iguana.test_script_pub_key_empty_stack' name='tests.test_iguana.test_script_pub_key_empty_stack']

tests/test_iguana.py::test_script_pub_key_false_stack ##teamcity[testStarted timestamp='2025-12-04T14:20:14.353' captureStandardOutput='false' flowId='tests.test_iguana.test_script_pub_key_false_stack' metainfo='test_script_pub_key_false_stack' name='tests.test_iguana.test_script_pub_key_false_stack']
PASSED             [ 60%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.376' duration='22' flowId='tests.test_iguana.test_script_pub_key_false_stack' name='tests.test_iguana.test_script_pub_key_false_stack']

tests/test_iguana.py::test_script_pub_key_cleanstack ##teamcity[testStarted timestamp='2025-12-04T14:20:14.377' captureStandardOutput='false' flowId='tests.test_iguana.test_script_pub_key_cleanstack' metainfo='test_script_pub_key_cleanstack' name='tests.test_iguana.test_script_pub_key_cleanstack']
PASSED              [ 65%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.408' duration='30' flowId='tests.test_iguana.test_script_pub_key_cleanstack' name='tests.test_iguana.test_script_pub_key_cleanstack']

tests/test_iguana.py::test_redeem_script_success ##teamcity[testStarted timestamp='2025-12-04T14:20:14.409' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_success' metainfo='test_redeem_script_success' name='tests.test_iguana.test_redeem_script_success']
PASSED                  [ 70%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.460' duration='50' flowId='tests.test_iguana.test_redeem_script_success' name='tests.test_iguana.test_redeem_script_success']

tests/test_iguana.py::test_redeem_script_error ##teamcity[testStarted timestamp='2025-12-04T14:20:14.461' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_error' metainfo='test_redeem_script_error' name='tests.test_iguana.test_redeem_script_error']
PASSED                    [ 75%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.484' duration='22' flowId='tests.test_iguana.test_redeem_script_error' name='tests.test_iguana.test_redeem_script_error']

tests/test_iguana.py::test_redeem_script_exception ##teamcity[testStarted timestamp='2025-12-04T14:20:14.485' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_exception' metainfo='test_redeem_script_exception' name='tests.test_iguana.test_redeem_script_exception']
PASSED                [ 80%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.509' duration='23' flowId='tests.test_iguana.test_redeem_script_exception' name='tests.test_iguana.test_redeem_script_exception']

tests/test_iguana.py::test_redeem_script_empty_stack ##teamcity[testStarted timestamp='2025-12-04T14:20:14.510' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_empty_stack' metainfo='test_redeem_script_empty_stack' name='tests.test_iguana.test_redeem_script_empty_stack']
PASSED              [ 85%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.523' duration='12' flowId='tests.test_iguana.test_redeem_script_empty_stack' name='tests.test_iguana.test_redeem_script_empty_stack']

tests/test_iguana.py::test_redeem_script_false ##teamcity[testStarted timestamp='2025-12-04T14:20:14.524' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_false' metainfo='test_redeem_script_false' name='tests.test_iguana.test_redeem_script_false']
PASSED                    [ 90%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.537' duration='12' flowId='tests.test_iguana.test_redeem_script_false' name='tests.test_iguana.test_redeem_script_false']

tests/test_iguana.py::test_redeem_script_cleanstack ##teamcity[testStarted timestamp='2025-12-04T14:20:14.538' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_cleanstack' metainfo='test_redeem_script_cleanstack' name='tests.test_iguana.test_redeem_script_cleanstack']
PASSED               [ 95%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.551' duration='12' flowId='tests.test_iguana.test_redeem_script_cleanstack' name='tests.test_iguana.test_redeem_script_cleanstack']

tests/test_iguana.py::test_redeem_script_input_sigchecks ##teamcity[testStarted timestamp='2025-12-04T14:20:14.552' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_input_sigchecks' metainfo='test_redeem_script_input_sigchecks' name='tests.test_iguana.test_redeem_script_input_sigchecks']
PASSED          [100%]##teamcity[testFinished timestamp='2025-12-04T14:20:14.582' duration='30' flowId='tests.test_iguana.test_redeem_script_input_sigchecks' name='tests.test_iguana.test_redeem_script_input_sigchecks']


============================== 20 passed in 0.69s ==============================
[219/501] Running pow test suite
PASSED: pow test suite
[223/501] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[486/501] Running bitcoin test suite
PASSED: bitcoin test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1
teamcity added a comment.Thu, Dec 4, 14:27
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Tail of the build log:

wallet_watchonly.py                              | ✓ Passed  | 1 s
wallet_watchonly.py --usecli                     | ✓ Passed  | 1 s
chronik_avalanche.py                             | ○ Skipped | 0 s
chronik_block.py                                 | ○ Skipped | 0 s
chronik_block_header.py                          | ○ Skipped | 0 s
chronik_block_info.py                            | ○ Skipped | 0 s
chronik_block_txs.py                             | ○ Skipped | 0 s
chronik_blockchain_info.py                       | ○ Skipped | 0 s
chronik_blocks.py                                | ○ Skipped | 0 s
chronik_chronik_info.py                          | ○ Skipped | 0 s
chronik_cors.py                                  | ○ Skipped | 0 s
chronik_disable_token_index.py                   | ○ Skipped | 0 s
chronik_disallow_prune.py                        | ○ Skipped | 0 s
chronik_electrum_basic.py                        | ○ Skipped | 0 s
chronik_electrum_blockchain.py                   | ○ Skipped | 0 s
chronik_lokad_id_group.py                        | ○ Skipped | 0 s
chronik_mempool_conflicts.py                     | ○ Skipped | 0 s
chronik_mempool_disconnectpool.py                | ○ Skipped | 0 s
chronik_pause.py                                 | ○ Skipped | 0 s
chronik_plugin_groups.py                         | ○ Skipped | 0 s
chronik_plugins.py                               | ○ Skipped | 0 s
chronik_plugins_setup.py                         | ○ Skipped | 0 s
chronik_raw_tx.py                                | ○ Skipped | 0 s
chronik_resync.py                                | ○ Skipped | 0 s
chronik_script_confirmed_txs.py                  | ○ Skipped | 0 s
chronik_script_history.py                        | ○ Skipped | 0 s
chronik_script_unconfirmed_txs.py                | ○ Skipped | 0 s
chronik_script_utxos.py                          | ○ Skipped | 0 s
chronik_scripthash.py                            | ○ Skipped | 0 s
chronik_serve.py                                 | ○ Skipped | 0 s
chronik_shutdown.py                              | ○ Skipped | 0 s
chronik_spent_by.py                              | ○ Skipped | 0 s
chronik_token_alp.py                             | ○ Skipped | 0 s
chronik_token_broadcast_txs.py                   | ○ Skipped | 0 s
chronik_token_burn.py                            | ○ Skipped | 0 s
chronik_token_id_group.py                        | ○ Skipped | 0 s
chronik_token_parse_failure.py                   | ○ Skipped | 0 s
chronik_token_script_group.py                    | ○ Skipped | 0 s
chronik_token_slp_fungible.py                    | ○ Skipped | 0 s
chronik_token_slp_mint_vault.py                  | ○ Skipped | 0 s
chronik_token_slp_nft1.py                        | ○ Skipped | 0 s
chronik_tx.py                                    | ○ Skipped | 0 s
chronik_tx_removal_order.py                      | ○ Skipped | 0 s
chronik_unconfirmed_txs.py                       | ○ Skipped | 0 s
chronik_ws.py                                    | ○ Skipped | 0 s
chronik_ws_avalanche.py                          | ○ Skipped | 0 s
chronik_ws_ordering.py                           | ○ Skipped | 0 s
chronik_ws_ping.py                               | ○ Skipped | 0 s
chronik_ws_script.py                             | ○ Skipped | 0 s
feature_bind_port_discover.py                    | ○ Skipped | 0 s
feature_bind_port_externalip.py                  | ○ Skipped | 0 s
interface_usdt_net.py                            | ○ Skipped | 0 s
interface_usdt_utxocache.py                      | ○ Skipped | 0 s
interface_usdt_validation.py                     | ○ Skipped | 0 s

ALL                                              | ✓ Passed  | 1200 s (accumulated) 
Runtime: 109 s

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1
Fabien requested changes to this revision.Thu, Dec 4, 14:35
Fabien added a subscriber: Fabien.

back to your queue

This revision now requires changes to proceed.Thu, Dec 4, 14:35
PiRK updated this revision to Diff 56878.Thu, Dec 4, 15:38

try reserving some space in the stream to avoid a false positive [-Werror=array-bounds]

Maybe the fold expression results in more aggressive inlining by the compiler, and it can't handle the complex serialization

PiRK added inline comments.Thu, Dec 4, 15:40
src/seeder/test/db_tests.cpp
41 ↗(On Diff #56878)

no reason to move the version, i will revert this move (after the CI completes)

The CDataStream is moved down because there is no need to initialize it tens of lines before it is used

Harbormaster completed remote builds in B35233: Diff 56878.Thu, Dec 4, 16:08
PiRK updated this revision to Diff 56879.Thu, Dec 4, 16:38

no reason to move the version

Harbormaster completed remote builds in B35234: Diff 56879.Thu, Dec 4, 16:58

Revision Contents
Changeset List

PathSize
src/
seeder/
test/
5 lines
18 lines

Diff 56879

View Options

src/seeder/test/db_tests.cpp

Loading...
View Options

src/serialize.h

Loading...