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

Refactoring dyn-abi to performance parity with ethabi #144

Merged
merged 14 commits into from
Jun 26, 2023

Conversation

prestwich
Copy link
Member

@prestwich prestwich commented Jun 24, 2023

This PR brings dyn-abi to performance parity with ethabi (or better) on all benchmarks

It does this by

  • removing unnecessary type checks during value encoding
  • removing buffer copies during decoding into tokens
  • bypassing tokenization during value encoding

Followup work may bypass decoding into tokens and decode directly into values

Motivation

dyn-abi is slow and should be fast 😤

Solution

make it faster

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@prestwich prestwich added the enhancement New feature or request label Jun 24, 2023
@prestwich prestwich self-assigned this Jun 24, 2023
@prestwich prestwich marked this pull request as ready for review June 25, 2023 19:28
@prestwich prestwich changed the title [WIP] Refactoring dyn-abi Refactoring dyn-abi to performance parity with ethabi Jun 25, 2023
@prestwich prestwich requested a review from DaniPopes June 25, 2023 19:29
let input = encode_struct_sol_values();
let input = DynSolValue::Tuple(input.to_vec());
b.iter(|| ty.encode_single(black_box(&input).clone()));
Copy link
Member Author

Choose a reason for hiding this comment

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

this was doing an extra alloc in the encode_single

@DaniPopes
Copy link
Member

Can you share the benchmark results (with % gains)?

@prestwich
Copy link
Member Author

prestwich commented Jun 26, 2023

Here's the comparison to main. To generate this I ran git switch main && cargo clean && cargo criterion && git switch prestwich/new-dyn && cargo criterion

A significant % of gains were from making the benches apples-to-apples. The versions in main actually cause the dyn-abi to do extra work that ethabi was not required to. I'd guess that about 100ns of the improvement is from making the bench equivalent

Screenshot 2023-06-26 at 8 59 37 AM

@prestwich prestwich merged commit 8c6a5bb into main Jun 26, 2023
@DaniPopes DaniPopes deleted the prestwich/new-dyn branch June 26, 2023 17:31
@prestwich prestwich mentioned this pull request Jun 26, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants