Commit 6f2ee0f3 authored by Josh Bleecher Snyder's avatar Josh Bleecher Snyder

cmd/compile: make builds reproducible in presence of **byte and **int8

CL 39915 introduced sorting of signats by ShortString
for reproducible builds. But ShortString treats types
byte and uint8 identically; same for rune and uint32.
CL 39915 attempted to compensate for this by only
adding the underlying type (uint8) to signats in addsignat.

This only works for byte and uint8. For e.g. *byte and *uint,
both get added, and their sort order is random,
leading to non-reproducible builds.

One fix would be to add yet another type printing mode
that doesn't eliminate byte and rune, and use it
for sorting signats. But the formatting routines
are complicated enough as it is.

Instead, just sort first by ShortString and then by String.
We can't just use String, because ShortString makes distinctions
that String doesn't. ShortString is really preferred here;
String is serving only as a backstop for handling of bytes and runes.

The long series of types in the test helps increase the odds of
failure, allowing a smaller number of iterations in the test.
On my machine, a full test takes 700ms.

Passes toolstash-check.

Updates #19961
Fixes #20272

name        old alloc/op      new alloc/op      delta
Template         37.9MB ± 0%       37.9MB ± 0%  +0.12%  (p=0.032 n=5+5)
Unicode          28.9MB ± 0%       28.9MB ± 0%    ~     (p=0.841 n=5+5)
GoTypes           110MB ± 0%        110MB ± 0%    ~     (p=0.841 n=5+5)
Compiler          463MB ± 0%        463MB ± 0%    ~     (p=0.056 n=5+5)
SSA              1.11GB ± 0%       1.11GB ± 0%  +0.02%  (p=0.016 n=5+5)
Flate            24.7MB ± 0%       24.8MB ± 0%  +0.14%  (p=0.032 n=5+5)
GoParser         31.1MB ± 0%       31.1MB ± 0%    ~     (p=0.421 n=5+5)
Reflect          73.9MB ± 0%       73.9MB ± 0%    ~     (p=1.000 n=5+5)
Tar              25.8MB ± 0%       25.8MB ± 0%  +0.15%  (p=0.016 n=5+5)
XML              41.2MB ± 0%       41.2MB ± 0%    ~     (p=0.310 n=5+5)
[Geo mean]       72.0MB            72.0MB       +0.07%

name        old allocs/op     new allocs/op     delta
Template           384k ± 0%         385k ± 1%    ~     (p=0.056 n=5+5)
Unicode            343k ± 0%         344k ± 0%    ~     (p=0.548 n=5+5)
GoTypes           1.16M ± 0%        1.16M ± 0%    ~     (p=0.421 n=5+5)
Compiler          4.43M ± 0%        4.44M ± 0%  +0.26%  (p=0.032 n=5+5)
SSA               9.86M ± 0%        9.87M ± 0%  +0.10%  (p=0.032 n=5+5)
Flate              237k ± 1%         238k ± 0%  +0.49%  (p=0.032 n=5+5)
GoParser           319k ± 1%         320k ± 1%    ~     (p=0.151 n=5+5)
Reflect            957k ± 0%         957k ± 0%    ~     (p=1.000 n=5+5)
Tar                251k ± 0%         252k ± 1%  +0.49%  (p=0.016 n=5+5)
XML                399k ± 0%         401k ± 1%    ~     (p=0.310 n=5+5)
[Geo mean]         739k              741k       +0.26%

Change-Id: Ic27995a8d374d012b8aca14546b1df9d28d30df7
Reviewed-on: https://go-review.googlesource.com/42955
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
parent 9fda4df9
......@@ -1412,7 +1412,7 @@ func itabsym(it *obj.LSym, offset int64) *obj.LSym {
}
func addsignat(t *types.Type) {
signatlist[formalType(t)] = true
signatlist[t] = true
}
func addsignats(dcls []*Node) {
......@@ -1432,7 +1432,7 @@ func dumpsignats() {
signats = signats[:0]
// Transfer entries to a slice and sort, for reproducible builds.
for t := range signatlist {
signats = append(signats, typeAndStr{t: t, s: typesymname(t)})
signats = append(signats, typeAndStr{t: t, short: typesymname(t), regular: t.String()})
delete(signatlist, t)
}
sort.Sort(typesByString(signats))
......@@ -1543,13 +1543,23 @@ func dumpbasictypes() {
type typeAndStr struct {
t *types.Type
s string
short string
regular string
}
type typesByString []typeAndStr
func (a typesByString) Len() int { return len(a) }
func (a typesByString) Less(i, j int) bool { return a[i].s < a[j].s }
func (a typesByString) Less(i, j int) bool {
if a[i].short != a[j].short {
return a[i].short < a[j].short
}
// When the only difference between the types is whether
// they refer to byte or uint8, such as **byte vs **uint8,
// the types' ShortStrings can be identical.
// To preserve deterministic sort ordering, sort these by String().
return a[i].regular < a[j].regular
}
func (a typesByString) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
func dalgsym(t *types.Type) *obj.LSym {
......
// Copyright 2017 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.
package gc_test
import (
"bytes"
"internal/testenv"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"testing"
)
func TestReproducibleBuilds(t *testing.T) {
testenv.MustHaveGoBuild(t)
iters := 10
if testing.Short() {
iters = 4
}
t.Parallel()
var want []byte
tmp, err := ioutil.TempFile("", "")
if err != nil {
t.Fatalf("temp file creation failed: %v", err)
}
defer os.Remove(tmp.Name())
defer tmp.Close()
for i := 0; i < iters; i++ {
out, err := exec.Command(testenv.GoToolPath(t), "tool", "compile", "-o", tmp.Name(), filepath.Join("testdata", "reproducible", "issue20272.go")).CombinedOutput()
if err != nil {
t.Fatalf("failed to compile: %v\n%s", err, out)
}
obj, err := ioutil.ReadFile(tmp.Name())
if err != nil {
t.Fatalf("failed to read object file: %v", err)
}
if i == 0 {
want = obj
} else {
if !bytes.Equal(want, obj) {
t.Fatalf("builds produced different output after %d iters (%d bytes vs %d bytes)", i, len(want), len(obj))
}
}
}
}
// Copyright 2017 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.
package p
var (
i0 uint8
b0 byte
i1 *uint8
b1 *byte
i2 **uint8
b2 **byte
i3 ***uint8
b3 ***byte
i4 ****uint8
b4 ****byte
i5 *****uint8
b5 *****byte
i6 ******uint8
b6 ******byte
i7 *******uint8
b7 *******byte
i8 ********uint8
b8 ********byte
)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment