Skip to content

Commit

Permalink
crypto/elliptic: use go:embed for the precomputed p256 table
Browse files Browse the repository at this point in the history
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 #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>
  • Loading branch information
mvdan committed Feb 8, 2022
1 parent 69e1711 commit c856fbf
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 1,473 deletions.
47 changes: 5 additions & 42 deletions src/crypto/elliptic/gen_p256_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,13 @@
package main

import (
"bytes"
"crypto/elliptic"
"encoding/binary"
"fmt"
"go/format"
"log"
"os"
)

func main() {
buf := new(bytes.Buffer)
fmt.Fprint(buf, `
// Copyright 2021 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// Generated by gen_p256_table.go. DO NOT EDIT.
//go:build amd64 || arm64
package elliptic
`[1:])

// Generate precomputed p256 tables.
var pre [43][32 * 8]uint64
basePoint := []uint64{
Expand Down Expand Up @@ -70,41 +53,21 @@ package elliptic
}
}

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

// Dump the precomputed tables, flattened, little-endian.
// These tables are used directly by assembly on little-endian platforms.
// Putting the data in a const string lets it be stored readonly.
// go:embedding the data into a string lets it be stored readonly.
for i := range &pre {
for j, v := range &pre[i] {
fmt.Fprintf(buf, "\"")
for _, v := range &pre[i] {
var u8 [8]byte
binary.LittleEndian.PutUint64(u8[:], v)
for _, b := range &u8 {
fmt.Fprintf(buf, "\\x%02x", b)
}
fmt.Fprintf(buf, "\"")
if i < len(pre)-1 || j < len(pre[i])-1 {
fmt.Fprint(buf, "+")
}
if j%8 == 7 {
fmt.Fprint(buf, "\n")
}
bin = append(bin, u8[:]...)
}
fmt.Fprint(buf, "\n")
}

src := buf.Bytes()
fmtsrc, fmterr := format.Source(src)
// If formatting failed, keep the original source for debugging.
if fmterr == nil {
src = fmtsrc
}
err := os.WriteFile("p256_asm_table.go", src, 0644)
err := os.WriteFile("p256_asm_table.bin", bin, 0644)
if err != nil {
log.Fatal(err)
}
if fmterr != nil {
log.Fatal(fmterr)
}
}
4 changes: 4 additions & 0 deletions src/crypto/elliptic/p256_asm.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@
package elliptic

import (
_ "embed"
"math/big"
)

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

//go:embed p256_asm_table.bin
var p256Precomputed string

type (
p256Curve struct {
*CurveParams
Expand Down
Binary file added src/crypto/elliptic/p256_asm_table.bin
Binary file not shown.

0 comments on commit c856fbf

Please sign in to comment.