Took the existing benchmark and made is something more exhaustive, targeting eviction of 2500 transactions of different shapes. of dependencies.
Magic number comes from: 100 children being RBF’d, each bumping 24 parents = 2500 transactions
Turns out the benchmark ComplexMemPool already does something similar, so if we don’t want to expand it, maybe just remove it in favor of the existing better benchmark?
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process. A summary of reviews will appear here.
80- tx4.vout[0].nValue = 10 * COIN;
81- tx4.vout[1].scriptPubKey = CScript() << OP_4 << OP_EQUAL;
82- tx4.vout[1].nValue = 10 * COIN;
83+ if (put_index < txns.size()) {
84+ // We spend put_index's txn's in the next available slot for each previous transaction
85+ tx.vin[put_index].prevout = COutPoint(txns[put_index]->GetHash(), put_index + txn_index - 1);
0bench/mempool_eviction.cpp:42:105: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'unsigned long'
158+ AddTx(txns[i], (txns.size() - i)*1000LL, pool);
159+ }
160+ assert(pool.size() == txns.size());
161+ pool.TrimToSize(pool.DynamicMemoryUsage() - 1);
162+ assert(pool.size() == txns.size() - 1);
163+ pool.TrimToSize(GetVirtualTransactionSize(*txns[0]));
26-// unique transactions for a more meaningful performance measurement.
27-static void MempoolEviction(benchmark::Bench& bench)
28+void add_parents_child(std::vector<CTransactionRef>& txns, size_t package_size, size_t set_num)
29+{
30+ // N+1 N-input-N-output txns, in a parents-and-child package
31+ // Where the final tx takes an input from all ancestors
I could be reading this code wrong, but isn’t each tx taking inputs from each tx before it? It’s still parents-and-child technically but it’s also a chain.
0 // Where each tx takes an input from all prior txns
173+
174+ for (size_t i=0; i<num_packages; i++) {
175+ add_parents_child(txns, package_size, i);
176+ }
177+
178+ /* Now we're ready */
187- AddTx(tx6_r, 1100LL, pool);
188- AddTx(tx7_r, 9000LL, pool);
189- pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4);
190- pool.TrimToSize(GetVirtualTransactionSize(*tx1_r));
191+ for (size_t i=0; i<txns.size(); i++) {
192+ // Monotonically decreasing fees
151- const CTransactionRef tx4_r{MakeTransactionRef(tx4)};
152- const CTransactionRef tx5_r{MakeTransactionRef(tx5)};
153- const CTransactionRef tx6_r{MakeTransactionRef(tx6)};
154- const CTransactionRef tx7_r{MakeTransactionRef(tx7)};
155+ bench.run([&]() NO_THREAD_SAFETY_ANALYSIS {
156+ for (size_t i=0; i<txns.size(); i++) {
nit: I think we tend to prefer this
0 for (size_t i{0}; i < txns.size(); ++i) {
33+ const size_t num_txns = package_size;
34+ for (size_t txn_index=0; txn_index<num_txns; txn_index++) {
35+ CMutableTransaction tx = CMutableTransaction();
36+ tx.vin.resize(num_puts);
37+ tx.vout.resize(num_puts);
38+ for (size_t put_index=0; put_index<num_puts; put_index++) {
Can you apply the changes to the other loops too? i.e.
0 for (size_t put_index{0}; put_index < num_puts; ++put_index) {
42+ tx.vout[put_index].nValue = txn_index;
43+
44+ if (put_index < txn_index) {
45+ // We spend put_index's txn's in the next available slot for each previous transaction
46+ assert(put_index + txn_index >= 1);
47+ tx.vin[put_index].prevout = COutPoint(txns[put_index]->GetHash(), put_index + txn_index - 1);
Wait, if you’re using txns[put_index] and you’re using the same txns reference each time you call add_parents_child, aren’t you having all subsequent packages spend outputs from txns[0:24] too? That means all these packages conflict with each other.
You could fix this by adding set_num * package_size or something to the index. But what would probably make a better interface is if add_parents_child returns the list of transactions it creates, and you append them to your larger list of packages.
Hmmm, traced out a package size of 4, looks like I’m referencing non-existent outputs, not re-using them(maybe both :) ).
Clearly this is confusing and broken either way.
To take a step back, after doing this draft I realized https://github.com/bitcoin/bitcoin/blob/master/src/bench/mempool_stress.cpp exists. I’m fine if this doesn’t get merged if it doesn’t add anything new? Helped me learn about the benchmarks if nothing else. Thoughts?
ComplexMemPool is already benching eviction of packages. Fine with closing. The effort is appreciated :pray: