Buffer optimizations

[thedevop]

Few buffer optimizations:

1. Avoid creating/writing to outbound buffer if outbound was nil and pkt is larger than ClientNetWriteBufferSize
2. Use mempool for Properties Encode as well
3. Use the more efficient Write instead of WriteTo for Buffer to Buffer writes (as it won't be partially successful)

Benchmark comparing Write vs WriteTo (the performance difference is reverse proportion to the packet size, using 128 bytes for benchmark as vast majority packets without payload should be less than that):

func BenchmarkWrite(b *testing.B) {
	const PacketSize = 128
	d := make([]byte, PacketSize)
	if _, err := rand.Read(d); err != nil {
		panic(err)
	}
	dst := new(bytes.Buffer)

	b.ResetTimer()
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		src := bytes.NewBuffer(d)
		dst.Write(src.Bytes())
		dst.Reset()
	}
}

func BenchmarkWriteTo(b *testing.B) {
	const PacketSize = 128
	d := make([]byte, PacketSize)
	if _, err := rand.Read(d); err != nil {
		panic(err)
	}
	dst := new(bytes.Buffer)

	b.ResetTimer()
	b.ReportAllocs()

	for i := 0; i < b.N; i++ {
		src := bytes.NewBuffer(d)
		src.WriteTo(dst)
		dst.Reset()
	}
}

cpu: Intel(R) Core(TM) i7-7920HQ CPU @ 3.10GHz
BenchmarkWrite-8     	163843377	         7.306 ns/op	       0 B/op	       0 allocs/op
BenchmarkWriteTo-8   	100000000	        12.15 ns/op	       0 B/op	       0 allocs/op

[coveralls]

Pull Request Test Coverage Report for Build 7344840336

• 24 of 26 (92.31%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.001%) to 98.751%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
clients.go	7	9	77.78%

Totals
Change from base Build 7294752060:	0.001%
Covered Lines:	5535
Relevant Lines:	5605

---

💛 - Coveralls

[mochi-co]

This looks good to my eyes, but would like to confirm with @werbenhu before we merge

[werbenhu]

@thedevop I added two benchmark test cases with different results. This scenario should be closer to our use case, where we are writing from one buffer to another. Take a look at the test results.

func BenchmarkWrite(b *testing.B) {
	const PacketSize = 128
	d := make([]byte, PacketSize)
	if _, err := rand.Read(d); err != nil {
		panic(err)
	}
	dst := new(bytes.Buffer)
	
	b.ResetTimer()
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		src := bytes.NewBuffer(d)
		dst.Write(src.Bytes())
		dst.Reset()
	}
}

func BenchmarkWriteTo(b *testing.B) {
	const PacketSize = 128
	d := make([]byte, PacketSize)
	if _, err := rand.Read(d); err != nil {
		panic(err)
	}
	dst := new(bytes.Buffer)

	b.ResetTimer()
	b.ReportAllocs()

	for i := 0; i < b.N; i++ {
		src := bytes.NewBuffer(d)
		src.WriteTo(dst)
		dst.Reset()
	}
}

func BenchmarkWrite2(b *testing.B) {
	const PacketSize = 128
	d := make([]byte, PacketSize)
	if _, err := rand.Read(d); err != nil {
		panic(err)
	}
	dst := new(bytes.Buffer)
	src := bytes.NewBuffer(d)

	b.ResetTimer()
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		dst.Write(src.Bytes())
		dst.Reset()
	}
}

func BenchmarkWriteTo2(b *testing.B) {
	const PacketSize = 128
	d := make([]byte, PacketSize)
	if _, err := rand.Read(d); err != nil {
		panic(err)
	}
	dst := new(bytes.Buffer)

	src := bytes.NewBuffer(d)
	b.ResetTimer()
	b.ReportAllocs()

	for i := 0; i < b.N; i++ {
		src.WriteTo(dst)
		dst.Reset()
	}
}

BenchmarkWrite-4        100000000               10.02 ns/op            0 B/op          0 allocs/op
BenchmarkWriteTo-4      70746446                14.61 ns/op            0 B/op          0 allocs/op
BenchmarkWrite2-4       125998378                8.958 ns/op           0 B/op          0 allocs/op
BenchmarkWriteTo2-4     300416527                3.880 ns/op           0 B/op          0 allocs/op

[thedevop]

@werbenhu , Buffer once is read, it will no longer be available to read again. Hence

	for i := 0; i < b.N; i++ {
		src.WriteTo(dst)
		dst.Reset()
	}

will only write its content to dst the 1st time, subsequent writes within the for loop will be a 0 byte write. That's the reason in the original benchmark the src is reinitialize within the for loop.

Also, if we look at the source for Buffer.WriteTo, it actually calls the dst.Write with some additional checks and error handling. So dst.Write(src.Bytes()) will always outperform src.WriteTo(dst) if both src/dst are Buffer.

func (b *Buffer) WriteTo(w io.Writer) (n int64, err error) {
	b.lastRead = opInvalid
	if nBytes := b.Len(); nBytes > 0 {
		m, e := w.Write(b.buf[b.off:])
		if m > nBytes {
			panic("bytes.Buffer.WriteTo: invalid Write count")
		}
		b.off += m
		n = int64(m)
		if e != nil {
			return n, e
		}
		// all bytes should have been written, by definition of
		// Write method in io.Writer
		if m != nBytes {
			return n, io.ErrShortWrite
		}
	}
	// Buffer is now empty; reset.
	b.Reset()
	return n, nil
}

[werbenhu]

@thedevop You are right; it does look good.

[mochi-co]

I'm also very happy with this, so nice work @thedevop! Thank you @werbenhu for your review. I'll merge it in!