]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: disable various write barrier optimizations
authorAustin Clements <austin@google.com>
Tue, 18 Oct 2016 14:26:28 +0000 (10:26 -0400)
committerAustin Clements <austin@google.com>
Fri, 28 Oct 2016 20:05:58 +0000 (20:05 +0000)
Several of our current write barrier elision optimizations are invalid
with the hybrid barrier. Eliding the hybrid barrier requires that
*both* the current and new pointer be already shaded and, since we
don't have the flow analysis to figure out anything about the slot's
current value, for now we have to just disable several of these
optimizations.

This has a slight impact on binary size. On linux/amd64, the go tool
binary increases by 0.7% and the compile binary increases by 1.5%.

It also has a slight impact on performance, as one would expect. We'll
win some of this back in subsequent commits.

name                      old time/op    new time/op    delta
BinaryTree17-12              2.38s ± 1%     2.40s ± 1%  +0.82%  (p=0.000 n=18+20)
Fannkuch11-12                2.84s ± 1%     2.70s ± 0%  -4.97%  (p=0.000 n=18+18)
FmtFprintfEmpty-12          44.2ns ± 1%    46.4ns ± 2%  +4.89%  (p=0.000 n=16+18)
FmtFprintfString-12          131ns ± 0%     134ns ± 1%  +2.05%  (p=0.000 n=12+19)
FmtFprintfInt-12             114ns ± 1%     117ns ± 1%  +3.26%  (p=0.000 n=19+20)
FmtFprintfIntInt-12          176ns ± 1%     181ns ± 1%  +3.25%  (p=0.000 n=20+20)
FmtFprintfPrefixedInt-12     185ns ± 1%     190ns ± 1%  +2.77%  (p=0.000 n=19+18)
FmtFprintfFloat-12           249ns ± 1%     254ns ± 1%  +1.71%  (p=0.000 n=18+20)
FmtManyArgs-12               747ns ± 1%     743ns ± 1%  -0.58%  (p=0.000 n=19+18)
GobDecode-12                6.57ms ± 1%    6.61ms ± 0%  +0.73%  (p=0.000 n=19+20)
GobEncode-12                5.58ms ± 1%    5.60ms ± 0%  +0.27%  (p=0.001 n=18+18)
Gzip-12                      223ms ± 1%     223ms ± 1%    ~     (p=0.351 n=19+20)
Gunzip-12                   37.9ms ± 0%    37.9ms ± 1%    ~     (p=0.095 n=16+20)
HTTPClientServer-12         77.8µs ± 1%    78.5µs ± 1%  +0.97%  (p=0.000 n=19+20)
JSONEncode-12               14.8ms ± 1%    14.8ms ± 1%    ~     (p=0.079 n=20+19)
JSONDecode-12               53.7ms ± 1%    54.2ms ± 1%  +0.92%  (p=0.000 n=20+19)
Mandelbrot200-12            3.81ms ± 1%    3.81ms ± 0%    ~     (p=0.916 n=19+18)
GoParse-12                  3.19ms ± 1%    3.19ms ± 1%    ~     (p=0.175 n=20+19)
RegexpMatchEasy0_32-12      71.9ns ± 1%    70.6ns ± 1%  -1.87%  (p=0.000 n=19+20)
RegexpMatchEasy0_1K-12       946ns ± 0%     944ns ± 0%  -0.22%  (p=0.000 n=19+16)
RegexpMatchEasy1_32-12      67.3ns ± 2%    66.8ns ± 1%  -0.72%  (p=0.008 n=20+20)
RegexpMatchEasy1_1K-12       374ns ± 1%     384ns ± 1%  +2.69%  (p=0.000 n=18+20)
RegexpMatchMedium_32-12      107ns ± 1%     107ns ± 1%    ~     (p=1.000 n=20+20)
RegexpMatchMedium_1K-12     34.3µs ± 1%    34.6µs ± 1%  +0.90%  (p=0.000 n=20+20)
RegexpMatchHard_32-12       1.78µs ± 1%    1.80µs ± 1%  +1.45%  (p=0.000 n=20+19)
RegexpMatchHard_1K-12       53.6µs ± 0%    54.5µs ± 1%  +1.52%  (p=0.000 n=19+18)
Revcomp-12                   417ms ± 5%     391ms ± 1%  -6.42%  (p=0.000 n=16+19)
Template-12                 61.1ms ± 1%    64.2ms ± 0%  +5.07%  (p=0.000 n=19+20)
TimeParse-12                 302ns ± 1%     305ns ± 1%  +0.90%  (p=0.000 n=18+18)
TimeFormat-12                319ns ± 1%     315ns ± 1%  -1.25%  (p=0.000 n=18+18)
[Geo mean]                  54.0µs         54.3µs       +0.58%

name         old time/op  new time/op  delta
XGarbage-12  2.24ms ± 2%  2.28ms ± 1%  +1.68%  (p=0.000 n=18+17)
XHTTP-12     11.4µs ± 1%  11.6µs ± 2%  +1.63%  (p=0.000 n=18+18)
XJSON-12     11.6ms ± 0%  12.5ms ± 0%  +7.84%  (p=0.000 n=18+17)

Updates #17503.

Change-Id: I1899f8e35662971e24bf692b517dfbe2b533c00c
Reviewed-on: https://go-review.googlesource.com/31572
Reviewed-by: Keith Randall <khr@golang.org>
src/cmd/compile/internal/gc/order.go
src/cmd/compile/internal/gc/sinit.go
src/cmd/compile/internal/gc/walk.go
test/fixedbugs/issue15747.go
test/writebarrier.go

index 6d235667821c250b8cecd037dab17991e9bf03c8..0a273556cd7ea3649d9c001776798583b43bf219 100644 (file)
@@ -423,7 +423,7 @@ func ordermapassign(n *Node, order *Order) {
                // We call writebarrierfat only for values > 4 pointers long. See walk.go.
                // TODO(mdempsky): writebarrierfat doesn't exist anymore, but removing that
                // logic causes net/http's tests to become flaky; see CL 21242.
-               if needwritebarrier(n.Left, n.Right) && n.Left.Type.Width > int64(4*Widthptr) && !isaddrokay(n.Right) {
+               if needwritebarrier(n.Left, n.Right) && n.Left.Type.Width > int64(4*Widthptr) && n.Right != nil && !isaddrokay(n.Right) {
                        m := n.Left
                        n.Left = ordertemp(m.Type, order, false)
                        a := nod(OAS, m, n.Left)
index 620d7c4b8966321c41bcded02303295ce8238aab..61b4245062014346025983463bc8c23f1a8deef7 100644 (file)
@@ -750,8 +750,13 @@ func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes)
                switch kind {
                case initKindStatic:
                        a = walkexpr(a, init) // add any assignments in r to top
+                       if a.Op == OASWB {
+                               // Static initialization never needs
+                               // write barriers.
+                               a.Op = OAS
+                       }
                        if a.Op != OAS {
-                               Fatalf("fixedlit: not as")
+                               Fatalf("fixedlit: not as, is %v", a)
                        }
                        a.IsStatic = true
                case initKindDynamic, initKindLocalCode:
index f0f4a998920eb2dcafbc07b8d39adc839ea22a02..143a2b08c6f01849c89130dbf7222b7144c7b53a 100644 (file)
@@ -721,7 +721,8 @@ opswitch:
                        break
                }
 
-               if n.Right == nil || iszero(n.Right) && !instrumenting {
+               if n.Right == nil {
+                       // TODO(austin): Check all "implicit zeroing"
                        break
                }
 
@@ -2255,17 +2256,20 @@ func needwritebarrier(l *Node, r *Node) bool {
                return false
        }
 
-       // No write barrier for implicit zeroing.
-       if r == nil {
-               return false
-       }
-
        // No write barrier if this is a pointer to a go:notinheap
        // type, since the write barrier's inheap(ptr) check will fail.
        if l.Type.IsPtr() && l.Type.Elem().NotInHeap {
                return false
        }
 
+       // Implicit zeroing is still zeroing, so it needs write
+       // barriers. In practice, these are all to stack variables
+       // (even if isstack isn't smart enough to figure that out), so
+       // they'll be eliminated by the backend.
+       if r == nil {
+               return true
+       }
+
        // Ignore no-op conversions when making decision.
        // Ensures that xp = unsafe.Pointer(&x) is treated
        // the same as xp = &x.
@@ -2273,15 +2277,13 @@ func needwritebarrier(l *Node, r *Node) bool {
                r = r.Left
        }
 
-       // No write barrier for zeroing or initialization to constant.
-       if iszero(r) || r.Op == OLITERAL {
-               return false
-       }
-
-       // No write barrier for storing static (read-only) data.
-       if r.Op == ONAME && strings.HasPrefix(r.Sym.Name, "statictmp_") {
-               return false
-       }
+       // TODO: We can eliminate write barriers if we know *both* the
+       // current and new content of the slot must already be shaded.
+       // We know a pointer is shaded if it's nil, or points to
+       // static data, a global (variable or function), or the stack.
+       // The nil optimization could be particularly useful for
+       // writes to just-allocated objects. Unfortunately, knowing
+       // the "current" value of the slot requires flow analysis.
 
        // No write barrier for storing address of stack values,
        // which are guaranteed only to be written to the stack.
@@ -2289,18 +2291,6 @@ func needwritebarrier(l *Node, r *Node) bool {
                return false
        }
 
-       // No write barrier for storing address of global, which
-       // is live no matter what.
-       if r.Op == OADDR && r.Left.isGlobal() {
-               return false
-       }
-
-       // No write barrier for storing global function, which is live
-       // no matter what.
-       if r.Op == ONAME && r.Class == PFUNC {
-               return false
-       }
-
        // Otherwise, be conservative and use write barrier.
        return true
 }
index 08aa09cbd74748f08494cb47b25c3d16634a1f12..c0209fbf634653230ffa6897152c1728682fff0d 100644 (file)
@@ -34,7 +34,7 @@ func f1(q *Q, xx []byte) interface{} { // ERROR "live at entry to f1: xx" "live
 //go:noinline
 func f2(d []byte, n int) (odata, res []byte, e interface{}) { // ERROR "live at entry to f2: d"
        if n > len(d) {
-               return d, nil, &T{M: "hello"} // ERROR "live at call to newobject: d"
+               return d, nil, &T{M: "hello"} // ERROR "live at call to newobject: d" "live at call to writebarrierptr: d"
        }
        res = d[:n]
        odata = d[n:]
index 6fb9cd7cfeee47e701c9733802471775a9d10c2e..6460a6f9daeed56a9c481a4bfe947329b404c164 100644 (file)
@@ -164,8 +164,9 @@ type T17 struct {
 }
 
 func f17(x *T17) {
-       // See golang.org/issue/13901
-       x.f = f17                      // no barrier
+       // Originally from golang.org/issue/13901, but the hybrid
+       // barrier requires both to have barriers.
+       x.f = f17                      // ERROR "write barrier"
        x.f = func(y *T17) { *y = *x } // ERROR "write barrier"
 }
 
@@ -207,8 +208,8 @@ func f21(x *int) {
        // Global -> heap pointer updates must have write barriers.
        x21 = x                   // ERROR "write barrier"
        y21.x = x                 // ERROR "write barrier"
-       x21 = &z21                // no barrier
-       y21.x = &z21              // no barrier
+       x21 = &z21                // ERROR "write barrier"
+       y21.x = &z21              // ERROR "write barrier"
        y21 = struct{ x *int }{x} // ERROR "write barrier"
 }