Skip to content

refactor #249

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

refactor #249

wants to merge 2 commits into from

Conversation

malik672
Copy link

📝 Summary

main improvements include better pattern matching, more efficient sorting, and cleaner control flow.

💡 Motivation and Context

increased perf and refactor

Copy link

Benchmark results for 6b89303

Report: https://flashbots-rbuilder-ci-stats.s3.us-east-2.amazonaws.com/benchmark/6b89303-70579d1/report/index.html

Date (UTC) 2024-11-16T09:01:26+00:00
Commit 6b89303d64318c2b76cb7084a4c08df4906fe80f
Base SHA 70579d128cda94081f1335e3cd3499ff94a7899a

Significant changes

None

@malik672 malik672 requested a review from liamaharon as a code owner December 16, 2024 06:50
@ZanCorDX
Copy link
Contributor

Sorry for the delay.
I don't think the pattern matching changes improve the readability of the code but we can take the other 2 changes if you change the PR:
conflict_sets.sort_by_key(|set| std::cmp::Reverse(set.len()));
let mut profits_alone = HashMap::with_capacity(orders.len());

@malik672
Copy link
Author

malik672 commented Mar 2, 2025

Sorry for the delay. I don't think the pattern matching changes improve the readability of the code but we can take the other 2 changes if you change the PR: conflict_sets.sort_by_key(|set| std::cmp::Reverse(set.len())); let mut profits_alone = HashMap::with_capacity(orders.len());

The pattern matching was never because of readability, the reason is simply becasue the compiler deals with it better tbh

@SozinM
Copy link
Collaborator

SozinM commented Mar 10, 2025

Checked out of curiosity and they produce the same asm code
https://godbolt.org/z/zEc74PjrY
https://godbolt.org/z/f8hW1qav4

@malik672
Copy link
Author

Checked out of curiosity and they produce the same asm code https://godbolt.org/z/zEc74PjrY https://godbolt.org/z/f8hW1qav4

yeah not suprising, it should definitely generate the same assembly

you should read this: https://stackoverflow.com/questions/71806844/is-matching-on-patterns-faster-than-if-else-in-rust

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.

3 participants