Skip to content

edwards: optimize point negation#413

Merged
gbotrel merged 4 commits intoConsensys:developfrom
jsign:jsign/spdneg
Jun 26, 2023
Merged

edwards: optimize point negation#413
gbotrel merged 4 commits intoConsensys:developfrom
jsign:jsign/spdneg

Conversation

@jsign
Copy link
Copy Markdown
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.

jsign added 4 commits June 22, 2023 16:52
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
Copy Markdown

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
Copy Markdown
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
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.

3 participants