Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Transaction pool: Adds benchmark and improves performance #9958

Merged
merged 11 commits into from
Oct 8, 2021

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Oct 7, 2021

This pr adds a transaction pool benchmark that uses the Substrate node to measure the real throughput we are able to achieve. While this actually also depends on all the other components that are part of building blocks etc, it still gives a nice benchmark for checking the throughput of different kinds of scenarios. The basic benchmark that is implemented in the pr is sending 2000 tx from 10 accounts. As result of this benchmark, a performance optimization of the BestIterator could already be implemented. Before this pr the iterator was using the all set in a shared way. The problem of doing that is that in the background there could have been a revalidation that removed some data while we currently try to build a block. (We are removing stuff like an entire subtree to readd it at a later point, but this could already be too late for the block production).

TLDR, bare numbers:

Before:

time:   [21.832 s 23.904 s 26.770 s]                                     
thrpt:  [747.11  elem/s 836.69  elem/s 916.10  elem/s]

After:

time:   [13.188 s 13.200 s 13.212 s]
thrpt:  [1.5138 Kelem/s 1.5151 Kelem/s 1.5165 Kelem/s]

So, a throughput increase of factor 1.81

@bkchr bkchr added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Oct 7, 2021
@bkchr bkchr requested a review from tomusdrw October 7, 2021 11:14
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both the benchmarks and the change in BestIterator look good to me, but I don't really see a relationship here. AFAICT the benchmark is simply inserting transactions and not really reading/iterating over them, how is it possible that there are some differences?

Initially I wasn't convinced about cloning the entire all map, because of transaction content, but noted that the ReadyTx only contains TarnsactionsRef, which in turn is an Arc. So we are not cloning the transactions here, just a bunch of additional metadata.

@bkchr
Copy link
Member Author

bkchr commented Oct 7, 2021

AFAICT the benchmark is simply inserting transactions and not really reading/iterating over them, how is it possible that there are some differences?

We are doing revalidations in between. When we for example re-validate tx1 and have tx2-tx10 that depend on it, we remove the entire subtree of tx1 before re-inserting all of them. When that happens while we are building a block, it means that we will loose an entire subtree for a moment, before it is re-inserted. This can lead to skipping all these txs and then we did not apply any of these transactions.

@crystalin
Copy link
Contributor

Awesome @bkchr , thank you for taking care of that

@bkchr bkchr merged commit d9944b0 into master Oct 8, 2021
@bkchr bkchr deleted the bkchr-transaction-pool-benchmark branch October 8, 2021 08:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants