codec/proto: reuse of marshal byte buffers

[adtac]

Performance benchmarks can be found below. Obviously, a 8 KiB
request/response is tailored to showcase this improvement as this is
where codec buffer reuse shines, but I've run other benchmarks too (like
1-byte requests and responses) and there's no discernable impact on
performance.

We do not allow reuse of buffers when stat handlers or binlogs are
turned on. This is because those two may need access to the data and
payload even after the data has been written to the wire. In such cases,
we never return the data back to the pool.

A buffer reuse threshold of 1 KiB was determined after several
experiments. There's diminished returns when buffer reuse is enabled for
smaller messages (actually, a negative impact).

unary-networkMode_none-bufConn_false-keepalive_false-benchTime_40s-trace_false-latency_0s-kbps_0-MTU_0-maxConcurrentCalls_6-reqSize_8192B-respSize_8192B-compressor_off-channelz_false-preloader_false
               Title       Before        After Percentage
            TotalOps       839638       906223     7.93%
             SendOps            0            0      NaN%
             RecvOps            0            0      NaN%
            Bytes/op    103788.29     80592.47   -22.35%
           Allocs/op       183.33       189.30     3.27%
             ReqT/op 1375662899.20 1484755763.20     7.93%
            RespT/op 1375662899.20 1484755763.20     7.93%
            50th-Lat    238.746µs    225.019µs    -5.75%
            90th-Lat    514.253µs    456.439µs   -11.24%
            99th-Lat    711.083µs    702.466µs    -1.21%
             Avg-Lat     285.45µs    264.456µs    -7.35%

Closes #2816

[adtac]

Well this turned out to be much more complicated than I originally thought:

• We can't have buffer reuse when stats handlers and binlogs are enabled because these can store references to the buffer internally.
• Buffer re-use is tricky when client retries are involved.

Previously, I returned the buffer back to the codec layer as soon as the transport layer (read: loopy) was done with the buffer, but that's not a good with retries because the buffered slice of retry funcs will retain references to the payload, which may actually be the plain encoded data if no compression is used. Therefore, we may return the buffer back to the encoder only after the entire RPC is concluded, so we need to store all the buffers to return.

[adtac]

Why is bufferReuseThreshold = 1024? It's because everything below it negatively impacts performance. Anything higher than that wastes memory when buffer reuse could positively impact performance.

[dfawley]

A few more comments otherwise LGTM. Note that vet is failing (go linter) because of some comments.

[adtac]

@dfawley thanks for the review, I've addressed all comments. CI should pass this time. PTAL; if you'd like to merge this in the future, let me know and I'll squash everything into two logical commits -- one for the leaktest change, one for the actual reuse change.

[dfawley]

FYI: travis is showing a data race that looks like it's caused by this PR. Can you take a look?

[adtac]

I literally can't reproduce this locally. I've run the failing test 8000 times (yes, 8000) with the race detector turned on and every single one test passed. And now Travis is green too. I don't want to say it's a memory corruption bug, but...

[dfawley]

I literally can't reproduce this locally. I've run the failing test 8000 times (yes, 8000) with the race detector turned on and every single one test passed. And now Travis is green too. I don't want to say it's a memory corruption bug, but...

travis? 🤷‍♂️

[tmthrgd]

Using a sync.Pool in this way is almost certainly incorrect and liable to cause performance problems and excessive heap growth: golang/go#23199 and golang/go#27735.