]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile/internal/gc: inline autogenerated (*T).M wrappers
authorIlya Tocar <ilya.tocar@intel.com>
Mon, 17 Sep 2018 19:08:03 +0000 (14:08 -0500)
committerIlya Tocar <ilya.tocar@intel.com>
Tue, 16 Oct 2018 22:02:06 +0000 (22:02 +0000)
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 <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
src/cmd/compile/internal/gc/inl_test.go
src/cmd/compile/internal/gc/subr.go

index 3fc0fbed1d639c5eb84152053bdfdb3037693977..2f15cc3828b4355fb248b059deaf9601512a8a95 100644 (file)
@@ -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
index 9a6c61a651a41c6a278698579f1f9eec185d6394..7c9c8a157dd815125839a293baf8329987bcaa80 100644 (file)
@@ -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)