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

edwards: optimize point negation #413

Merged
merged 4 commits into from Jun 26, 2023
Merged

Conversation

jsign
Copy link
Contributor

@jsign jsign commented Jun 22, 2023

This PR adds benchmarks for Neg(...) of points in Edwards variants, and an optimization to avoid redundant assignment in point coordinates.

If we run the new benchmarks available in the second commit of this branch against the same ones after the optimization here's what we get:

name               old time/op  new time/op  delta
pkg:github.com/consensys/gnark-crypto/ecc/bls12-377/twistededwards goos:linux goarch:amd64
Neg/Affine-16      4.60ns ± 2%  2.23ns ± 2%  -51.59%  (p=0.008 n=5+5)
Neg/Projective-16  5.28ns ± 3%  2.24ns ± 1%  -57.60%  (p=0.008 n=5+5)
Neg/Extended-16    7.73ns ± 1%  5.02ns ± 6%  -35.04%  (p=0.008 n=5+5)
pkg:github.com/consensys/gnark-crypto/ecc/bls12-378/twistededwards goos:linux goarch:amd64
Neg/Affine-16      4.77ns ± 5%  2.23ns ± 2%  -53.33%  (p=0.008 n=5+5)
Neg/Projective-16  5.29ns ± 4%  2.25ns ± 2%  -57.55%  (p=0.008 n=5+5)
Neg/Extended-16    7.68ns ± 2%  4.88ns ± 1%  -36.51%  (p=0.008 n=5+5)
pkg:github.com/consensys/gnark-crypto/ecc/bls12-381/bandersnatch goos:linux goarch:amd64
Neg/Affine-16      4.58ns ± 1%  2.22ns ± 1%  -51.60%  (p=0.008 n=5+5)
Neg/Projective-16  5.19ns ± 2%  2.30ns ± 4%  -55.64%  (p=0.016 n=4+5)
Neg/Extended-16    8.02ns ± 7%  5.01ns ± 2%  -37.48%  (p=0.016 n=5+4)
pkg:github.com/consensys/gnark-crypto/ecc/bls12-381/twistededwards goos:linux goarch:amd64
Neg/Affine-16      4.92ns ±13%  2.24ns ± 1%  -54.54%  (p=0.008 n=5+5)
Neg/Projective-16  5.61ns ± 9%  2.25ns ± 2%  -59.90%  (p=0.008 n=5+5)
Neg/Extended-16    8.04ns ± 5%  5.08ns ± 2%  -36.78%  (p=0.008 n=5+5)
pkg:github.com/consensys/gnark-crypto/ecc/bls24-315/twistededwards goos:linux goarch:amd64
Neg/Affine-16      4.90ns ± 9%  2.25ns ± 1%  -54.10%  (p=0.008 n=5+5)
Neg/Projective-16  5.22ns ± 2%  2.27ns ± 2%  -56.61%  (p=0.008 n=5+5)
Neg/Extended-16    7.80ns ± 5%  4.90ns ± 3%  -37.12%  (p=0.008 n=5+5)
pkg:github.com/consensys/gnark-crypto/ecc/bls24-317/twistededwards goos:linux goarch:amd64
Neg/Affine-16      4.71ns ± 3%  2.29ns ± 4%  -51.37%  (p=0.008 n=5+5)
Neg/Projective-16  5.32ns ± 3%  2.27ns ± 2%  -57.32%  (p=0.008 n=5+5)
Neg/Extended-16    7.75ns ± 4%  5.01ns ± 1%  -35.39%  (p=0.008 n=5+5)
pkg:github.com/consensys/gnark-crypto/ecc/bn254/twistededwards goos:linux goarch:amd64
Neg/Affine-16      4.61ns ± 1%  2.30ns ± 9%  -50.13%  (p=0.008 n=5+5)
Neg/Projective-16  5.22ns ± 1%  2.25ns ± 2%  -56.86%  (p=0.016 n=4+5)
Neg/Extended-16    7.64ns ± 4%  4.87ns ± 3%  -36.27%  (p=0.008 n=5+5)
pkg:github.com/consensys/gnark-crypto/ecc/bw6-633/twistededwards goos:linux goarch:amd64
Neg/Affine-16      5.22ns ± 5%  2.56ns ± 5%  -50.99%  (p=0.008 n=5+5)
Neg/Projective-16  6.48ns ± 7%  2.59ns ± 4%  -60.04%  (p=0.008 n=5+5)
Neg/Extended-16    8.96ns ± 1%  6.60ns ± 3%  -26.32%  (p=0.008 n=5+5)
pkg:github.com/consensys/gnark-crypto/ecc/bw6-756/twistededwards goos:linux goarch:amd64
Neg/Affine-16      6.85ns ± 4%  2.73ns ± 1%  -60.17%  (p=0.008 n=5+5)
Neg/Projective-16  7.25ns ± 9%  2.77ns ± 6%  -61.82%  (p=0.008 n=5+5)
Neg/Extended-16    10.4ns ± 1%   7.2ns ± 1%  -31.14%  (p=0.008 n=5+5)
pkg:github.com/consensys/gnark-crypto/ecc/bw6-761/twistededwards goos:linux goarch:amd64
Neg/Affine-16      6.04ns ± 2%  2.82ns ± 7%  -53.36%  (p=0.016 n=4+5)
Neg/Projective-16  7.11ns ± 4%  2.86ns ± 3%  -59.79%  (p=0.008 n=5+5)
Neg/Extended-16    10.3ns ± 2%   7.4ns ± 6%  -28.31%  (p=0.008 n=5+5)

The TL;DR is quite simple: a .Set(...) call will copy coordinates that need to be negated afterward, so that's unnecessary work.

I double-checked, and this optimization looks to be already present for the base curves, so it was only missing for edwards variants.

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
…ions

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
@CLAassistant
Copy link

CLAassistant commented Jun 22, 2023

CLA assistant check
All committers have signed the CLA.

@gbotrel gbotrel changed the base branch from master to develop June 26, 2023 02:24
Copy link
Collaborator

@gbotrel gbotrel left a comment

Choose a reason for hiding this comment

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

lgtm ✅

@gbotrel gbotrel merged commit f5f856b into Consensys:develop Jun 26, 2023
6 checks passed
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.

None yet

3 participants