bench: BlockAssembler on a mempool with packages #26695

glozow · 2022-12-13T16:38:39Z

Performance of block template building matters as miners likely want to be able to start mining on a block with transactions asap after a block is found. We would want to know if a mempool PR accidentally caused, for example, a 100x slowdown. An AssembleBlock() bench exists, but it operates on a mempool with 101 transactions, each with 0 ancestors or descendants and with the same fee. Adding a bench with a more complex mempool is useful because (1) it's more realistic (2) updating packages can potentially cause the algorithm to take a long time.

DrahtBot · 2022-12-13T16:38:42Z

DrahtBot commented

Dec 13, 2022

•

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	stickies-v, kevkevinpal, achow101
Concept ACK	pablomartin4btc
Stale ACK	jamesob, Emzy, hernanmarino

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

jamesob

Approach ACK

Change and idea are good, small issue with the first commit.

src/test/util/mining.h

jamesob

ACK 4c20bee (jamesob/ackr/26695.2.glozow.bench_blockassembler_on)

Nice! Good thing to have.

Could possibly later populate the mempool with a more stressful load, something
like 10,000 transactions?

Show signature data

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 4c20bee65009bd2c95c2a3426030b042f55007ad ([`jamesob/ackr/26695.2.glozow.bench_blockassembler_on`](https://github.com/jamesob/bitcoin/tree/ackr/26695.2.glozow.bench_blockassembler_on))

Nice! Good thing to have.

Could possibly later populate the mempool with a more stressful load, something
like 10,000 transactions? 
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEGNRVI1NPYuZCSIrGepNdrbLETwUFAmOZx4sACgkQepNdrbLE
TwVdwA//bHGKG97lUP5JPiknjulnwVzPvfPyXJi8azZQ2XoK3T1HEA/kJo31wHrY
2mdRgis+kst/emY7wfu/LAAlc06M8cKt5DuPfEgJIPzkidV8JAePMvw8QpDgbd/K
mTu+6mdeGq5Mspy0eS2ItsQEeC4NvZeQSIIf9yCSIhlnCBmeshqBEydN3tduW6Ot
nr6RKGizFO7yJ1ecWX+UA9eYNUYUz4isLehBgoyTh5hz0IJobWIFRmzVCXUQci0y
S3WxFf4RlGnJpWMOOwd/6WZHAdsFOtZ6T4Nux9HEC7nboxb+pfib47CvqCWc0kG3
4mAsJtxasIna/0HTMFD9gApOteG3NByNDKfOkhG1gZYu8tnRA6wm+61WKX6BgJT9
MHHd4WMlov9ye4YaQQazpV8ApXqbjMbFpOLxSeCn3vad/bXo4IWq2x+rpQZTpLJs
+pPNTvqahtk96njeqZcukbnu+JYuEbst4sSlOGMd29fV52M2lyOICK5Jt0PgjfBR
f/lZXhNdd7+1ydj5XOkKE4RAqaQhEf1FYhvR0u+HmFcIVUsEfIYQkvB4z89j28KI
hFGQD3eFPxFPdxBV32GMWHKQ52KBgvzL0Hbbe4a7D/FSbE8YAnRhFXx3jd4AhlAV
8xCDcN/eAthz45ZsTvvoF6NagwRfoXQoP4jzjzFIsNDGD0CKri0=
=cB3y
-----END PGP SIGNATURE-----

Show platform data

Tested on Linux-6.0.11-arch1-1-x86_64-with-glibc2.36

Configured with ./configure --without-bdb 'CXXFLAGS=-fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare ' --disable-gui-tests --disable-fuzz-binary --enable-wallet --enable-debug --with-daemon --enable-natpmp-default CXX=/usr/bin/clang++ CC=/usr/bin/clang PYTHONPATH= --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --enable-module-schnorrsig --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --enable-module-schnorrsig --no-create --no-recursion

Compiled with /usr/bin/ccache /usr/bin/clang++ -std=c++17 -mavx -mavx2 -mpclmul -fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare  -O0 -g3 -ftrapv -fdebug-prefix-map=$(abs_top_srcdir)=.  -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -msse4.1 -msse4.2 -msse4 -msha  i

Compiler version: clang version 14.0.6

maflcko

lgtm. Left some nits

src/test/util/mining.cpp

src/bench/block_assemble.cpp

stickies-v

Concept ACK - nice to have this kind of benchmark, will be helpful in e.g. #26289

src/test/util/mining.cpp

src/test/util/mining.h

src/node/miner.h

src/node/miner.cpp

src/test/util/setup_common.cpp

src/bench/block_assemble.cpp

stickies-v · 2022-12-14T14:57:47Z

src/bench/block_assemble.cpp

    });
 }

 BENCHMARK(AssembleBlock, benchmark::PriorityLevel::HIGH);
+BENCHMARK(BlockAssemblerAddPackageTxns, benchmark::PriorityLevel::HIGH);


This is among the slowest benchmarks in the suite atm. On my machine, --sanity-check takes ~1.2s. I think that's an acceptable additional slowdown for make check, but perhaps we should consider downgrading this to PriorityLevel::MEDIUM?

this is what I get when trying to run the BlockAssemblerAddPackageTxns benchmark

Specs:

mac book pro 2019 2.3 GHz 8-Core Intel Core i9 16 GB 2667 MHz DDR4 AMD Radeon Pro 5500M 4 GB

Results:

./src/bench/bench_bitcoin -filter=BlockAssemblerAddPackageTxns --sanity-check Running with --sanity-check option, benchmark results will be useless. | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 813,714,108.00 | 1.23 | 0.0% | 0.81 | `BlockAssemblerAddPackageTxns`

Without --sanity-check:

./src/bench/bench_bitcoin -filter=BlockAssemblerAddPackageTxns | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 830,079,779.00 | 1.20 | 1.4% | 9.16 | `BlockAssemblerAddPackageTxns`

When compiled with --enable-debug, this slows down make check by ~30s for me. When run without --sanity-check, it takes ~280s. I think this needs to be a low priority benchmark.

Changed to low, I think we don't have medium? Is priority based on time only, and if so at what time threshold would we want to knock the priority down?

Yeah sorry, we had medium in the initial PR but it was removed later on, so agreed with changing it to low. We haven't defined any guidelines on when to downgrade to low afaik, but since devs run make check often, I think adding benches with meaningful (e.g. >1s?) --sanity-check overhead should run in low priority - unless their complexity/importance warrants otherwise.

jamesob

ACK 628ce92 (jamesob/ackr/26695.3.glozow.bench_blockassembler_on)

re-ACK

Show signature data

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 628ce92c494c6d3b6181d2ee6f64494de38a67e3 ([`jamesob/ackr/26695.3.glozow.bench_blockassembler_on`](https://github.com/jamesob/bitcoin/tree/ackr/26695.3.glozow.bench_blockassembler_on))

re-ACK
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEGNRVI1NPYuZCSIrGepNdrbLETwUFAmOcpWIACgkQepNdrbLE
TwW/5w/+MpLKrQ5eLWc7uXQ2P1ML82Q1EKDXzGC4bmBFZZ2t7Ynj78enPUf1ukD2
o8hT28touxw5XSWF2Zcwjhh+DRWU6sqYJInpOXhW1366iqS2DcvDQZOctwEEUasO
RM6lcRfNNvz4FyuiV4Cfn/liVHK6ipCQoPY88mjyy/I32XzwB6wXKNopRCV8GfFV
pIU10BM9tVobwHJhFRvTQve3nhwXjIbwJ0Vn9peUqKRSmuxDkFf2R6l/3YUwEFjk
iPJQ+SW4fo0h6bDThZ8GOfT8F4u52cnsx97zOck9f6wmehQoKUDIj6s2Epa/n/0q
WZXx6+xI2RS3GuS7fM9et1+0H8TmyHhkvSpzkSiOg+npWBqPl+8tCgqQftUS4s+D
HY/BoP1arD2nHsQ/yBksymASmnFKMyR0RWz8r8JheUY3gGXRDCQZkUCkVMMjpV3O
tOIx6k1h8dfyaJqxW2e6o3r2dNho79Ap2nQS7UnRfFu80KebgyV0SZYusK3+OT4a
Wkm4VBO3l36BQJwOBmu6nQ3mpbgF4TvQOhj9F65GUaLrsAKrNn/YrcnV2U+g/RmS
JaXsSUio7DukkYccLLLOh9WvYOXkxohIXNA7DBpk+VES8PBDN5Q5ZBuCmJSRt0HC
VwSxSbNA4Pedk6pmaebOAgTUcQxY9yJOaBQLEqyhR3SRE4YgrVQ=
=rMYG
-----END PGP SIGNATURE-----

Show platform data

Tested on Linux-6.0.11-arch1-1-x86_64-with-glibc2.36

Configured with ./configure --without-bdb 'CXXFLAGS=-fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare ' --disable-gui-tests --disable-fuzz-binary --enable-wallet --enable-debug --with-daemon --enable-natpmp-default CXX=/usr/bin/clang++ CC=/usr/bin/clang PYTHONPATH= --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --enable-module-schnorrsig --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --enable-module-schnorrsig --no-create --no-recursion

Compiled with /usr/bin/ccache /usr/bin/clang++ -std=c++17 -mavx -mavx2 -mpclmul -fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare  -O0 -g3 -ftrapv -fdebug-prefix-map=$(abs_top_srcdir)=.  -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -msse4.1 -msse4.2 -msse4 -msha  i

Compiler version: clang version 14.0.6

jamesob

ACK b053472 (jamesob/ackr/26695.4.glozow.bench_blockassembler_on)

Oops, previous ACK already stale. New ACK incorporates

-            m_node.mempool->addUnchecked(CTxMemPoolEntry(ptx, 1000, 0, 1, false, 4, lp));
+            m_node.mempool->addUnchecked(CTxMemPoolEntry(ptx, /*fee=*/(total_in - num_outputs * amount_per_output),
+                                                         /*time=*/0, /*entry_height=*/1,
+                                                         /*spends_coinbase=*/false, /*sigops_cost=*/4, lp));
         }
         --num_transactions;
     }

glozow · 2022-12-16T17:10:48Z

Yes apologies @jamesob, I noticed I was making the tx fee random but still setting the mempool entry fee to 1000. Thanks for such a quick review!

maflcko · 2022-12-19T09:46:50Z

src/bench/block_assemble.cpp

+    auto testing_setup{MakeNoLogFileContext<TestChain100Setup>()};
+    testing_setup->PopulateMempool(det_rand, /*num_transactions=*/1000, /*submit=*/true);
+    node::BlockAssembler::Options assembler_options;
+    assembler_options.test_block_validity = false;


Is there any value in disabling it? Sure, the benchmark will be less "micro", but at the same time it would be missing if a "package" block is slowing down CNB via TBV. Or is taking TBV a lot longer than the remainder of CNB, which would make this benchmark ineffective?

One reason is TBV taking time, another is TBV would fail because PopulateMempool() doesn't sign the txns and spends immature coinbases. I can change it to generate keys and mine 100 blocks in between creating and submitting the transactions if people want.

I think I'd prefer keeping TBV disabled here. Block validation is important to benchmark too, but since this scales linearly with the amount of transactions selected - I think the current AssembleBlock benchmark suffices there? Narrower benchmarks help more easily pinpoint where performance issues are.

kevkevinpal · 2022-12-21T02:48:50Z

I saw in 5021832 you change
LOCK2(m_node.mempool->cs, cs_main); -> LOCK2(cs_main, m_node.mempool->cs);
I'm just curious as to the reason

glozow · 2022-12-21T10:02:01Z

I saw in 5021832 you change LOCK2(m_node.mempool->cs, cs_main); -> LOCK2(cs_main, m_node.mempool->cs); I'm just curious as to the reason

It's an inconsistent order of acquiring mutexes, which along with other things can cause deadlock. I missed it when first implementing PopulateMempool() - until this PR, it was only called with both locks already acquired. Compiling with --enable-debug / -DDEBUG_LOCKORDER found it.

kevkevinpal · 2022-12-21T14:04:01Z

I saw in 5021832 you change LOCK2(m_node.mempool->cs, cs_main); -> LOCK2(cs_main, m_node.mempool->cs); I'm just curious as to the reason

It's an inconsistent order of acquiring mutexes, which along with other things can cause deadlock. I missed it when first implementing PopulateMempool() - until this PR, it was only called with both locks already acquired. Compiling with --enable-debug / -DDEBUG_LOCKORDER found it.

Ok I'll try compiling with --enable-debug thanks that helps explain

Emzy · 2022-12-21T18:21:51Z

Tested ACK b053472

this is what I get when running the BlockAssemblerAddPackageTxns benchmark

Specs:
mac book pro 2019
2,6 GHz 6-Core Intel Core i7
16 GB 2667 MHz DDR4
macos 13.0.1 (22A400)

$ ./src/bench/bench_bitcoin -filter=BlockAssemblerAddPackageTxns --sanity-check
Running with --sanity-check option, benchmark results will be useless.

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|      777,853,228.00 |                1.29 |    0.0% |      0.78 | `BlockAssemblerAddPackageTxns`

Without --sanity-check:

$ ./src/bench/bench_bitcoin -filter=BlockAssemblerAddPackageTxns 

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|      787,270,151.00 |                1.27 |    0.8% |      8.94 | `BlockAssemblerAddPackageTxns`

Specs:
Thin Client
2,6 GHz 2-Core AMD G-T56N Processor
4 GB
Ubuntu 20.04.5 LTS

$ ./src/bench/bench_bitcoin -filter=BlockAssemblerAddPackageTxns --sanity-check
Running with --sanity-check option, benchmark results will be useless.
Warning, results might be unstable:
* CPU frequency scaling enabled: CPU 0 between 825.0 and 1,650.0 MHz
* CPU governor is 'ondemand' but should be 'performance'
* Turbo is enabled, CPU frequency will fluctuate

Recommendations
* Use 'pyperf system tune' before benchmarking. See https://github.com/psf/pyperf

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|    3,488,170,147.00 |                0.29 |    0.0% |      3.49 | `BlockAssemblerAddPackageTxns`

Without --sanity-check:

$ ./src/bench/bench_bitcoin -filter=BlockAssemblerAddPackageTxns
Warning, results might be unstable:
* CPU frequency scaling enabled: CPU 0 between 825.0 and 1,650.0 MHz
* CPU governor is 'ondemand' but should be 'performance'
* Turbo is enabled, CPU frequency will fluctuate

Recommendations
* Use 'pyperf system tune' before benchmarking. See https://github.com/psf/pyperf

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|    3,477,761,468.00 |                0.29 |    0.0% |     38.30 | `BlockAssemblerAddPackageTxns`

hernanmarino

ACK b053472 . Tested / bench'ed correctly

kevkevinpal · 2022-12-21T18:33:06Z

Tested ACK b053472

left some test results here
#26695 (comment)

src/node/miner.cpp

pablomartin4btc

Tested ACK.
I've executed the bench successfully; compiled with --enable-debug.

This allows us to both manually manipulate options and grab values from ArgsManager (i.e. -blockmaxweight and -blockmintxfee config options) when constructing BlockAssembler::Options. Prior to this change, the only way to apply the config options is by ctoring BlockAssembler with no options, which calls DefaultOptions().

Allows us to test BlockAssembler on transactions without signatures or mature coinbases (which is what PopulateMempool creates). Also means that `TestBlockValidity()` is not included in the bench timing.

This makes the contents of the mempool more realistic and iterating by ancestor feerate order more meaningful. If transactions have varying feerates, it's also more likely that packages will need to be updated during block template assembly.

The current BlockAssembler bench only tests on a mempool where all transactions have 0 ancestors or descendants, which does not exercise any of the package-handling logic in BlockAssembler

stickies-v

ACK 0452805

I think this comment can be resolved in a follow-up if preferable but I'd prefer the gArgs parameter be renamed in this PR already since it's quite confusing.

stickies-v · 2022-12-21T16:06:56Z

src/node/miner.cpp

@@ -72,18 +74,22 @@ BlockAssembler::BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool
    nBlockMaxWeight = std::max<size_t>(4000, std::min<size_t>(MAX_BLOCK_WEIGHT - 4000, options.nBlockMaxWeight));
 }

-static BlockAssembler::Options DefaultOptions()
+void ApplyArgsManOptions(const ArgsManager& gArgs, BlockAssembler::Options& options)


Suggested change

void ApplyArgsManOptions(const ArgsManager& gArgs, BlockAssembler::Options& options)

void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& options)

stickies-v · 2022-12-22T14:45:34Z

src/bench/block_assemble.cpp

+    auto testing_setup{MakeNoLogFileContext<TestChain100Setup>()};
+    testing_setup->PopulateMempool(det_rand, /*num_transactions=*/1000, /*submit=*/true);
+    node::BlockAssembler::Options assembler_options;
+    assembler_options.test_block_validity = false;


I think I'd prefer keeping TBV disabled here. Block validation is important to benchmark too, but since this scales linearly with the amount of transactions selected - I think the current AssembleBlock benchmark suffices there? Narrower benchmarks help more easily pinpoint where performance issues are.

stickies-v · 2022-12-22T15:20:06Z

src/bench/block_assemble.cpp

    });
 }

 BENCHMARK(AssembleBlock, benchmark::PriorityLevel::HIGH);
+BENCHMARK(BlockAssemblerAddPackageTxns, benchmark::PriorityLevel::HIGH);


Yeah sorry, we had medium in the initial PR but it was removed later on, so agreed with changing it to low. We haven't defined any guidelines on when to downgrade to low afaik, but since devs run make check often, I think adding benches with meaningful (e.g. >1s?) --sanity-check overhead should run in low priority - unless their complexity/importance warrants otherwise.

kevkevinpal · 2022-12-23T00:08:32Z

Tested ACK 0452805
Compiled this time using --enable-debug

Specs:

mac book pro 2019
2.3 GHz 8-Core Intel Core i9
16 GB 2667 MHz DDR4
AMD Radeon Pro 5500M 4 GB

➜ bitcoin git:(PR26695) ✗ ./src/bench/bench_bitcoin -filter=BlockAssemblerAddPackageTxns --sanity-check

Running with --sanity-check option, benchmark results will be useless.
Warning, results might be unstable:
* DEBUG defined

Recommendations
* Make sure you compile for Release

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|   45,742,309,331.00 |                0.02 |    0.0% |     45.74 | `BlockAssemblerAddPackageTxns`

➜ bitcoin git:(PR26695) ✗ ./src/bench/bench_bitcoin -filter=BlockAssemblerAddPackageTxns

Warning, results might be unstable:
* DEBUG defined

Recommendations
* Make sure you compile for Release

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|   43,927,650,019.00 |                0.02 |    0.7% |    488.03 | `BlockAssemblerAddPackageTxns`

achow101 · 2023-01-11T23:02:39Z

ACK 0452805

stickies-v · 2023-01-12T13:38:45Z

I think this comment can be resolved in a follow-up if preferable but I'd prefer the gArgs parameter be renamed in this PR already since it's quite confusing.

Follow-ups in #26883

6a5e88e miner: don't re-apply default Options value if argument is unset (stickies-v) ea72c3d refactor: avoid duplicating BlockAssembler::Options members (stickies-v) cba749a refactor: rename local gArgs to args (stickies-v) Pull request description: Two follow-ups for #26695, both refactoring and no observed (*) behaviour change: - Rename `gArgs` to `args` because it's not actually a global - Add `BlockAssembler::Options` as a (private) member to `BlockAssembler` to avoid having to assign all the options individually, essentially duplicating them Reduces LoC and makes the code more readable, in my opinion. --- (*) as [pointed out by ajtowns](#26883 (comment)), this PR changes the interface of `ApplyArgsManOptions()`, making this not a pure refactoring PR. In practice, `ApplyArgsManOptions()` is never called in such a way that this leads to observed behaviour change. Regardless, I've carved out the potential behaviour change into a separate commit and would be okay with dropping it, should it turn out to be controversial. ACKs for top commit: glozow: ACK 6a5e88e TheCharlatan: Light code review ACK 6a5e88e Tree-SHA512: 15c30442ff0e070b1a58dc4c9615550d619ce35b4a2596b2c0a9d790259bbf987cab708f7cbb1057a8cf8b4c3226f3ad981282d3499ac442094806492a5f68ce

…26695

glozow added the Tests label Dec 13, 2022

jamesob reviewed Dec 13, 2022

View reviewed changes

src/test/util/mining.h Outdated Show resolved Hide resolved

This was referenced Dec 14, 2022

p2p: remove adjusted time #25908

Closed

test: Extend stale_tip_peer_management test #23352

Closed

fuzz: BIP 30, CVE-2018-17144 #17860

Merged

maflcko removed the Tests label Dec 14, 2022

DrahtBot added the Tests label Dec 14, 2022

glozow force-pushed the 2022-12-bench-miner branch from e2ef70c to 4c20bee Compare December 14, 2022 10:14

jamesob approved these changes Dec 14, 2022

View reviewed changes

maflcko approved these changes Dec 14, 2022

View reviewed changes

src/test/util/mining.cpp Outdated Show resolved Hide resolved

src/test/util/mining.cpp Outdated Show resolved Hide resolved

src/bench/block_assemble.cpp Outdated Show resolved Hide resolved

src/bench/block_assemble.cpp Outdated Show resolved Hide resolved

stickies-v reviewed Dec 15, 2022

View reviewed changes

glozow force-pushed the 2022-12-bench-miner branch 3 times, most recently from 628ce92 to b053472 Compare December 16, 2022 16:54

jamesob approved these changes Dec 16, 2022

View reviewed changes

maflcko approved these changes Dec 19, 2022

View reviewed changes

hernanmarino reviewed Dec 21, 2022

View reviewed changes

pablomartin4btc reviewed Dec 21, 2022

View reviewed changes

src/node/miner.cpp Outdated Show resolved Hide resolved

pablomartin4btc reviewed Dec 21, 2022

View reviewed changes

glozow added 3 commits December 22, 2022 11:33

[refactor] parameterize BlockAssembler::Options in PrepareBlock

c058852

[miner] allow bypassing TestBlockValidity

cba5934

Allows us to test BlockAssembler on transactions without signatures or mature coinbases (which is what PopulateMempool creates). Also means that `TestBlockValidity()` is not included in the bench timing.

glozow added 3 commits December 22, 2022 11:33

[test util] randomize fee in PopulateMempool

8791410

This makes the contents of the mempool more realistic and iterating by ancestor feerate order more meaningful. If transactions have varying feerates, it's also more likely that packages will need to be updated during block template assembly.

[test util] lock cs_main before pool.cs in PopulateMempool

6ce265a

[bench] BlockAssembler with mempool packages

0452805

The current BlockAssembler bench only tests on a mempool where all transactions have 0 ancestors or descendants, which does not exercise any of the package-handling logic in BlockAssembler

glozow force-pushed the 2022-12-bench-miner branch from b053472 to 0452805 Compare December 22, 2022 11:34

stickies-v approved these changes Dec 22, 2022

View reviewed changes

achow101 merged commit 2f6a8e5 into bitcoin:master Jan 11, 2023

glozow deleted the 2022-12-bench-miner branch January 12, 2023 11:06

stickies-v mentioned this pull request Jan 12, 2023

src/node/miner cleanups, follow-ups for #26695 #26883

Merged

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 12, 2023

Merge bitcoin#26695: bench: BlockAssembler on a mempool with packages

F178

b974575

DrahtBot mentioned this pull request Feb 2, 2023

mempool / miner: regularly flush <=0-fee entries and mine everything in the mempool #27018

Closed

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 25, 2023

Merge bitcoin#26883: src/node/miner cleanups, follow-ups for bitcoin#…

40eec91

…26695

bitcoin locked and limited conversation to collaborators Jan 12, 2024

	void ApplyArgsManOptions(const ArgsManager& gArgs, BlockAssembler::Options& options)
	void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& options)

bench: BlockAssembler on a mempool with packages #26695

bench: BlockAssembler on a mempool with packages #26695

Uh oh!

Conversation

Uh oh!

Uh oh!

Reviews

Conflicts

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!