From: Ilya Tocar Date: Mon, 17 Sep 2018 19:08:03 +0000 (-0500) Subject: cmd/compile/internal/gc: inline autogenerated (*T).M wrappers X-Git-Tag: go1.12beta1~753 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=fa913a36a2793524a65972c7c65f4a0578cb3392;p=gostls13.git cmd/compile/internal/gc: inline autogenerated (*T).M wrappers Currently all inlining of autogenerated wrappers is disabled, because it causes build failures, when indexed export format is enabled. Turns out we can reenable it for common case of (*T).M wrappers. This fixes most performance degradation of 1.11 vs 1.10. encoding/binary: name old time/op new time/op delta ReadSlice1000Int32s-6 14.8µs ± 2% 11.5µs ± 2% -22.01% (p=0.000 n=10+10) WriteSlice1000Int32s-6 14.8µs ± 2% 11.7µs ± 2% -20.95% (p=0.000 n=10+10) bufio: name old time/op new time/op delta WriterFlush-6 32.4ns ± 1% 28.8ns ± 0% -11.17% (p=0.000 n=9+10) sort: SearchWrappers-6 231ns ± 1% 231ns ± 0% ~ (p=0.129 n=9+10) SortString1K-6 365µs ± 1% 298µs ± 1% -18.43% (p=0.000 n=9+10) SortString1K_Slice-6 274µs ± 2% 276µs ± 1% ~ (p=0.105 n=10+10) StableString1K-6 490µs ± 1% 373µs ± 1% -23.73% (p=0.000 n=10+10) SortInt1K-6 210µs ± 1% 142µs ± 1% -32.69% (p=0.000 n=10+10) StableInt1K-6 243µs ± 0% 151µs ± 1% -37.75% (p=0.000 n=10+10) StableInt1K_Slice-6 130µs ± 1% 130µs ± 0% ~ (p=0.237 n=10+8) SortInt64K-6 19.9ms ± 1% 13.5ms ± 1% -32.32% (p=0.000 n=10+10) SortInt64K_Slice-6 11.5ms ± 1% 11.5ms ± 1% ~ (p=0.912 n=10+10) StableInt64K-6 21.5ms ± 0% 13.5ms ± 1% -37.30% (p=0.000 n=9+10) Sort1e2-6 108µs ± 2% 83µs ± 3% -23.26% (p=0.000 n=10+10) Stable1e2-6 218µs ± 0% 161µs ± 1% -25.99% (p=0.000 n=8+9) Sort1e4-6 22.6ms ± 1% 16.8ms ± 0% -25.45% (p=0.000 n=10+7) Stable1e4-6 67.6ms ± 1% 49.7ms ± 0% -26.48% (p=0.000 n=10+10) Sort1e6-6 3.44s ± 0% 2.55s ± 1% -26.05% (p=0.000 n=8+9) Stable1e6-6 13.7s ± 0% 9.9s ± 1% -27.68% (p=0.000 n=8+10) Fixes #27621 Updates #25338 Change-Id: I6fe633202f63fa829a6ab849c44d7e45f8835dff Reviewed-on: https://go-review.googlesource.com/c/135697 Run-TryBot: Ilya Tocar TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky --- diff --git a/src/cmd/compile/internal/gc/inl_test.go b/src/cmd/compile/internal/gc/inl_test.go index 3fc0fbed1d..2f15cc3828 100644 --- a/src/cmd/compile/internal/gc/inl_test.go +++ b/src/cmd/compile/internal/gc/inl_test.go @@ -26,7 +26,8 @@ func TestIntendedInlining(t *testing.T) { t.Parallel() // want is the list of function names (by package) that should - // be inlined. + // be inlinable. If they have no callers in thier packages, they + // might not actually be inlined anywhere. want := map[string][]string{ "runtime": { // TODO(mvdan): enable these once mid-stack @@ -111,6 +112,11 @@ func TestIntendedInlining(t *testing.T) { "(*Buffer).UnreadByte", "(*Buffer).tryGrowByReslice", }, + "compress/flate": { + "byLiteral.Len", + "byLiteral.Less", + "byLiteral.Swap", + }, "unicode/utf8": { "FullRune", "FullRuneInString", @@ -162,6 +168,13 @@ func TestIntendedInlining(t *testing.T) { want["runtime"] = append(want["runtime"], "rotl_31") } + // Functions that must actually be inlined; they must have actual callers. + must := map[string]bool{ + "compress/flate.byLiteral.Len": true, + "compress/flate.byLiteral.Less": true, + "compress/flate.byLiteral.Swap": true, + } + notInlinedReason := make(map[string]string) pkgs := make([]string, 0, len(want)) for pname, fnames := range want { @@ -188,6 +201,7 @@ func TestIntendedInlining(t *testing.T) { scanner := bufio.NewScanner(pr) curPkg := "" canInline := regexp.MustCompile(`: can inline ([^ ]*)`) + haveInlined := regexp.MustCompile(`: inlining call to ([^ ]*)`) cannotInline := regexp.MustCompile(`: cannot inline ([^ ]*): (.*)`) for scanner.Scan() { line := scanner.Text() @@ -195,11 +209,20 @@ func TestIntendedInlining(t *testing.T) { curPkg = line[2:] continue } - if m := canInline.FindStringSubmatch(line); m != nil { + if m := haveInlined.FindStringSubmatch(line); m != nil { fname := m[1] delete(notInlinedReason, curPkg+"."+fname) continue } + if m := canInline.FindStringSubmatch(line); m != nil { + fname := m[1] + fullname := curPkg + "." + fname + // If function must be inlined somewhere, beeing inlinable is not enough + if _, ok := must[fullname]; !ok { + delete(notInlinedReason, fullname) + continue + } + } if m := cannotInline.FindStringSubmatch(line); m != nil { fname, reason := m[1], m[2] fullName := curPkg + "." + fname diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go index 9a6c61a651..7c9c8a157d 100644 --- a/src/cmd/compile/internal/gc/subr.go +++ b/src/cmd/compile/internal/gc/subr.go @@ -1728,11 +1728,10 @@ func genwrapper(rcvr *types.Type, method *types.Field, newnam *types.Sym) { Curfn = fn typecheckslice(fn.Nbody.Slice(), Etop) - // TODO(mdempsky): Investigate why this doesn't work with - // indexed export. For now, we disable even in non-indexed - // mode to ensure fair benchmark comparisons and to track down - // unintended compilation differences. - if false { + // Inline calls within (*T).M wrappers. This is safe because we only + // generate those wrappers within the same compilation unit as (T).M. + // TODO(mdempsky): Investigate why we can't enable this more generally. + if rcvr.IsPtr() && rcvr.Elem() == method.Type.Recv().Type && rcvr.Elem().Sym != nil { inlcalls(fn) } escAnalyze([]*Node{fn}, false)