Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add benchmarks for Scilla and ERC-20 transfers #2168

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JamesHinshelwood
Copy link
Contributor

Previously we would generate pre-emptively generate 3.2 million transactions, just in case we ran enough iterations to need them. Instead, we use Criterion's iter_batched method to generate the transactions as they are needed. This ensures we don't generate more transactions than we need and that we don't consume loads of memory. The two downsides of this approach are:

  • We don't generate transactions in parallel any more, but this doesn't seem to make much difference to the total benchmarking time now that we aren't over-generating transactions.
  • The generated flamegraphs now include the transaction generation calls, but drilling down and ignoring them is only one keypress.

I've also renamed the benchmark from produce-full to full-blocks-evm-transfers since I think it better represents what we are testing.

Copy link
Contributor

github-actions bot commented Jan 17, 2025

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will investigate benchmark backfilling and resolve this comment when done. Leaving here to avoid merging :)

86667
86667 previously approved these changes Jan 20, 2025
@JamesHinshelwood JamesHinshelwood force-pushed the improve-bench branch 2 times, most recently from d0837a8 to 3869480 Compare January 20, 2025 17:40
@JamesHinshelwood JamesHinshelwood changed the title Improve existing full block benchmark Add benchmarks for Scilla and ERC-20 transfers Jan 20, 2025
Previously we would generate pre-emptively generate 3.2 million
transactions, just in case we ran enough iterations to need them.
Instead, we use Criterion's `iter_batched` method to generate the
transactions as they are needed. This ensures we don't generate
more transactions than we need and that we don't consume loads of
memory. The two downsides of this approach are:

* We don't generate transactions in parallel any more, but this
doesn't seem to make much difference to the total benchmarking
time now that we aren't over-generating transactions.
* The generated flamegraphs now include the transaction generation
calls, but drilling down and ignoring them is only one keypress.

I've also renamed the benchmark from `produce-full` to
`full-blocks-evm-transfers` since I think it better represents
what we are testing.
@JamesHinshelwood JamesHinshelwood force-pushed the improve-bench branch 3 times, most recently from d368d5b to ceb3c36 Compare January 20, 2025 20:49
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