By the way the thread that performs buf = x with write barrier will always do shade(*slot). If there is buf = buf[1:] racing with buf = x, even if the former does not use write barrier, the shade(*slot) of buf = x thread will shade original buf object in any case (it either sees buf or buf+1 as *slot but they both actually lead to the same allocated object). If so, since shade actually greys an object and grey objects never become white - only eventually black, we can say that original buf underlying object will stay alive - not deallocated, and so it is safe to do buf = buf[1:] without write barrier.

@aclements, would you mind to please provide your feedback here?

If there is a chance write barriers could be avoided on slice updates it would be a pity to miss 1.11, because, even though Go is compiled, performance is sometimes too painfull, especially when GC bits are intermixed.

Thanks beforehand,
Kirill

@navytux, good call. You're right that we can do this optimization now. It wasn't safe the last time we considered it, but the barrier has changed since then.

Consider two goroutines:

goroutine A does buf = buf[1:]:

1. r1 = buf.ptr
2. buf.ptr = r1+1

goroutine B does buf = s:

1. r2 = s.ptr
2. r1 = buf.ptr
3. shade(r1)
4. shade(r2)
5. buf.ptr = r2

(Ignoring len and cap, which a race could obviously shear, but that's an orthogonal problem.)

Since the reads commute with each other and everything commutes with shade, the only orderings that matter are A1/B5, A2/B2, and A2/B5. This leads to four schedules worth considering:

• A1-A2, B1-B5. This is simply a sequential schedule.
• A1, B1, B2, A2, B3-B5. The original buf.ptr is no longer reachable. It was shaded, but this is harmless.
• A1, B1-B5, A2. The original buf.ptr is still reachable, but this is okay because B3 shaded it.
• B1-B5, A1-A2. Again, this is simply a sequential schedule.

Prior to the hybrid barrier, we didn't have step B3, which is what makes the third schedule above safe.

Change https://golang.org/cl/105258 mentions this issue: cmd/compile: eliminate write barrier for b = b[n:]

@navytux, thanks for making us reconsider this optimization!

@RLH, can you double-check my logic? You're better at finding bugs in these things than I am. :)

@aclements thanks a lot for feedback and coming with the patch. I'm glad my observation could be useful.

@josharian thanks also for triggering whole this topic with b854339.

Status update: CL 105258 started to implement the idea but also along the way decided to generalize WB relax rule so that anything could happen between corresponding load and store in the same goroutine, including other writes to pointer in question. That generalization however brought correctness issues because now the race problem could be e.g. 3-body problem of 2 goroutines and GC, for example:

( buf.ptr initially points to A )

where G2.r2 results pointing to B while B was not marked.

( https://go-review.googlesource.com/c/go/+/105258/3/src/cmd/compile/internal/ssa/writebarrier.go#49 )

I do not have exact proof, but if we allow to omit generating write barrier only when:

write to pointer points to the same allocation as last seen by current goroutine for that pointer.

there won't be situations where intermediate buf.ptr instability could be leaking to another goroutine without being eventually marked, like in example above for G1 -> G2 leak of B:

• if update to pointer points to another allocation as last seen by current G - either by reading that pointer or by writing to it - we just generate WB as we do currently. This way there is no chance for another goroutine to observe either current write, or later write, even if that later write would point back to original allocation, with GC missing to mark corresponding object as live.

• if update to pointer points to the same allocation as last seen by current G - we know that without WB we are safe against write races from another mutator because WB triggered from that mutator would shade both inserted and deleted objects (see e.g. cmd/compile: slice update with self subslice - write barrier probably not needed (?) #24314 (comment) for details).

  additionally for this case, if WB is runtime disabled at the time of the write, because enabling or disabling WB happens a) only with the world stopped and b) even with planned safepoints everywhere still atomically with respect to

  if writeBarrier.needed {
      // perform write-barrier notification to GC
  }

  generated code parts, we can say that all mutators would see WB either enabled or disabled consistently, and so if WB is not yet enabled GC is not yet running and thus, even though there is race for write, GC will start scanning stacks and heap only later after the race is over and this way cannot miss to mark survived objects as live.

@aclements I understand we are in code freeze now, but CL 105258 was mailed before the freeze and so it would be a pity not to get it finished for Go1.11. Like I already said even though Go is compiled, performance is sometimes too painfull, especially when GC bits are intermixed, especially taking into account that slice self-update idiom is very common. It would be a pity to miss relaxing write-barriers for this common case ...

I appologize if I miss something and this optimization still cannot be applied even with my corrected rule.

Thanks beforehand,
Kirill

/cc @cherrymui

Silence... "words are very unnecessary - they can only do harm..."

This is still the case as of go version devel +0951939fd9 Fri Jul 17 19:09:40 2020 +0000 linux/amd64:

package issue24314

type Sumit struct {
        v []int
}

func (s *Sumit) SumAndFlush() int {
        r := 0
        for len(s.v) > 0 { 
                r += s.v[0]
                s.v = s.v[1:]        // <-- NOTE HERE
        }
        return r
}

#include "funcdata.h"

TEXT ·(*Sumit).SumAndFlush(SB), ABIInternal, $8-16 // bufself.go:7
        NO_LOCAL_POINTERS
pc0:
        // MOVQ    (TLS), CX (stack growth prologue)
        // CMPQ    SP, 16(CX)
        // PCDATA  $0, $-2
        // JLS     128
        // PCDATA  $0, $-1
        // SUBQ    $8, SP
        // MOVQ    BP, (SP) (BP save)
        // LEAQ    (SP), BP (BP init)
        // FUNCDATA $0, gclocals·a36216b97439c93dafebe03e7f0808b5(SB) (args)
        // FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB) (no locals)
        MOVQ       s+16(SP), AX  // bufself.go:9
        XORL       CX, CX
pc34:
        MOVQ       8(AX), DX
        MOVQ       (AX), BX
        MOVQ       16(AX), SI
        TESTQ      DX, DX
        JLE        pc109
        MOVQ       (BX), R8      // bufself.go:10
        DECQ       DX            // bufself.go:11
        MOVQ       DX, 8(AX)
        LEAQ       -1(SI), DX
        MOVQ       DX, 16(AX)
        ADDQ       R8, CX        // bufself.go:10
        NEGQ       DX            // bufself.go:11
        SARQ       $63, DX
        ANDQ       $8, DX
        ADDQ       BX, DX
        PCDATA     $0, $-2
        CMPL       runtime.writeBarrier(SB), $0               // <-- NOTE HERE
        JNE        pc99
        MOVQ       DX, (AX)
        JMP        pc34
pc99:
        MOVQ       AX, DI
        CALL       runtime.gcWriteBarrierDX(SB)               // <-- NOTE HERE
        JMP        pc34
pc109:
        PCDATA     $0, $-1       // bufself.go:13
        MOVQ       CX, _r0+24(SP)
        // MOVQ    (SP), BP (BP restore)
        // ADDQ    $8, SP (SP restore)
        RET
        NOP
        PCDATA     $1, $-1       // bufself.go:7
        PCDATA     $0, $-2
        NOP
        CALL       runtime.morestack_noctxt(SB)
        PCDATA     $0, $-1
        JMP        pc0