diff --git a/Makefile b/Makefile index 2001273f..62058f9d 100644 --- a/Makefile +++ b/Makefile @@ -12,4 +12,43 @@ FLAGS = go_json golbat: FORCE go build -tags $(FLAGS) golbat +# --- Profile-guided optimization -------------------------------------------- +# Go (>= 1.21) automatically applies ./default.pgo when building the golbat +# main package — locally and in the Docker build (which copies the repo). So +# committing default.pgo makes every build profile-guided; no flag needed. +# Refresh it occasionally (game updates shift hot paths), then commit: +# +# make pgo-capture # reads port + api_secret from config.toml +# GOLBAT_URL=https://host:9001 GOLBAT_SECRET=... make pgo-capture # or override +# +# Port and secret are auto-detected from config.toml so a local operator can +# just `make pgo-capture`; any of GOLBAT_URL / GOLBAT_HOST / GOLBAT_PORT / +# GOLBAT_SECRET / CONFIG override the detection. +CONFIG ?= config.toml +DETECTED_PORT := $(shell grep -E '^[[:space:]]*port[[:space:]]*=' $(CONFIG) 2>/dev/null | head -1 | sed -E 's/^[^=]*=[[:space:]]*([0-9]+).*/\1/') +DETECTED_SECRET := $(shell grep -E '^[[:space:]]*api_secret[[:space:]]*=' $(CONFIG) 2>/dev/null | head -1 | sed -E 's/^[^=]*=[[:space:]]*"([^"]*)".*/\1/') +GOLBAT_HOST ?= 127.0.0.1 +GOLBAT_PORT ?= $(if $(DETECTED_PORT),$(DETECTED_PORT),9001) +GOLBAT_SECRET ?= $(DETECTED_SECRET) +GOLBAT_URL ?= http://$(GOLBAT_HOST):$(GOLBAT_PORT) +PGO_SECONDS ?= 120 + +pgo-config: FORCE ## show the URL/secret pgo-capture will use (from config.toml) + @echo "config file : $(CONFIG)" + @echo "capture URL : $(GOLBAT_URL)" + @echo "secret : $(if $(GOLBAT_SECRET),(set, $(shell printf %s '$(GOLBAT_SECRET)' | wc -c | tr -d ' ') chars),(none))" + +pgo-capture: FORCE ## capture a CPU profile from a running golbat into default.pgo + curl -fsS $(if $(GOLBAT_SECRET),-H "X-Golbat-Secret: $(GOLBAT_SECRET)") \ + "$(GOLBAT_URL)/debug/pprof/profile?seconds=$(PGO_SECONDS)" -o default.pgo.tmp + mv default.pgo.tmp default.pgo + @echo "captured default.pgo ($$(du -h default.pgo | cut -f1)) from $(GOLBAT_URL) — commit it to apply to all builds" + +pgo-status: FORCE ## report whether builds will be profile-guided + @if [ -f default.pgo ]; then \ + echo "default.pgo present ($$(du -h default.pgo | cut -f1)) — builds are profile-guided"; \ + else \ + echo "no default.pgo — builds are NOT profile-guided (run make pgo-capture against prod)"; \ + fi + FORCE: ; \ No newline at end of file diff --git a/decode.go b/decode.go index aaa1c871..8a5389b6 100644 --- a/decode.go +++ b/decode.go @@ -13,6 +13,21 @@ import ( "google.golang.org/protobuf/proto" ) +// clientProtoUnmarshalOpts decodes inbound game-client protos with +// DiscardUnknown. This is safe — and a measured win (+~3.7% decode rate, +// −5.5% allocated objects) — because Golbat only ever READS these protos and +// never re-serializes them, so protobuf-go's default behavior of retaining +// unknown fields (present because production payloads are a newer game version +// than our vbase schema) is pure allocation churn. Applies on both ingest +// paths, since both funnel their per-packet decode through the calls below. +var clientProtoUnmarshalOpts = proto.UnmarshalOptions{DiscardUnknown: true} + +// unmarshalClientProto is proto.Unmarshal with DiscardUnknown. Use it for +// every inbound client proto (see clientProtoUnmarshalOpts). +func unmarshalClientProto(b []byte, m proto.Message) error { + return clientProtoUnmarshalOpts.Unmarshal(b, m) +} + func decode(ctx context.Context, method int, protoData *ProtoData) { getMethodName := func(method int, trimString bool) string { if val, ok := pogo.Method_name[int32(method)]; ok { @@ -144,7 +159,7 @@ func decodeQuest(ctx context.Context, sDec []byte, haveAr *bool) string { return "No AR quest info" } decodedQuest := &pogo.FortSearchOutProto{} - if err := proto.Unmarshal(sDec, decodedQuest); err != nil { + if err := unmarshalClientProto(sDec, decodedQuest); err != nil { log.Errorf("Failed to parse %s", err) statsCollector.IncDecodeQuest("error", "parse") return "Parse failure" @@ -164,7 +179,7 @@ func decodeQuest(ctx context.Context, sDec []byte, haveAr *bool) string { func decodeSocialActionWithRequest(request []byte, payload []byte) string { var proxyRequestProto pogo.ProxyRequestProto - if err := proto.Unmarshal(request, &proxyRequestProto); err != nil { + if err := unmarshalClientProto(request, &proxyRequestProto); err != nil { log.Errorf("Failed to parse %s", err) statsCollector.IncDecodeSocialActionWithRequest("error", "request_parse") return fmt.Sprintf("Failed to parse %s", err) @@ -172,7 +187,7 @@ func decodeSocialActionWithRequest(request []byte, payload []byte) string { var proxyResponseProto pogo.ProxyResponseProto - if err := proto.Unmarshal(payload, &proxyResponseProto); err != nil { + if err := unmarshalClientProto(payload, &proxyResponseProto); err != nil { log.Errorf("Failed to parse %s", err) statsCollector.IncDecodeSocialActionWithRequest("error", "response_parse") return fmt.Sprintf("Failed to parse %s", err) @@ -199,7 +214,7 @@ func decodeSocialActionWithRequest(request []byte, payload []byte) string { func decodeGetFriendDetails(payload []byte) string { var getFriendDetailsOutProto pogo.InternalGetFriendDetailsOutProto - getFriendDetailsError := proto.Unmarshal(payload, &getFriendDetailsOutProto) + getFriendDetailsError := unmarshalClientProto(payload, &getFriendDetailsOutProto) if getFriendDetailsError != nil { statsCollector.IncDecodeGetFriendDetails("error", "parse") @@ -229,7 +244,7 @@ func decodeGetFriendDetails(payload []byte) string { func decodeSearchPlayer(proxyRequestProto *pogo.ProxyRequestProto, payload []byte) string { var searchPlayerOutProto pogo.InternalSearchPlayerOutProto - searchPlayerOutError := proto.Unmarshal(payload, &searchPlayerOutProto) + searchPlayerOutError := unmarshalClientProto(payload, &searchPlayerOutProto) if searchPlayerOutError != nil { log.Errorf("Failed to parse %s", searchPlayerOutError) @@ -243,7 +258,7 @@ func decodeSearchPlayer(proxyRequestProto *pogo.ProxyRequestProto, payload []byt } var searchPlayerProto pogo.InternalSearchPlayerProto - searchPlayerError := proto.Unmarshal(proxyRequestProto.GetPayload(), &searchPlayerProto) + searchPlayerError := unmarshalClientProto(proxyRequestProto.GetPayload(), &searchPlayerProto) if searchPlayerError != nil || searchPlayerProto.GetFriendCode() == "" { statsCollector.IncDecodeSearchPlayer("error", "parse") @@ -263,7 +278,7 @@ func decodeSearchPlayer(proxyRequestProto *pogo.ProxyRequestProto, payload []byt func decodeFortDetails(ctx context.Context, sDec []byte) string { decodedFort := &pogo.FortDetailsOutProto{} - if err := proto.Unmarshal(sDec, decodedFort); err != nil { + if err := unmarshalClientProto(sDec, decodedFort); err != nil { log.Errorf("Failed to parse %s", err) statsCollector.IncDecodeFortDetails("error", "parse") return fmt.Sprintf("Failed to parse %s", err) @@ -284,7 +299,7 @@ func decodeFortDetails(ctx context.Context, sDec []byte) string { func decodeGetMapForts(ctx context.Context, sDec []byte) string { decodedMapForts := &pogo.GetMapFortsOutProto{} - if err := proto.Unmarshal(sDec, decodedMapForts); err != nil { + if err := unmarshalClientProto(sDec, decodedMapForts); err != nil { log.Errorf("Failed to parse %s", err) statsCollector.IncDecodeGetMapForts("error", "parse") return fmt.Sprintf("Failed to parse %s", err) @@ -317,7 +332,7 @@ func decodeGetMapForts(ctx context.Context, sDec []byte) string { func decodeGetRoutes(ctx context.Context, payload []byte) string { getRoutesOutProto := &pogo.GetRoutesOutProto{} - if err := proto.Unmarshal(payload, getRoutesOutProto); err != nil { + if err := unmarshalClientProto(payload, getRoutesOutProto); err != nil { return fmt.Sprintf("failed to decode GetRoutesOutProto %s", err) } @@ -358,7 +373,7 @@ func decodeGetRoutes(ctx context.Context, payload []byte) string { func decodeGetGymInfo(ctx context.Context, sDec []byte) string { decodedGymInfo := &pogo.GymGetInfoOutProto{} - if err := proto.Unmarshal(sDec, decodedGymInfo); err != nil { + if err := unmarshalClientProto(sDec, decodedGymInfo); err != nil { log.Errorf("Failed to parse %s", err) statsCollector.IncDecodeGetGymInfo("error", "parse") return fmt.Sprintf("Failed to parse %s", err) @@ -377,7 +392,7 @@ func decodeGetGymInfo(ctx context.Context, sDec []byte) string { func decodeEncounter(ctx context.Context, sDec []byte, username string, timestampMs int64) string { decodedEncounterInfo := &pogo.EncounterOutProto{} - if err := proto.Unmarshal(sDec, decodedEncounterInfo); err != nil { + if err := unmarshalClientProto(sDec, decodedEncounterInfo); err != nil { log.Errorf("Failed to parse %s", err) statsCollector.IncDecodeEncounter("error", "parse") return fmt.Sprintf("Failed to parse %s", err) @@ -396,7 +411,7 @@ func decodeEncounter(ctx context.Context, sDec []byte, username string, timestam func decodeDiskEncounter(ctx context.Context, sDec []byte, username string) string { decodedEncounterInfo := &pogo.DiskEncounterOutProto{} - if err := proto.Unmarshal(sDec, decodedEncounterInfo); err != nil { + if err := unmarshalClientProto(sDec, decodedEncounterInfo); err != nil { log.Errorf("Failed to parse %s", err) statsCollector.IncDecodeDiskEncounter("error", "parse") return fmt.Sprintf("Failed to parse %s", err) @@ -415,7 +430,7 @@ func decodeDiskEncounter(ctx context.Context, sDec []byte, username string) stri func decodeStartIncident(ctx context.Context, sDec []byte) string { decodedIncident := &pogo.StartIncidentOutProto{} - if err := proto.Unmarshal(sDec, decodedIncident); err != nil { + if err := unmarshalClientProto(sDec, decodedIncident); err != nil { log.Errorf("Failed to parse %s", err) statsCollector.IncDecodeStartIncident("error", "parse") return fmt.Sprintf("Failed to parse %s", err) @@ -435,7 +450,7 @@ func decodeStartIncident(ctx context.Context, sDec []byte) string { func decodeOpenInvasion(ctx context.Context, request []byte, payload []byte) string { decodeOpenInvasionRequest := &pogo.OpenInvasionCombatSessionProto{} - if err := proto.Unmarshal(request, decodeOpenInvasionRequest); err != nil { + if err := unmarshalClientProto(request, decodeOpenInvasionRequest); err != nil { log.Errorf("Failed to parse %s", err) statsCollector.IncDecodeOpenInvasion("error", "parse") return fmt.Sprintf("Failed to parse %s", err) @@ -445,7 +460,7 @@ func decodeOpenInvasion(ctx context.Context, request []byte, payload []byte) str } decodedOpenInvasionResponse := &pogo.OpenInvasionCombatSessionOutProto{} - if err := proto.Unmarshal(payload, decodedOpenInvasionResponse); err != nil { + if err := unmarshalClientProto(payload, decodedOpenInvasionResponse); err != nil { log.Errorf("Failed to parse %s", err) statsCollector.IncDecodeOpenInvasion("error", "parse") return fmt.Sprintf("Failed to parse %s", err) @@ -465,7 +480,7 @@ func decodeOpenInvasion(ctx context.Context, request []byte, payload []byte) str func decodeGMO(ctx context.Context, protoData *ProtoData, scanParameters decoder.ScanParameters) string { decodedGmo := &pogo.GetMapObjectsOutProto{} - if err := proto.Unmarshal(protoData.Data, decodedGmo); err != nil { + if err := unmarshalClientProto(protoData.Data, decodedGmo); err != nil { statsCollector.IncDecodeGMO("error", "parse") log.Errorf("Failed to parse %s", err) } @@ -587,14 +602,14 @@ func isCellNotEmpty(mapCell *pogo.ClientMapCellProto) bool { func decodeGetContestData(ctx context.Context, request []byte, data []byte) string { var decodedContestData pogo.GetContestDataOutProto - if err := proto.Unmarshal(data, &decodedContestData); err != nil { + if err := unmarshalClientProto(data, &decodedContestData); err != nil { log.Errorf("Failed to parse GetContestDataOutProto %s", err) return fmt.Sprintf("Failed to parse GetContestDataOutProto %s", err) } var decodedContestDataRequest pogo.GetContestDataProto if request != nil { - if err := proto.Unmarshal(request, &decodedContestDataRequest); err != nil { + if err := unmarshalClientProto(request, &decodedContestDataRequest); err != nil { log.Errorf("Failed to parse GetContestDataProto %s", err) return fmt.Sprintf("Failed to parse GetContestDataProto %s", err) } @@ -604,7 +619,7 @@ func decodeGetContestData(ctx context.Context, request []byte, data []byte) stri func decodeGetPokemonSizeContestEntry(ctx context.Context, request []byte, data []byte) string { var decodedPokemonSizeContestEntry pogo.GetPokemonSizeLeaderboardEntryOutProto - if err := proto.Unmarshal(data, &decodedPokemonSizeContestEntry); err != nil { + if err := unmarshalClientProto(data, &decodedPokemonSizeContestEntry); err != nil { log.Errorf("Failed to parse GetPokemonSizeLeaderboardEntryOutProto %s", err) return fmt.Sprintf("Failed to parse GetPokemonSizeLeaderboardEntryOutProto %s", err) } @@ -615,7 +630,7 @@ func decodeGetPokemonSizeContestEntry(ctx context.Context, request []byte, data var decodedPokemonSizeContestEntryRequest pogo.GetPokemonSizeLeaderboardEntryProto if request != nil { - if err := proto.Unmarshal(request, &decodedPokemonSizeContestEntryRequest); err != nil { + if err := unmarshalClientProto(request, &decodedPokemonSizeContestEntryRequest); err != nil { log.Errorf("Failed to parse GetPokemonSizeLeaderboardEntryOutProto %s", err) return fmt.Sprintf("Failed to parse GetPokemonSizeLeaderboardEntryOutProto %s", err) } @@ -626,14 +641,14 @@ func decodeGetPokemonSizeContestEntry(ctx context.Context, request []byte, data func decodeGetStationDetails(ctx context.Context, request []byte, data []byte) string { var decodedGetStationDetails pogo.GetStationedPokemonDetailsOutProto - if err := proto.Unmarshal(data, &decodedGetStationDetails); err != nil { + if err := unmarshalClientProto(data, &decodedGetStationDetails); err != nil { log.Errorf("Failed to parse GetStationedPokemonDetailsOutProto %s", err) return fmt.Sprintf("Failed to parse GetStationedPokemonDetailsOutProto %s", err) } var decodedGetStationDetailsRequest pogo.GetStationedPokemonDetailsProto if request != nil { - if err := proto.Unmarshal(request, &decodedGetStationDetailsRequest); err != nil { + if err := unmarshalClientProto(request, &decodedGetStationDetailsRequest); err != nil { log.Errorf("Failed to parse GetStationedPokemonDetailsProto %s", err) return fmt.Sprintf("Failed to parse GetStationedPokemonDetailsProto %s", err) } @@ -651,14 +666,14 @@ func decodeGetStationDetails(ctx context.Context, request []byte, data []byte) s func decodeTappable(ctx context.Context, request, data []byte, username string, timestampMs int64) string { var tappable pogo.ProcessTappableOutProto - if err := proto.Unmarshal(data, &tappable); err != nil { + if err := unmarshalClientProto(data, &tappable); err != nil { log.Errorf("Failed to parse %s", err) return fmt.Sprintf("Failed to parse ProcessTappableOutProto %s", err) } var tappableRequest pogo.ProcessTappableProto if request != nil { - if err := proto.Unmarshal(request, &tappableRequest); err != nil { + if err := unmarshalClientProto(request, &tappableRequest); err != nil { log.Errorf("Failed to parse %s", err) return fmt.Sprintf("Failed to parse ProcessTappableProto %s", err) } @@ -676,14 +691,14 @@ func decodeTappable(ctx context.Context, request, data []byte, username string, func decodeGetEventRsvp(ctx context.Context, request []byte, data []byte) string { var rsvp pogo.GetEventRsvpsOutProto - if err := proto.Unmarshal(data, &rsvp); err != nil { + if err := unmarshalClientProto(data, &rsvp); err != nil { log.Errorf("Failed to parse %s", err) return fmt.Sprintf("Failed to parse GetEventRsvpsOutProto %s", err) } var rsvpRequest pogo.GetEventRsvpsProto if request != nil { - if err := proto.Unmarshal(request, &rsvpRequest); err != nil { + if err := unmarshalClientProto(request, &rsvpRequest); err != nil { log.Errorf("Failed to parse %s", err) return fmt.Sprintf("Failed to parse GetEventRsvpsProto %s", err) } @@ -705,7 +720,7 @@ func decodeGetEventRsvp(ctx context.Context, request []byte, data []byte) string func decodeGetEventRsvpCount(ctx context.Context, data []byte) string { var rsvp pogo.GetEventRsvpCountOutProto - if err := proto.Unmarshal(data, &rsvp); err != nil { + if err := unmarshalClientProto(data, &rsvp); err != nil { log.Errorf("Failed to parse %s", err) return fmt.Sprintf("Failed to parse GetEventRsvpCountOutProto %s", err) } diff --git a/decode_nebula.go b/decode_nebula.go index c8b47bba..4dbbb0be 100644 --- a/decode_nebula.go +++ b/decode_nebula.go @@ -8,7 +8,6 @@ import ( "golbat/pogo" log "github.com/sirupsen/logrus" - "google.golang.org/protobuf/proto" ) // decodeNebula routes on the typed context (the proto `oneof context` case, @@ -36,7 +35,7 @@ func decodeNebula(ctx context.Context, endpoint string, nd *NebulaData) string { func decodeNebulaInvasionState(ctx context.Context, fortId, incidentId string, payload []byte) string { var out pogo.BattleStateOutProto - if err := proto.Unmarshal(payload, &out); err != nil { + if err := unmarshalClientProto(payload, &out); err != nil { return "failed to parse BattleStateOutProto" } return decoder.UpdateIncidentLineupFromBattleState(ctx, dbDetails, fortId, incidentId, &out) diff --git a/decode_push_gateway.go b/decode_push_gateway.go index ca8bea04..90b27c5e 100644 --- a/decode_push_gateway.go +++ b/decode_push_gateway.go @@ -7,7 +7,6 @@ import ( "golbat/pogo" log "github.com/sirupsen/logrus" - "google.golang.org/protobuf/proto" ) // decodePushGateway classifies a push-gateway message by message_type, unmarshals @@ -22,7 +21,7 @@ func decodePushGateway(ctx context.Context, messageType string, payload []byte) } var msg pogo.PushGatewayMessage - if err := proto.Unmarshal(payload, &msg); err != nil { + if err := unmarshalClientProto(payload, &msg); err != nil { log.Warnf("PushGateway: failed to parse %s: %v", messageType, err) return } diff --git a/docs/decode-performance-findings.md b/docs/decode-performance-findings.md new file mode 100644 index 00000000..8a2cf974 --- /dev/null +++ b/docs/decode-performance-findings.md @@ -0,0 +1,113 @@ +# Decode-path performance findings + +Consolidated learnings from the proto-decode optimization exploration +(PRs #378 hyperpb harness, #381 hyperpb migration). **hyperpb itself is being +left behind** — v0.1.x was fast but too immature (three silent correctness +bugs found in the first week of testing) to justify the maintenance for the +~10% CPU it bought over the base branch. This branch implements the +engine-independent wins that survive that decision. + +All numbers are from the `protobench` harness (standalone decode-at-volume +rig) against a size-stratified, frozen corpus of real production payloads. +Harness figures are **relative**; absolute wins were confirmed on a prod +canary. GMO = GetMapObjects, the dominant method. + +## Implemented on this branch + +### 1. `DiscardUnknown` on client-proto unmarshals (the universal win) + +**Measured: +3.7% decode rate, −5.5% allocated objects, −3% bytes.** Production +payloads carry fields newer than our `vbase` schema vintage; protobuf-go +otherwise retains them in a per-message unknown-fields buffer. Golbat **never +re-serializes** client protos, so discarding is free. + +Applied via a shared `unmarshalClientProto` helper at every inbound client-proto +decode site (`decode.go`, `decode_nebula.go`, `decode_push_gateway.go`). +**This wins on both ingest paths** — HTTP `/raw` and gRPC — because both funnel +their per-packet decode through the same `decode()` unmarshal calls. This is the +win that reaches everyone. + +### 2. Ingest buffer pooling — HTTP path only + +**Measured: −14% bytes/decode on the HTTP path**, GC share 5.4%→4.9%. The HTTP +`/raw` handler base64-decodes each payload into a fresh `[]byte` that lives for +one `decode()` call; a `sync.Pool` (`raw_bufpool.go`) recycles those buffers. +Verified 0 allocs/op for the decode; safe because standard protobuf-go copies +bytes out during Unmarshal, so the buffer is free the moment `decode()` returns. + +**Scope caveat (important):** this only helps the HTTP path. **Most deployments +use the gRPC ingest path**, where payloads arrive as raw `[]byte` fields of the +gRPC request — there is no base64 buffer of ours to pool. Those payload bytes +are allocated by protobuf-go unmarshaling the `RawProtoRequest`, and grpc-go +(v1.81) already pools its transport receive buffers by default. So there is no +equivalent buffer-pooling change to make for gRPC users — the framework already +does it, and their decode-allocation win comes from `DiscardUnknown` above. + +### 3. `make pgo-capture` / `pgo-status` — refresh tooling for compiler PGO + +The base branch committed a `default.pgo` (Go compiler PGO — auto-applied to +every build, ~profile-guided inlining/devirtualization) but *not* the tooling +to refresh it, leaving the profile un-maintainable when game updates shift hot +paths. This branch ports the targets and extends them to **auto-detect the port +and `api_secret` from `config.toml`** so a local operator can just run +`make pgo-capture` (env vars still override; `make pgo-config` shows what it +will use). Requires `profile_routes = true` in config to expose `/debug/pprof`. + +## Already on the base branch (don't redo) + +- **Go compiler PGO** — `default.pgo` committed and auto-applied (this branch + adds the refresh tooling above). +- **Runtime GC tuning** — `gogc_percent` / `go_mem_limit_mib` config. Measured: + on a large live heap (Golbat's caches/R-trees), **GOGC 300–400 reclaims 10%+ + of GC CPU**, trading heap headroom for fewer collections. Set + `go_mem_limit_mib` below available RAM as a backstop when raising GOGC. + +## Reusable infrastructure (parked on PR #378/#381, resurrectable) + +- **`protobench` harness** — standalone decode-at-volume rig reporting + decodes/s, B/op, allocs/op, GC CPU share, pause distribution, with a + pointer-dense heap ballast to reproduce Golbat's big-heap regime. A/B any + future engine/knob here first. +- **Payload capture hook** — debug-gated, size-stratified capture of real raw + payloads for building corpora. +- **Shadow verification pattern** — decode a sampled fraction of live packets + with *both* the old and new path and compare a field-level digest. Caught a + silent data-corruption bug in a third-party parser at 1% sampling within + seconds. Reusable for any risky decode change — a proto version bump, an + opaque-API trial, vtprotobuf, etc. +- **Getter-style call sites** — the ~31-file conversion to `GetX()` access + (required by the Opaque API; the prerequisite for any future opaque attempt). + Retargeting from the hyperpb `pogoshim` wrappers to native `pogo` getters is + mechanical (≈760 scalar calls transfer unchanged; ≈150 repeated/presence + idioms need translation). See the "park the rest" analysis on PR #381. + +## Documented dead-ends (do not re-explore without new evidence) + +- **Opaque API + lazy decoding** — measured to *hurt* on GMO-shaped data (many + tiny unread subtrees; lazy bookkeeping outweighs deferral). Opaque without + lazy is roughly neutral. The presence-bitfield win doesn't apply — Golbat's + hot messages are pure proto3 implicit-presence with no pointer-boxed scalars. +- **hyperpb** — genuinely fast (arena decode; parse ~3.5× cheaper, decode + allocation 47%→19% of total, GC mark 26.7%→22.3% on prod, all complementary + to the base's cache work), but v0.1.x maturity was the dealbreaker (Recompile + corruption #39, 64-bit-varint-oneof data loss #42, UTF-8 validation #41, all + in one week). If allocation reduction is ever wanted without hyperpb's exotic + dependency, **`vtprotobuf` + message pooling** was the measured runner-up + (55.7k decodes/s, −68% bytes vs std) with a 1.0-stable generator, at the cost + of more per-decode object churn and pooled-graph lifecycle discipline. + +## Measurement methodology worth reusing + +- **Size-stratified corpus over 24h+** — complex shapes (raid-heavy GMOs, + invasions, multi-reward quests) live in the size tail; a small uniform sample + misses them. +- **Freeze the corpus** before an A/B — it grows during capture and shifts + absolute numbers between runs. +- **Large heap ballast (~2 GB) to reveal GC wins** — on an idle box the + decode-allocation → GC relationship is invisible; it only shows when mark + cost scales against a big resident heap (which prod has). +- **Load-normalize prod profiles** — the raw-ingest handler's CPU share is a + usable load proxy (cost ÷ ingest-share beats raw percentages). +- **Relative from the harness, absolute from the canary** — the harness proves + methodology gain; prod GC shares the heap with caches, R-trees, and + write-behind queues, so absolute numbers only come from a canary. diff --git a/raw_bufpool.go b/raw_bufpool.go new file mode 100644 index 00000000..d717cc67 --- /dev/null +++ b/raw_bufpool.go @@ -0,0 +1,45 @@ +package main + +import ( + b64 "encoding/base64" + "sync" +) + +// payloadBufPool recycles the decoded-payload byte buffers on the raw ingest +// hot path. The raw handler base64-decodes every packet's payload into a fresh +// []byte that lives only for the duration of one decode() call; pooling those +// buffers removes a per-packet allocation the size of the whole payload +// (15–300 KB for a GMO). +// +// Safety: this is sound only because standard protobuf-go Unmarshal COPIES the +// bytes it retains (strings/bytes fields are copied into the message), so a +// payload buffer is no longer referenced once decode()'s unmarshal returns. +// (This is exactly why the same pooling is unsafe under a zero-copy arena +// parser, whose message strings would alias the pooled buffer.) +var payloadBufPool = sync.Pool{New: func() any { b := make([]byte, 0, 4096); return &b }} + +// decodeBase64Pooled base64-decodes s into a buffer borrowed from +// payloadBufPool and returns the decoded slice plus a release func. The caller +// MUST call release exactly once, and only after every reader of the returned +// slice is done (i.e. after decode() returns). On empty input or a decode +// error it returns (nil, no-op release) — matching the previous +// DecodeString-with-ignored-error behavior on the raw path. +func decodeBase64Pooled(s string) ([]byte, func()) { + if s == "" { + return nil, func() {} + } + bp := payloadBufPool.Get().(*[]byte) + need := b64.StdEncoding.DecodedLen(len(s)) + if cap(*bp) < need { + *bp = make([]byte, need) + } + // []byte(s) here does not escape the Decode call, so the compiler elides + // its allocation — verified 0 allocs/op. The only allocation is the pooled + // output buffer above, amortized away across packets. + n, err := b64.StdEncoding.Decode((*bp)[:need], []byte(s)) + if err != nil { + payloadBufPool.Put(bp) + return nil, func() {} + } + return (*bp)[:n], func() { payloadBufPool.Put(bp) } +} diff --git a/raw_bufpool_test.go b/raw_bufpool_test.go new file mode 100644 index 00000000..4eb2801d --- /dev/null +++ b/raw_bufpool_test.go @@ -0,0 +1,92 @@ +package main + +import ( + "bytes" + b64 "encoding/base64" + "strings" + "sync" + "testing" + + "google.golang.org/protobuf/encoding/protowire" + "google.golang.org/protobuf/proto" + + "golbat/pogo" +) + +func TestDecodeBase64Pooled(t *testing.T) { + // Empty input -> nil, no-op release. + if b, rel := decodeBase64Pooled(""); b != nil || rel == nil { + t.Fatalf("empty: got %v", b) + } + + // Round-trip: various sizes, including bigger-than-initial-pool-cap. + for _, raw := range []string{"x", "hello world", strings.Repeat("abc", 5000)} { + enc := b64.StdEncoding.EncodeToString([]byte(raw)) + got, release := decodeBase64Pooled(enc) + if string(got) != raw { + t.Fatalf("round-trip mismatch: got %q want %q", got, raw) + } + release() + } + + // Reuse: a small decode after a big one must not read stale tail bytes. + big := b64.StdEncoding.EncodeToString(bytes.Repeat([]byte{0xAB}, 8000)) + _, relBig := decodeBase64Pooled(big) + relBig() // return the (grown) buffer to the pool + smallRaw := []byte("tiny") + small := b64.StdEncoding.EncodeToString(smallRaw) + got, rel := decodeBase64Pooled(small) + if !bytes.Equal(got, smallRaw) { + t.Fatalf("reuse tail-bleed: got %q want %q", got, smallRaw) + } + rel() +} + +func TestDecodeBase64PooledConcurrent(t *testing.T) { + // Race-detector target: many goroutines borrowing/returning concurrently, + // each verifying its own payload round-trips (no cross-contamination). + var wg sync.WaitGroup + for g := 0; g < 16; g++ { + wg.Add(1) + go func(g int) { + defer wg.Done() + raw := strings.Repeat(string(rune('a'+g)), 100+g*37) + enc := b64.StdEncoding.EncodeToString([]byte(raw)) + for i := 0; i < 200; i++ { + got, release := decodeBase64Pooled(enc) + if string(got) != raw { + t.Errorf("g%d: mismatch", g) + release() + return + } + release() + } + }(g) + } + wg.Wait() +} + +func TestUnmarshalClientProtoDiscardsUnknownKeepsKnown(t *testing.T) { + // A real fort proto with a known field. + src := &pogo.FortDetailsOutProto{Id: "FORT_1"} + raw, err := proto.Marshal(src) + if err != nil { + t.Fatal(err) + } + // Append an unknown field (field 99999, varint) on the wire. + withUnknown := protowire.AppendTag(append([]byte{}, raw...), 99999, protowire.VarintType) + withUnknown = protowire.AppendVarint(withUnknown, 42) + + var got pogo.FortDetailsOutProto + if err := unmarshalClientProto(withUnknown, &got); err != nil { + t.Fatalf("unmarshal: %v", err) + } + // Known field survives. + if got.GetId() != "FORT_1" { + t.Fatalf("known field lost: %q", got.GetId()) + } + // Unknown field discarded (not retained in the message). + if n := len(got.ProtoReflect().GetUnknown()); n != 0 { + t.Fatalf("expected unknown fields discarded, got %d bytes retained", n) + } +} diff --git a/routes.go b/routes.go index c8410133..14ef29c3 100644 --- a/routes.go +++ b/routes.go @@ -382,15 +382,22 @@ func Raw(c *gin.Context) { ScanContext: scanContext, TimestampMs: dataReceivedTimestamp, } - protoData.Data, _ = b64.StdEncoding.DecodeString(payload) - if request != "" { - protoData.Request, _ = b64.StdEncoding.DecodeString(request) - } + // Pool the decoded-payload buffers. This is the HTTP /raw path; + // the payload lives only for this one decode() call and standard + // protobuf-go copies bytes out during Unmarshal, so the buffers are + // safe to recycle the moment decode() returns. (The gRPC ingest + // path has no base64 buffer of ours to pool — its payloads arrive + // as raw bytes and grpc-go already pools its transport buffers.) + var releaseData, releaseReq func() + protoData.Data, releaseData = decodeBase64Pooled(payload) + protoData.Request, releaseReq = decodeBase64Pooled(request) // provide independent cancellation contexts for each proto decode ctx, cancel := context.WithTimeout(context.Background(), timeout) decode(ctx, method, &protoData) cancel() + releaseData() + releaseReq() } for _, entry := range nebulaItems {