perf: engine-independent decode wins (DiscardUnknown, HTTP buffer pool, PGO tooling) - #389
Merged
Conversation
…l, PGO tooling) Salvages the measured, engine-independent optimizations from the hyperpb exploration (now being left behind for its v0.1.x immaturity): - DiscardUnknown on all inbound client-proto unmarshals via a shared unmarshalClientProto helper (decode.go/decode_nebula.go/decode_push_gateway.go). +3.7% decode rate, -5.5% objects. The universal win: applies on BOTH the gRPC and HTTP ingest paths, which share decode()'s unmarshal. - Ingest buffer pooling (raw_bufpool.go) for the HTTP /raw base64 payload: 0 allocs/op, -14% bytes on that path. HTTP-only by nature — the gRPC majority has no base64 buffer of ours to pool, and grpc-go v1.81 already pools its transport buffers by default; those users get their win from DiscardUnknown. Safe because std protobuf-go copies bytes out on Unmarshal. - make pgo-capture/pgo-status, extended to auto-detect port + api_secret from config.toml (env still overrides; make pgo-config shows the target). The base committed default.pgo but not the tooling to refresh it. Tests: pool round-trip/reuse/concurrent (race-clean) + DiscardUnknown drops-unknown-keeps-known. Full suite + vet green. docs/decode-performance-findings.md collects the full retrospective (what's applied, what's already on base, reusable rig, dead-ends, method). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Three engine-independent decode optimizations — they need no change to the proto decoder or schema, are cheap, and are safe to ship on their own. Extracted from the decode-performance investigation (see
docs/decode-performance-findings.md) after the hyperpb engine swap was abandoned as too immature.1.
DiscardUnknownon inbound client protosEvery inbound game-client proto is now decoded through
unmarshalClientProto(proto.UnmarshalOptions{DiscardUnknown: true}) instead ofproto.Unmarshal. Golbat is read-only against these protos and never re-marshals them, so retaining unknown fields is pure overhead. Measured +~3.7% decode rate on its own.Golbat's own internal proto (
GolbatInternalindecoder/pokemon_decode.go) intentionally keeps plainproto.Unmarshal— it round-trips and must retain unknowns.2. HTTP base64 buffer pool
The HTTP
/rawingest path base64-decodes payloads into fresh buffers per request.raw_bufpool.gopools those buffers (sync.Pool), verified 0 allocs/op on the decode step. Note: this only helps the HTTP ingest path — the gRPC path already receives raw[]byteand grpc-go pools its transport buffers, so there's nothing of ours to pool there.3. PGO tooling (Makefile)
make pgo-capture/pgo-status/pgo-configtargets to capture a live CPU profile and drop it atdefault.pgo, which the Go compiler (≥1.21) applies automatically. No code change — just tooling to make profile-guided optimization repeatable. (Adefault.pgoalready lives in the repo; these targets refresh it.)Safety / testing
go build ./...andgo test ./...green (all 7 packages).raw_bufpool_test.gocovers pool round-trip/reuse/concurrency and thatDiscardUnknowndrops unknown fields while keeping known ones.Relationship to #388 (proto thinning)
The
DiscardUnknownchange here is also in #388, where it's a prerequisite for schema thinning (without it, thinned-away fields are retained as raw unknown-field blobs, negating the win). The two copies are the identical change, so they don't conflict — whichever merges second, the overlap is a no-op (or a trivial rebase). This PR additionally carries the HTTP buffer pool and PGO tooling, which #388 does not. They can merge in either order.