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

Devmode to Reduce compilation time (including G2 and ECNTT) #395

Merged
merged 57 commits into from
Apr 9, 2024

Conversation

vhnatyk
Copy link
Contributor

@vhnatyk vhnatyk commented Feb 22, 2024

Describe the changes

Introduces 'develop' mode - with reduced -O0 optimizations instead of -O3 and disabled inlining and loop unrolling
with slightly lower performance but much reduced build time (6 minutes vs reported 163 minutes on i9 13900K? for all curves)

Linked Issues

Resolves #316

@DmytroTym
Copy link
Contributor

If we just disable the mixed-radix EC NTT and only use radix-2, would that give a further significant speedup?

@vhnatyk
Copy link
Contributor Author

vhnatyk commented Feb 22, 2024

umm - like still ~80% compilation time it's msm.cu g2 - also weird C++ gtests fail but rust all passing, and didn't expect that unroll commenting to improve times - hoped #pragma unroll 1 was supposed to but it even increases the time by 10%. Maybe enable g2 and new ecntt only on final merge to main

@yshekel
Copy link
Collaborator

yshekel commented Feb 25, 2024

I don't think mixed-radix NTT is compiled for ECNTT here since it's not instantiated.
The following commit is instantiating it:
e1ff687

@vhnatyk
Copy link
Contributor Author

vhnatyk commented Feb 25, 2024

oh right - I forgot to also push merged e1ff687 I tested locally

@jeremyfelder jeremyfelder mentioned this pull request Mar 12, 2024
3 tasks
@vhnatyk vhnatyk requested a review from LeonHibnik April 8, 2024 16:07
icicle/common.cuh Outdated Show resolved Hide resolved
Copy link
Collaborator

@yshekel yshekel left a comment

Choose a reason for hiding this comment

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

I think 163 minutes was when building ECNTT for mix-radix algorithm so I am not sure this is the case.
Can you locally build it with and without devmode for a specific curve and tell the difference.

Copy link
Collaborator

@yshekel yshekel left a comment

Choose a reason for hiding this comment

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

Approving based on the answers. Note that I wrote a minor comment about the commented mixed-radix ECTT. You can ignore it if you want.

Copy link
Contributor

@LeonHibnik LeonHibnik left a comment

Choose a reason for hiding this comment

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

TODO add devmode to golang

@vhnatyk vhnatyk merged commit 4c9b3c0 into ingonyama-zk:main Apr 9, 2024
21 checks passed
@vhnatyk vhnatyk deleted the feat/vhnat/reduce-build-time branch April 9, 2024 04:09
LeonHibnik pushed a commit that referenced this pull request Apr 9, 2024
yshekel pushed a commit that referenced this pull request May 19, 2024
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.

Rust compilation time
5 participants