Skip to content

perf(txpool): poll best transactions' update channel only after the pool sends - #36

Closed
haardikk21 wants to merge 1 commit into
perf/best-keep-popped-in-snapshotfrom
perf/best-poll-new-transactions-on-count
Closed

haardikk21 wants to merge 1 commit into
perf/best-keep-popped-in-snapshotfrom
perf/best-poll-new-transactions-on-count

Conversation

@haardikk21

Copy link
Copy Markdown

Stacked on #35 (which is stacked on #31); the diff is the last commit only.

Summary

BestTransactions::next calls add_new_transactions before every yield, which calls try_recv on the pending pool's broadcast channel. For tokio's broadcast channel, try_recv on an empty channel takes a read lock on the next slot and then the channel's tail mutex. Senders take the same mutex. The channel is empty for nearly every call, so each yield paid for both locks, and the iterator contended with the pool's insert path on that mutex.

PendingPool now keeps a shared new_transaction_count: Arc<AtomicU64>, incremented (Release) after each send. Each BestTransactions from PendingPool::best holds the counter and the value it had when the receiver was last found empty. add_new_transactions loads the counter (Acquire) and only polls the receiver when the counter has moved.

  • The counter is incremented after the send. An iterator that sees the new count is guaranteed to find the transaction in the channel.
  • The iterator updates its received count only when try_recv reports nothing left. A burst larger than MAX_NEW_TRANSACTIONS_PER_BATCH still drains over the following calls, as before. test_best_receives_more_live_transactions_than_one_batch covers this.
  • A send racing with a yield is picked up on a later call, as it already was.
  • no_updates drops the counter along with the receiver.
  • Iterators without a counter poll before every yield, as before. That includes the tests that replace the receiver with their own channel.

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 = #35 (c156e4be3), 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 65.7 51.6 −21.5% 66.0 / 65.4 52.3 / 50.9
take_512, 10k senders 87.2 73.0 −16.4% 88.8 / 85.7 72.9 / 73.0
take_512, 40k senders 105.4 90.4 −14.3% 105.3 / 105.5 90.0 / 90.7
take_512, 10k senders × 4 113.6 100.5 −11.6% 113.6 / 113.6 100.7 / 100.2
drain, 1k senders 125.0 98.4 −21.3% 126.0 / 124.1 98.9 / 97.9
drain, 10k senders 1,778.1 1,506.2 −15.3% 1,802.3 / 1,753.8 1,504.3 / 1,508.1
drain, 40k senders 9,190.3 8,046.6 −12.4% 9,213.6 / 9,166.9 8,064.0 / 8,029.1
drain, 10k senders × 4 9,226.1 8,195.7 −11.2% 9,223.1 / 9,229.1 8,171.9 / 8,219.6
snapshot, 40k senders 0.072 0.079 +10.5% 0.072 / 0.071 0.079 / 0.079
add_128_with_live_snapshot, 40k senders 135.4 134.4 −0.7% 136.1 / 134.7 132.3 / 136.6

Each yield saves about 25–30 ns. Taking a snapshot costs about 8 ns more, for the counter's Arc clone and load, once per iterator. These runs are single-threaded, so they don't show the lock contention between the iterator and concurrent pool inserts that this also removes.

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

Validation

  • cargo test -p reth-transaction-pool --all-features: 281 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

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