Skip to content

perf(txpool): keep popped transactions in the best transactions snapshot - #35

Closed
haardikk21 wants to merge 1 commit into
perf/pending-best-shared-independentfrom
perf/best-keep-popped-in-snapshot
Closed

haardikk21 wants to merge 1 commit into
perf/pending-best-shared-independentfrom
perf/best-keep-popped-in-snapshot

Conversation

@haardikk21

Copy link
Copy Markdown

Stacked on #31; the diff is the last commit only.

Summary

BestTransactions::pop_best removed each yielded transaction from all. all is an imbl::OrdMap cloned from the pending pool, so its nodes are shared with the pool. The first write to a shared node copies that node, including a clone of every PendingTransaction (and its Arc) in it. Transaction ids from different senders are spread over the whole map, so almost every pop copied a fresh leaf.

Popped transactions now stay in all. A popped: FxHashSet<TransactionId> records them instead:

  • pop_best skips keys whose id is in popped, then marks the id it yields.
  • ancestor ignores popped ids, so a live transaction whose ancestor was already yielded still becomes independent, as before.
  • Inserting a live transaction removes its id from popped, so a replacement of an already yielded transaction is yielded again, as before.
  • size_hint subtracts popped.len(). popped only holds ids in all, so the bound is exact, as before.

Yield order and outcomes are unchanged. The new test test_best_live_transactions_after_their_ancestor_or_original_was_popped covers both live-update cases. On the parent commit its yield assertions pass too; only its new check that popped transactions remain in all fails there.

Benchmarks

benches/best_transactions.rs (added in #31), ABBA on a Linux devbox (AMD Ryzen AI MAX+ 395, 32 threads, Linux 7.2, rustc 1.96.0). Each arm was built once and the binaries ran in the order A, B, B, A, pinned to cores 4–15, with 2s warm-up and 5s measurement. A = #31 (e69a3ac09), B = this change. Times are means of the A runs and of the B runs, in µs.

Scenario A B Δ A runs B runs
take_512, 1k senders 71.1 64.7 −8.9% 70.9 / 71.2 64.1 / 65.4
take_512, 10k senders 100.4 85.7 −14.7% 100.4 / 100.4 85.8 / 85.6
take_512, 40k senders 125.6 106.7 −15.1% 125.5 / 125.7 105.4 / 108.0
take_512, 10k senders × 4 130.5 114.3 −12.4% 130.7 / 130.3 114.0 / 114.7
drain, 1k senders 129.7 123.5 −4.8% 130.2 / 129.2 121.4 / 125.6
drain, 10k senders 1,824.2 1,777.9 −2.5% 1,822.1 / 1,826.4 1,764.2 / 1,791.6
drain, 40k senders 9,051.6 9,130.1 +0.9% 9,082.6 / 9,020.7 9,113.5 / 9,146.7
drain, 10k senders × 4 9,470.2 9,230.0 −2.5% 9,467.1 / 9,473.4 9,225.2 / 9,234.8
snapshot, 40k senders 0.070 0.072 +2.1% 0.070 / 0.070 0.071 / 0.072
add_128_with_live_snapshot, 40k senders 135.4 136.3 +0.7% 137.7 / 133.0 134.1 / 138.5

take_512, which matches how payload builders use the iterator (take a prefix, then drop it), saves about 30–40 ns per yielded transaction. A full drain eventually copies most of the shared map whichever way it is mutated, so it gains less.

Pool writes while a snapshot is live (add_128_with_live_snapshot) are unchanged.

cargo bench -p reth-transaction-pool --features test-utils --bench best_transactions

Validation

  • cargo test -p reth-transaction-pool --all-features: 280 unit, 15 integration and 3 doc tests pass.
  • cargo clippy -p reth-transaction-pool --all-features --all-targets -- -D warnings
  • cargo +nightly fmt -p reth-transaction-pool -- --check

@haardikk21

Copy link
Copy Markdown
Author

CI failures here are not from this diff and match #31 / the releases/v2.7.0 base:

reth-transaction-pool tests, clippy (-D warnings, all targets/features) and fmt pass locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants