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

chore!: change ec_add to unsafe implementation (but much better perf) #8723

Closed
wants to merge 1 commit into from

Conversation

ludamad
Copy link
Collaborator

@ludamad ludamad commented Sep 24, 2024

Reverts the revert #8722

original description:
Use unconditional_add for ec_add ACIR opcode.
This improves a lot the performance of the opcode, but also makes it unsafe.
I keep the PR as draft until aztec packages are sync with Noir PR noir-lang/noir#5858 which adds the checks in the stdlib function.
The unsafe version can then be used when we know the inputs are valid (for instance if they come from a previous add).
n.b.: the real performance boost will happen when we will be able to use the unsafe version.

@ludamad ludamad added e2e-all CI: Deprecated, use ci-full. Enable all master checks. bench-all CI: Enables this CI job. labels Sep 24, 2024
Copy link
Contributor

Changes to circuit sizes

Generated at commit: 4830e1d0452df21f3018864b24d7aae56a38d55d, compared to commit: 80acfd943ac3cd42b548043824f530018ac07a2d

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
private_kernel_reset_tiny 0 ➖ 0.00% -4 ✅ -0.01%
private_kernel_reset_small 0 ➖ 0.00% -8 ✅ -0.01%
private_kernel_reset_medium 0 ➖ 0.00% -16 ✅ -0.01%
private_kernel_reset_big 0 ➖ 0.00% -32 ✅ -0.01%
private_kernel_reset 0 ➖ 0.00% -64 ✅ -0.01%
private_kernel_reset_full_inner 0 ➖ 0.00% -64 ✅ -0.01%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
private_kernel_reset_tiny 35,560 (0) 0.00% 76,374 (-4) -0.01%
private_kernel_reset_small 39,387 (0) 0.00% 102,743 (-8) -0.01%
private_kernel_reset_medium 47,044 (0) 0.00% 155,660 (-16) -0.01%
private_kernel_reset_big 62,353 (0) 0.00% 261,697 (-32) -0.01%
private_kernel_reset 92,596 (0) 0.00% 472,771 (-64) -0.01%
private_kernel_reset_full_inner 85,690 (0) 0.00% 443,083 (-64) -0.01%

@ludamad ludamad closed this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bench-all CI: Enables this CI job. e2e-all CI: Deprecated, use ci-full. Enable all master checks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants