nsqd: remove cached len buf optimization

[ploxiln]

which was added in 9dde14f in 2013

the 4 byte read buffer should be on the stack

@mreiferson I just happened to notice this while looking around ... I still need to come up with a good way of validating that this doesn't cause any additional allocations (but I would be surprised and disappointed if it did)

[mreiferson]

nice catch... can probably verify with:

go build -gcflags -m 2>&1 | grep protocol_v2.go

[ploxiln]

Thanks, that's probably the easiest way (but I added /debug/pprof/allocs while I was trying some stuff). The readLen() function is on lines 1008 - 1016, so the relevant escape analysis output is:

$ go build -gcflags -m ./nsqd 2>&1 | grep protocol_v2.go:10
nsqd/protocol_v2.go:1011:23: inlining call to io.ReadFull
nsqd/protocol_v2.go:1015:38: inlining call to binary.bigEndian.Uint32
nsqd/protocol_v2.go:1019:23: inlining call to (*NSQD).getOpts
nsqd/protocol_v2.go:1019:23: inlining call to "sync/atomic".(*Value).Load
nsqd/protocol_v2.go:1020:36: inlining call to protocol.NewFatalClientErr
nsqd/protocol_v2.go:1001:6: can inline getMessageID
nsqd/protocol_v2.go:1003:25: inlining call to errors.New
nsqd/protocol_v2.go:1008:14: leaking param: r
nsqd/protocol_v2.go:1009:6: moved to heap: arr
nsqd/protocol_v2.go:1018:23: leaking param: client
nsqd/protocol_v2.go:1018:41: leaking param content: p
nsqd/protocol_v2.go:1018:56: leaking param: command
nsqd/protocol_v2.go:1021:15: ... argument does not escape
nsqd/protocol_v2.go:1021:16: command escapes to heap
nsqd/protocol_v2.go:1020:36: &protocol.FatalClientErr literal escapes to heap
nsqd/protocol_v2.go:108:21: ... argument does not escape
nsqd/protocol_v2.go:1001:19: leaking param: p to result ~r1 level=0
nsqd/protocol_v2.go:1003:25: &errors.errorString literal escapes to heap

It says "1009:6: moved to heap: arr" which doesn't look good ...

[ploxiln]

some more detail:

nsqd/protocol_v2.go:1009:6: arr escapes to heap:
nsqd/protocol_v2.go:1009:6:   flow: tmp = &arr:
nsqd/protocol_v2.go:1009:6:     from arr (address-of) at nsqd/protocol_v2.go:1010:12
nsqd/protocol_v2.go:1009:6:     from arr[0:3] (slice) at nsqd/protocol_v2.go:1010:12
nsqd/protocol_v2.go:1009:6:     from tmp := arr[0:3] (assign) at nsqd/protocol_v2.go:1010:6
nsqd/protocol_v2.go:1009:6:   flow: io.buf = tmp:
nsqd/protocol_v2.go:1009:6:     from io.r, io.buf = <N> (assign-pair) at nsqd/protocol_v2.go:1011:23
nsqd/protocol_v2.go:1009:6:   flow: {heap} = io.buf:
nsqd/protocol_v2.go:1009:6:     from io.ReadAtLeast(io.r, io.buf, len(io.buf)) (call parameter) at nsqd/protocol_v2.go:1011:23

(inlined io.ReadFull() uses io.ReadAtLeast())

[ploxiln]

I tried one more thing, and it seems like any kind of read demands a buffer on the heap.

nsqd/protocol_v2.go:1009:6: arr escapes to heap:
nsqd/protocol_v2.go:1009:6:   flow: {heap} = &arr:
nsqd/protocol_v2.go:1009:6:     from arr (address-of) at nsqd/protocol_v2.go:1012:24
nsqd/protocol_v2.go:1009:6:     from arr[n:] (slice) at nsqd/protocol_v2.go:1012:24
nsqd/protocol_v2.go:1009:6:     from r.Read(arr[n:]) (call parameter) at nsqd/protocol_v2.go:1012:20
nsqd/protocol_v2.go:1008:14: parameter r leaks to {heap} with derefs=0:
nsqd/protocol_v2.go:1008:14:   flow: {heap} = r:
nsqd/protocol_v2.go:1008:14:     from r.Read(arr[n:]) (call parameter) at nsqd/protocol_v2.go:1012:20

so I guess I'll just close this.

[ploxiln]

hah I guess this is actually supposed to work
golang/go#41543

[mreiferson]

want to re-land this?

[ploxiln]

after go 1.5.3 is released, I'll check again whether this works

[ploxiln]

go-1.5.3 was released a while ago, I'll check this again soon ...

[ploxiln]

welp, arr still seems to escape to heap (even though io.ReadFull() is inlined, and I also tried manually inlining it again)

$ go build -gcflags -m ./nsqd 2>&1 | grep protocol_v2.go:10
nsqd/protocol_v2.go:1011:23: inlining call to io.ReadFull
nsqd/protocol_v2.go:1015:38: inlining call to binary.bigEndian.Uint32
nsqd/protocol_v2.go:1019:23: inlining call to (*NSQD).getOpts
nsqd/protocol_v2.go:1019:23: inlining call to "sync/atomic".(*Value).Load
nsqd/protocol_v2.go:1020:36: inlining call to protocol.NewFatalClientErr
nsqd/protocol_v2.go:1001:6: can inline getMessageID
nsqd/protocol_v2.go:1003:25: inlining call to errors.New
nsqd/protocol_v2.go:1008:14: leaking param: r
nsqd/protocol_v2.go:1009:6: moved to heap: arr