Reduce WritePacket memory allocation

[AkmalFairuz]

No description provided.

[TwistedAsylumMC]

Have you got any benchmarks to prove allocations have been reduced?

[Sandertv]

Any update on this? Any benchmarks?

[AkmalFairuz]

Tested in production, and I see no significant improvement in memory allocations. I have no idea how to benchmark this.

[TrippleAWap]

Benchmark Code

package minecraft

import (
        "context"
        "testing"

        "github.com/sandertv/gophertunnel/minecraft/protocol/packet"
)

func BenchmarkWritePacket(b *testing.B) {
        b.ReportAllocs()
        conn := &Conn{
                ctx:   context.Background(),
                hdr:   &packet.Header{},
                proto: DefaultProtocol,
        }
        p := &packet.Login{}
        for b.Loop() {
                _ = conn.WritePacket(p)
        }
}

---

Benchmark results for commit c7387286375eeb4933b1320a9e615d01724b5242 (master)

Benchmark Results for commit 102aa9295edbf648e337f0af9bd2ee7104e3eb44 (parent of PR)

Benchmark results for commit b4340106d95a2ac25ce8fb5b017dfb750c825d2e (PR)

---

In conclusion it seems this PR actually causes more allocations per WritePacket call.

[AkmalFairuz]

In conclusion it seems this PR actually causes more allocations per WritePacket call.

It's because your benchmark code never calls Flush, so the buffer is never reused.

[TrippleAWap]

In conclusion it seems this PR actually causes more allocations per WritePacket call.

It's because your benchmark code never calls Flush, so the buffer is never reused.

New benchmark code

package minecraft

import (
        "context"
        "io"
        "testing"

        "github.com/sandertv/gophertunnel/minecraft/protocol/packet"
)

func BenchmarkWritePacket(b *testing.B) {
        b.ReportAllocs()
        conn := &Conn{
                ctx:   context.Background(),
                hdr:   &packet.Header{},
                proto: DefaultProtocol,
                enc:   packet.NewEncoder(io.Discard),
        }
        p := &packet.Login{}
        for b.Loop() {
                _ = conn.WritePacket(p)
                _ = conn.Flush()
        }
}

---

Benchmark results for commit c7387286375eeb4933b1320a9e615d01724b5242 (master)

Benchmark results for commit b4340106d95a2ac25ce8fb5b017dfb750c825d2e (PR)

---

It still seems to cause more allocations, although the difference is obviously very inflated it still clearly causes more allocs.

[AkmalFairuz]

Your benchmark only writes a single packet, which is why this PR doesn't provide any benefit.

(1) Benchmark:

func BenchmarkWritePacket(b *testing.B) {
	b.ReportAllocs()
	conn := &Conn{
		ctx:   context.Background(),
		hdr:   &packet.Header{},
		proto: DefaultProtocol,
		enc:   packet.NewEncoder(io.Discard),
	}
	p := &packet.Login{}
	for b.Loop() {
		for i := 0; i < 5; i++ {
			_ = conn.WritePacket(p)
		}
		_ = conn.Flush()
	}
}

PR:

goos: darwin
goarch: arm64
pkg: github.com/sandertv/gophertunnel/minecraft
cpu: Apple M1 Pro
BenchmarkWritePacket
BenchmarkWritePacket-8   	 1716578	       694.1 ns/op	     280 B/op	      18 allocs/op
PASS

Process finished with the exit code 0

Parent PR (102aa92):

goos: darwin
goarch: arm64
pkg: github.com/sandertv/gophertunnel/minecraft
cpu: Apple M1 Pro
BenchmarkWritePacket
BenchmarkWritePacket-8   	 1890261	       630.5 ns/op	     336 B/op	      23 allocs/op
PASS

Process finished with the exit code 0

(2) Benchmark

func BenchmarkWritePacket(b *testing.B) {
	b.ReportAllocs()
	conn := &Conn{
		ctx:   context.Background(),
		hdr:   &packet.Header{},
		proto: DefaultProtocol,
		enc:   packet.NewEncoder(io.Discard),
	}
	p := &packet.Login{
		ConnectionRequest: make([]byte, 1024*1024),
	}
	for b.Loop() {
		for i := 0; i < 5; i++ {
			_ = conn.WritePacket(p)
		}
		_ = conn.Flush()
	}
}

PR:

goos: darwin
goarch: arm64
pkg: github.com/sandertv/gophertunnel/minecraft
cpu: Apple M1 Pro
BenchmarkWritePacket
BenchmarkWritePacket-8   	    2148	    556903 ns/op	 5268877 B/op	      19 allocs/op
PASS

Process finished with the exit code 0

Parent PR (102aa92)::

goos: darwin
goarch: arm64
pkg: github.com/sandertv/gophertunnel/minecraft
cpu: Apple M1 Pro
BenchmarkWritePacket
BenchmarkWritePacket-8   	    1342	    774996 ns/op	10585467 B/op	      24 allocs/op
PASS

Process finished with the exit code 0

In benchmark (2), as you can see, this PR reduces memory allocations (B/op) by ~50% compared to the parent PR.

[TrippleAWap]

I see, it lwk took me a minute to understand how this optimization worked while reading it but that's pretty neat.