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

crypto, x/crypto: apply golang.org/wiki/TargetSpecific policy #31470

Open
FiloSottile opened this issue Apr 15, 2019 · 3 comments
Open

crypto, x/crypto: apply golang.org/wiki/TargetSpecific policy #31470

FiloSottile opened this issue Apr 15, 2019 · 3 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@FiloSottile
Copy link
Contributor

FiloSottile commented Apr 15, 2019

A lot of packages need to be adapted to follow the new https://golang.org/wiki/TargetSpecific policy.

In particular, point 1 would make the generic code always available for testing, fuzzing, and analysis like #31456 (comment).

@FiloSottile FiloSottile added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Apr 15, 2019
@FiloSottile FiloSottile added this to the Go1.13 milestone Apr 15, 2019
@FiloSottile FiloSottile changed the title crypto, x/crypto: apply golang.org/wiki/MultiTarget policy crypto, x/crypto: apply golang.org/wiki/TargetSpecific policy Apr 16, 2019
@yaminisridaran
Copy link

@FiloSottile I would like to help in this issue. Can you please explain me the issue.

@gopherbot
Copy link

Change https://golang.org/cl/205861 mentions this issue: curve25519: update package structure per golang.org/wiki/TargetSpecific

@gopherbot
Copy link

Change https://golang.org/cl/185439 mentions this issue: internal/chacha20: refactor for readability and consistency

gopherbot pushed a commit to golang/crypto that referenced this issue Nov 7, 2019
This was extracted from CL 205157.

Updates golang/go#31470

Change-Id: I8109b874d4c3897ffc920bb4e12c71836da9d44a
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/205861
Reviewed-by: Katie Hockman <katie@golang.org>
FiloSottile added a commit to FiloSottile/go that referenced this issue Nov 24, 2019
The new code is meant to be readable without external references for
Poly1305, and explains the field logic. The generic code is now 30-50%
faster on a Intel(R) Xeon(R) CPU E5-2690 v3 @ 2.60GHz, and even better
on a 3.1 GHz i7 MacBook.

name        old time/op    new time/op    delta
64-48          126ns ± 0%      80ns ± 1%  -36.24%  (p=0.000 n=16+20)
1K-48         1.07µs ± 0%    0.81µs ± 2%  -23.63%  (p=0.000 n=19+20)
2M-48         2.07ms ± 0%    1.61ms ± 1%  -22.31%  (p=0.000 n=20+20)
Write64-48    79.3ns ± 0%    58.0ns ± 1%  -26.89%  (p=0.000 n=20+19)
Write1K-48    1.02µs ± 0%    0.79µs ± 1%  -22.91%  (p=0.000 n=19+19)
Write2M-48    2.07ms ± 0%    1.61ms ± 2%  -22.33%  (p=0.000 n=17+20)

name        old speed      new speed      delta
64-48        508MB/s ± 0%   797MB/s ± 1%  +56.95%  (p=0.000 n=16+20)
1K-48        960MB/s ± 0%  1257MB/s ± 2%  +30.94%  (p=0.000 n=18+20)
2M-48       1.01GB/s ± 0%  1.30GB/s ± 1%  +28.73%  (p=0.000 n=20+20)
Write64-48   807MB/s ± 0%  1104MB/s ± 1%  +36.78%  (p=0.000 n=18+19)
Write1K-48  1.00GB/s ± 0%  1.30GB/s ± 1%  +29.71%  (p=0.000 n=18+19)
Write2M-48  1.01GB/s ± 0%  1.31GB/s ± 2%  +28.77%  (p=0.000 n=17+20)

The assembly is still 50-90% faster on the Xeon, 30-60% on the MacBook.
The Go code does not use all the arithmetic tricks the assembly does,
and it does not have access to the three operand wide shift instruction.

name        old time/op    new time/op    delta
64-48         80.3ns ± 1%    54.2ns ± 0%  -32.50%  (p=0.000 n=20+17)
1K-48          815ns ± 2%     446ns ± 1%  -45.27%  (p=0.000 n=20+20)
2M-48         1.61ms ± 1%    0.86ms ± 0%  -46.54%  (p=0.000 n=20+17)
Write64-48    58.0ns ± 1%    34.0ns ± 0%  -41.34%  (p=0.000 n=19+20)
Write1K-48     790ns ± 1%     427ns ± 0%  -45.92%  (p=0.000 n=19+17)
Write2M-48    1.61ms ± 2%    0.86ms ± 0%  -46.51%  (p=0.000 n=20+20)

name        old speed      new speed      delta
64-48        797MB/s ± 1%  1180MB/s ± 0%  +48.09%  (p=0.000 n=20+19)
1K-48       1.26GB/s ± 2%  2.30GB/s ± 1%  +82.71%  (p=0.000 n=20+20)
2M-48       1.30GB/s ± 1%  2.44GB/s ± 0%  +87.04%  (p=0.000 n=20+17)
Write64-48  1.10GB/s ± 1%  1.88GB/s ± 0%  +70.52%  (p=0.000 n=19+18)
Write1K-48  1.30GB/s ± 1%  2.40GB/s ± 0%  +84.84%  (p=0.000 n=19+18)
Write2M-48  1.31GB/s ± 2%  2.44GB/s ± 0%  +86.93%  (p=0.000 n=20+20)

Hopefully this will also avoid the need for an arm64 implementation.

Since now the Go and the amd64/ppc64le assembly use the same limb
schedule, drop the assembly initialize and finalize implementations,
and make the wrapper code match. It comes with a minor slowdown.

name        old time/op    new time/op    delta
64-48         50.3ns ± 0%    54.2ns ± 0%  +7.73%  (p=0.000 n=20+17)
1K-48          441ns ± 0%     446ns ± 1%  +1.10%  (p=0.000 n=19+20)
2M-48          860µs ± 0%     859µs ± 0%    ~     (p=0.178 n=19+17)
Write64-48    34.0ns ± 0%    34.0ns ± 0%    ~     (all equal)
Write1K-48     424ns ± 0%     427ns ± 0%  +0.71%  (p=0.000 n=17+17)
Write2M-48     860µs ± 0%     859µs ± 0%  -0.04%  (p=0.000 n=19+20)

name        old speed      new speed      delta
64-48       1.27GB/s ± 0%  1.18GB/s ± 0%  -7.20%  (p=0.000 n=20+19)
1K-48       2.32GB/s ± 0%  2.30GB/s ± 1%  -1.07%  (p=0.000 n=18+20)
2M-48       2.44GB/s ± 0%  2.44GB/s ± 0%    ~     (p=0.173 n=19+17)
Write64-48  1.88GB/s ± 0%  1.88GB/s ± 0%  +0.04%  (p=0.000 n=19+18)
Write1K-48  2.41GB/s ± 0%  2.40GB/s ± 0%  -0.67%  (p=0.000 n=19+18)
Write2M-48  2.44GB/s ± 0%  2.44GB/s ± 0%  +0.04%  (p=0.000 n=19+20)

Since poly1305/sum_generic.go was almost entirely rewritten, it's
probably best reviewed on gitiles.

This is the implementation published at
https://blog.filippo.io/a-literate-go-implementation-of-poly1305/

Updates golang#31470

Change-Id: I74f9011d3ee317a43b05ae7f05d96081d08bffd3
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169037
Reviewed-by: Katie Hockman <katie@golang.org>
@FiloSottile FiloSottile modified the milestones: Backlog, Go1.15 Mar 31, 2020
@katiehockman katiehockman removed their assignment Apr 20, 2020
@katiehockman katiehockman modified the milestones: Go1.15, Backlog Apr 20, 2020
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
This was extracted from CL 205157.

Updates golang/go#31470

Change-Id: I8109b874d4c3897ffc920bb4e12c71836da9d44a
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/205861
Reviewed-by: Katie Hockman <katie@golang.org>
c-expert-zigbee added a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
This was extracted from CL 205157.

Updates golang/go#31470

Change-Id: I8109b874d4c3897ffc920bb4e12c71836da9d44a
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/205861
Reviewed-by: Katie Hockman <katie@golang.org>
c-expert-zigbee added a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
This was extracted from CL 205157.

Updates golang/go#31470

Change-Id: I8109b874d4c3897ffc920bb4e12c71836da9d44a
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/205861
Reviewed-by: Katie Hockman <katie@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
This was extracted from CL 205157.

Updates golang/go#31470

Change-Id: I8109b874d4c3897ffc920bb4e12c71836da9d44a
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/205861
Reviewed-by: Katie Hockman <katie@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
This was extracted from CL 205157.

Updates golang/go#31470

Change-Id: I8109b874d4c3897ffc920bb4e12c71836da9d44a
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/205861
Reviewed-by: Katie Hockman <katie@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants