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.

Sign up for GitHub

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

Jump to bottom

perf: Inline some hot spots in witness generation #715

Merged
merged 2 commits into from
Oct 10, 2024
Merged

perf: Inline some hot spots in witness generation #715

merged 2 commits into from
Oct 10, 2024

Conversation

Nashtare
Copy link
Collaborator

@Nashtare Nashtare commented Oct 9, 2024 •
edited
Loading

Benchmarking CPU frequency against the Fibonacci benchmark yields an average improvement of 16% in witness generation (1.08MHz -> 1.25MHz).

@Nashtare Nashtare added this to the Performance Tuning milestone Oct 9, 2024
@Nashtare Nashtare self-assigned this Oct 9, 2024
@github-actions github-actions bot added the crate: evm_arithmetization Anything related to the evm_arithmetization crate. label Oct 9, 2024
sai-deng
sai-deng approved these changes Oct 9, 2024
View reviewed changes
Copy link
Contributor

@sai-deng sai-deng left a comment

Choose a reason for hiding this comment

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

Nice speed up!

@sai-deng
Copy link
Contributor

sai-deng commented Oct 9, 2024

By the way, if we turn on lto = true, we can get about a 3.5% performance improvement. However, I expect our tests will be slower due to the significantly longer compile times.

@Nashtare
Copy link
Collaborator Author

Nashtare commented Oct 9, 2024 •
edited
Loading

By the way, if we turn on lto = true, we can get about a 3.5% performance improvement. However, I expect our tests will be slower due to the significantly longer compile times.

Did you just run this? We had actually disabled lto after observing no noticeable difference. Was it only on witness gen or full proving?

@sai-deng
Copy link
Contributor

sai-deng commented Oct 9, 2024

By the way, if we turn on lto = true, we can get about a 3.5% performance improvement. However, I expect our tests will be slower due to the significantly longer compile times.

Did you just run this? We had actually disabled lto after observing no noticeable difference. Was it only on witness gen or full proving?

Yes. I just ran #716

@Nashtare Nashtare merged commit 08976ab into develop Oct 10, 2024
20 checks passed
@Nashtare Nashtare deleted the perf/inlining branch October 10, 2024 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@muursh muursh muursh approved these changes

@sai-deng sai-deng sai-deng approved these changes

@wborgeaud wborgeaud Awaiting requested review from wborgeaud wborgeaud is a code owner

@LindaGuiga LindaGuiga Awaiting requested review from LindaGuiga LindaGuiga is a code owner

Assignees

@Nashtare Nashtare

Labels
crate: evm_arithmetization Anything related to the evm_arithmetization crate.
Projects
Zero EVM
Status: Done
Milestone
Performance Tuning
Development

Successfully merging this pull request may close these issues.
3 participants
@Nashtare @sai-deng @muursh