Skip to content

Commit c856fbf

Browse files
mvdanmvdan
committed
crypto/elliptic: use go:embed for the precomputed p256 table
go.dev/cl/339591 changed the code generation to use a constant string, so that the ~88KiB table can be marked read-only. The compiled code became a lot better, but unfortunately, the generated Go source became significantly more inefficient. The numbers below compare "gofmt -l" and "go tool compile" of said file, where "old" is the file as of Go 1.17, and "new" as of master in 2022/01/19: name old time/op new time/op delta Gofmt 22.8ms ± 6% 898.5ms ± 3% +3837.32% (p=0.000 n=8+8) GoToolCompile 26.9ms ± 2% 371.1ms ± 2% +1278.36% (p=0.000 n=7+8) name old user-time/op new user-time/op delta Gofmt 25.7ms ±65% 897.1ms ± 3% +3383.86% (p=0.000 n=8+8) GoToolCompile 35.1ms ±26% 367.2ms ± 3% +945.80% (p=0.000 n=8+8) name old sys-time/op new sys-time/op delta Gofmt 6.42ms ±276% 7.23ms ±38% ~ (p=0.412 n=8+6) GoToolCompile 9.20ms ±100% 13.90ms ±53% ~ (p=0.105 n=8+8) name old peak-RSS-bytes new peak-RSS-bytes delta Gofmt 9.11MB ± 7% 22.79MB ± 1% +150.23% (p=0.000 n=8+8) GoToolCompile 25.1MB ± 2% 68.6MB ± 2% +173.57% (p=0.000 n=8+8) "+" operators are binary expressions at the syntax tree level, which are represented by packages like go/ast as roughly: struct { X Expr Op Token Y Expr } Since each node is a pointer, chains of "+" operators act like linked lists. The generated code has about 14k lines, and 8 "+" operators per line, meaning that we end up with a linked list with over 11k elements. This explains the slow-down in gofmt; the printer must walk said list, and it does so more than once to work out how to format it. It seems like the compiler is similarly affected by the huge length. To remedy the effect of the linked list, use go:embed instead. This results in the same string variable with the binary table, but it greatly reduces the amount of syntax and its cost above. We still keep the generator around, but modified so it produces the binary file to be embedded rather than a large Go file. Finally, we update go/build/deps_test.go to allow crypto/elliptic to depend on embed; it's a tiny package and crypto/elliptic was already manually embedding assets via code generation. The change to deps_test.go was briefly discussed in the issue below. Fixes golang#50995. Change-Id: I0c8b432710e971a296a2e9c99147cc2cad9662aa Reviewed-on: https://go-review.googlesource.com/c/go/+/380475 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Trust: Daniel Martí <mvdan@mvdan.cc> Trust: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org>
1 parent 69e1711 commit c856fbf

File tree

5 files changed

+10
-1473
lines changed

5 files changed

+10
-1473
lines changed

‎src/crypto/elliptic/gen_p256_table.go‎

Lines changed: 5 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -7,30 +7,13 @@
77
package main
88

99
import (
10-
"bytes"
1110
"crypto/elliptic"
1211
"encoding/binary"
13-
"fmt"
14-
"go/format"
1512
"log"
1613
"os"
1714
)
1815

1916
func main() {
20-
buf := new(bytes.Buffer)
21-
fmt.Fprint(buf, `
22-
// Copyright 2021 The Go Authors. All rights reserved.
23-
// Use of this source code is governed by a BSD-style
24-
// license that can be found in the LICENSE file.
25-
26-
// Generated by gen_p256_table.go. DO NOT EDIT.
27-
28-
//go:build amd64 || arm64
29-
30-
package elliptic
31-
32-
`[1:])
33-
3417
// Generate precomputed p256 tables.
3518
var pre [43][32 * 8]uint64
3619
basePoint := []uint64{
@@ -70,41 +53,21 @@ package elliptic
7053
}
7154
}
7255

73-
fmt.Fprint(buf, "const p256Precomputed = \"\" +\n\n")
56+
var bin []byte
7457

7558
// Dump the precomputed tables, flattened, little-endian.
7659
// These tables are used directly by assembly on little-endian platforms.
77-
// Putting the data in a const string lets it be stored readonly.
60+
// go:embedding the data into a string lets it be stored readonly.
7861
for i := range &pre {
79-
for j, v := range &pre[i] {
80-
fmt.Fprintf(buf, "\"")
62+
for _, v := range &pre[i] {
8163
var u8 [8]byte
8264
binary.LittleEndian.PutUint64(u8[:], v)
83-
for _, b := range &u8 {
84-
fmt.Fprintf(buf, "\\x%02x", b)
85-
}
86-
fmt.Fprintf(buf, "\"")
87-
if i < len(pre)-1 || j < len(pre[i])-1 {
88-
fmt.Fprint(buf, "+")
89-
}
90-
if j%8 == 7 {
91-
fmt.Fprint(buf, "\n")
92-
}
65+
bin = append(bin, u8[:]...)
9366
}
94-
fmt.Fprint(buf, "\n")
9567
}
9668

97-
src := buf.Bytes()
98-
fmtsrc, fmterr := format.Source(src)
99-
// If formatting failed, keep the original source for debugging.
100-
if fmterr == nil {
101-
src = fmtsrc
102-
}
103-
err := os.WriteFile("p256_asm_table.go", src, 0644)
69+
err := os.WriteFile("p256_asm_table.bin", bin, 0644)
10470
if err != nil {
10571
log.Fatal(err)
10672
}
107-
if fmterr != nil {
108-
log.Fatal(fmterr)
109-
}
11073
}

‎src/crypto/elliptic/p256_asm.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,15 @@
1515
package elliptic
1616

1717
import (
18+
_ "embed"
1819
"math/big"
1920
)
2021

2122
//go:generate go run -tags=tablegen gen_p256_table.go
2223

24+
//go:embed p256_asm_table.bin
25+
var p256Precomputed string
26+
2327
type (
2428
p256Curve struct {
2529
*CurveParams

‎src/crypto/elliptic/p256_asm_table.bin‎

86 KB
Binary file not shown.

0 commit comments

Comments
 (0)