packet writing optimizations - #63
Conversation
Changed: - minecraft/conn.go:532: encodePacketsTo now queues owned pooled buffers instead of copying each encoded packet. - minecraft/conn.go:516: WritePacket now uses the optimized pooled queue path. - minecraft/conn.go:641: WritePacketDirect uses a reusable per-conn queue and is now 0 alloc in the benchmark. - minecraft/protocol/packet/encoder.go:132: encoder can use a writer-based compression fast path. - minecraft/protocol/packet/compression.go:109: flate now compresses directly into the encoder’s pooled output buffer. - Added focused tests/benchmarks in conn_bench_test.go, encoder_bench_test.go, and compression_test.go. Measured locally: - BenchmarkConnWritePacketDirect: now ~80-90 ns/op, 0 B/op, 0 allocs/op (baseline was ~160-169 ns/op, 720 B/op, 4 allocs/op). - BenchmarkConnWritePacketDirectBatch: now ~798-883 ns/op, 0 B/op, 0 allocs/op (baseline was ~1890-1965 ns/op, 10664 B/op, 51 allocs/op). - BenchmarkEncoderEncodeFlate: now 0 allocs/op (baseline was 1 alloc/op). Validation: - go test ./minecraft/protocol/packet passed. - go test ./minecraft -run 'Test|^$' passed. - go test -run '^$' ./... passed compile/no-test run. - go test excluding minecraft/room and minecraft/service/signaling passed; full go test ./... hung in those existing network-oriented packages and was stopped. I left Reader.ByteSlice unchanged because zero-copy there can alias Decoder.buf on uncompressed reads, which would change slice lifetime semantics.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…izations-67 # Conflicts: # minecraft/conn.go
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
1 participant
Note
Medium Risk
Changes core send/flush/encode paths and buffer ownership; behavior should match prior logic but regressions could affect ordering, memory, or wire format under edge cases.
Overview
This PR speeds up outbound packet encoding on
Connand in the batch encoder by cutting allocations and improving buffer lifecycle management.Connnow queues sends with apacketQueuethat tracks pooledbytes.Bufferinstances alongside packet slices, releases them after flush/encode, and skips returning oversized buffers to the pool (1 MiB cap). Encoding no longer copies every packet to a new[]byte; marshaling can reuse a resettable protocol writer, andWritePacketDirectuses a dedicated queue plusdirectMuinstead of a stack buffer.Flushswaps full/spare queues and callsrelease()on the batch being encoded.Compression/encoding: Flate can stream into an existing buffer via
compressTo, whichEncoderprefers (like Snappy’s append path).Connbenchmarks and encoder/compression benchmarks plus acompressTotest document the behavior.Reviewed by Cursor Bugbot for commit 3d9fc70. Bugbot is set up for automated code reviews on this repo. Configure here.
Related PR
Closes Sandertv#341.