Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Compile time of configure gadget #1254

Conversation

naure
Copy link
Contributor

@naure naure commented Feb 24, 2023

Description

This improves compilation times.

Issue Link

No issue, quality-of-life PR.

Type of change

  • [ x ] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Contents

The function configure_gadget is split into a generic and a non-generic functions.

Rationale

Generics are not free. They are the biggest contributor to compile times and to CPU code cache. configure_gadget is the largest example of this phenomenon.

I measured about 2x fewer LLVM IR with cargo llvm-lines. Even 3x in Scroll’s fork. The improvement in clock time depends a lot on exactly what you do, but it will be faster.

How Has This Been Tested?

cargo test --features test -p zkevm-circuits --release

@github-actions github-actions bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Feb 24, 2023
@hero78119 hero78119 self-requested a review February 27, 2023 03:14
@hero78119
Copy link
Member

Hi @naure The changes looks neat to me!!
Just wanna further ask, is there more insightful elaboration/reference on why this craft trick can improve compiling time instead of compiler optimize itself?
Want to learn more and see whether can also optimise other parts in current codebase

@naure
Copy link
Contributor Author

naure commented Feb 27, 2023

The Rust Performance Book!

@naure
Copy link
Contributor Author

naure commented Feb 27, 2023

The next largest occurence of this would be create_gate. That one should be fixed naturally along with this improvement: privacy-scaling-explorations/halo2#144

Copy link
Member

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

LGTM! This is really nice!

The github action for this PR took 6m 28s to build the tests (see https://github.com/privacy-scaling-explorations/zkevm-circuits/actions/runs/4266506184/jobs/7427108345)
Previoulsy this was 7m 13s (see https://github.com/privacy-scaling-explorations/zkevm-circuits/actions/runs/4303670677/jobs/7503692807)

So it's a reduction of more than 1 minute :D

@hero78119 hero78119 added this pull request to the merge queue Mar 2, 2023
Merged via the queue into privacy-scaling-explorations:main with commit 30a2b10 Mar 2, 2023
cameron1024 pushed a commit to cameron1024/zkevm-circuits that referenced this pull request Jun 30, 2024
* fix poseidon-base default short

* fix clippy
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crate-zkevm-circuits Issues related to the zkevm-circuits workspace member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants