Parser performance improvements, round 2#181
Open
jcoene wants to merge 11 commits into
Open
Conversation
Fresh CPU/alloc profiles on master show a new profile shape after the Phase 1-4 work: unaligned readBytes in onCDemoPacket (13.5% of allocs), readBitsAsBytes growth (11.6%), protobuf envelope decode (~30% of allocs, 32.7% of alloc space in consumeBytes), fieldState.clone (19% of space), and the p.entities map (~9% CPU). Baseline (BenchmarkMatch8552595443, benchtime=10x, count=10, M4 Pro): sec/op 876.7m +/- 8% B/op 389.4Mi +/- 0% allocs 4.478M +/- 0% Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Allocate the exact (n+7)/8 result up front instead of growing a cap-0 slice byte-at-a-time, and fill via readBits(32) words with a byte/bit tail. String-table values retain the slice, so one fresh allocation per value is the floor; this removes the append-growth allocations on top. benchstat vs P5.0 (BenchmarkMatch8552595443, 10x, count=10): sec/op 876.7m +/- 8% -> 756.3m +/- 1% -13.74% (p=0.000)* B/op 389.4Mi +/- 0% -> 384.4Mi +/- 0% -1.29% (p=0.000) allocs 4.478M +/- 0% -> 4.181M +/- 0% -6.64% (p=0.000) * P5.0 sec/op was thermally inflated (+/-8%); allocs/B are the reliable signal for this change. go test ./... PASS (identical goldens) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The onCDemoPacket inner stream is bit-shifted after the leading 6-bit readUBitVar, so nearly every embedded message body hit the unaligned readBytes slow path: a fresh zeroed make([]byte, n) filled one readBits(8) at a time (422K allocs, 49.5MB, plus most of the 8% memclrNoHeapPointers CPU). New reader.readBytesInto(dst) copies unaligned data a 32-bit word at a time (readBytes' slow path routes through it), and onCDemoPacket carves message buffers from a single parser-level arena sized to the packet and reused across packets. Buffer lifetime matches pendingMsgBuf: dispatch only, and the protobuf unmarshal copies what it keeps. benchstat vs P5.1 (BenchmarkMatch8552595443, 10x, count=10): sec/op 756.3m +/- 1% -> 676.7m +/- 1% -10.52% (p=0.000) B/op 384.4Mi +/- 0% -> 336.3Mi +/- 0% -12.52% (p=0.000) allocs 4.181M +/- 0% -> 3.757M +/- 0% -10.13% (p=0.000) go test ./... PASS (identical goldens) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entity indices are 14-bit by the handle encoding, and the PacketEntities update loop did 1-3 map lookups per entity update (~9% CPU in mapaccess1_fast32). Replace map[int32]*Entity with a dense []*Entity of 1<<indexBits slots (128 KB per parser). FindEntity bounds-checks and keeps its signature. FilterEntity skips nil slots, which also fixes a latent hazard: deletion stored nil into the map, so FilterEntity could previously pass nil entities to callbacks. Iteration is now deterministic by index instead of map-random. benchstat vs P5.2 (BenchmarkMatch8552595443, 10x, count=10): sec/op 676.7m +/- 1% -> 624.8m +/- 2% -7.66% (p=0.000) B/op 336.3Mi flat, allocs 3.757M flat (CPU-only change) go test ./... PASS (identical goldens) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
parseStringTable allocated a &stringTableItem per item plus cap-0 items append growth, shifted the whole 32-entry key-history window per item once full, and the UpdateStringTable apply loop did up to four map lookups per item. Preallocate items and back them with a single []stringTableItem slab (cap bounded at 4096 so a corrupt numUpdates cannot OOM; pointer identity survives slab regrowth because items are only touched through the taken pointers). The key history becomes a fixed ring buffer with histCount-based indexing that reproduces the shift-window semantics exactly. The apply loop hoists to a single map lookup. benchstat vs P5.3 (BenchmarkMatch8552595443, 10x, count=10): sec/op 624.8m +/- 2% -> 618.7m +/- 4% -0.98% (p=0.043) B/op 336.3Mi +/- 0% -> 335.0Mi +/- 0% -0.38% (p=0.000) allocs 3.757M +/- 0% -> 3.597M +/- 0% -4.26% (p=0.000) go test ./... PASS (identical goldens) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Unaligned readLeUint32/64 allocated through the readBytes slow path (24K objects via fixed64Decoder and friends). Read them straight from the bit accumulator instead: bits are consumed LSB-first from little-endian bytes, so readBits(32) equals the LE decode of the next four stream bytes; readLeUint64 uses two accumulator words (readBits is capped at 32 bits per call). Aligned paths keep the zero-copy readBytes fast path and the exact pos contract the reader tests assert. readString starts at cap 32 instead of growing a cap-0 buffer per byte. benchstat vs P5.4 (BenchmarkMatch8552595443, 10x, count=10): sec/op 618.7m +/- 4% -> 618.8m +/- 2% ~ (p=0.481) B/op 335.0Mi +/- 0% -> 334.5Mi +/- 0% -0.16% (p=0.000) allocs 3.597M +/- 0% -> 3.540M +/- 0% -1.59% (p=0.000) go test ./... PASS (identical goldens) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every CDemoPacket / CSVCMsg_PacketEntities / CSVCMsg_UpdateStringTable / CNETMsg_Tick went through the reflective protobuf unmarshal: reflect.New per message, a pointer alloc per scalar field, and - 32.7% of all allocated bytes - a fresh copy of every bytes payload. The replay's data was effectively copied twice through the proto layer. envelope_fast.go decodes these four envelopes by hand with protowire, aliasing the payload instead of copying, and calls scalar-arg core methods split out of the internal handlers. Gating: NewParser registers exactly one internal handler per list before returning, so len(list) == 1 means no user callbacks are registered; any user registration reverts that type to the full protobuf path, where the user-visible message owns its own copies as before. Aliasing lifetimes: entity_data/string_data alias the packet arena (stable through the dispatch loop, consumed synchronously; string-table values are copied out by readBitsAsBytes). CDemoPacket.data aliases the outer-message buffer (stable until the next readOuterMessage; processDemoPacket copies bodies into the arena before dispatch). benchstat vs P5.5 (BenchmarkMatch8552595443, 10x, count=10): sec/op 618.8m +/- 2% -> 580.5m +/- 0% -6.18% (p=0.000) B/op 334.5Mi +/- 0% -> 192.7Mi +/- 0% -42.39% (p=0.000) allocs 3.540M +/- 0% -> 2.615M +/- 0% -26.13% (p=0.000) go test ./... PASS (identical goldens; the goldens exercise the fast path since tests register OnEntity/OnGameEvent, not raw message callbacks) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
clone() allocated a *fieldState plus a []cell array per nested state on every entity create (210K objects on the profile). A counting pre-pass now sizes two slabs ([]fieldState, []cell) and the copy carves from them; vector/blob leaves keep their individual deep copies (P4.1 semantics). Cell slices are cap-limited so a later set() grow allocates a fresh array instead of stomping a neighbouring state's arena region; each entity's slabs are private and die with the entity. benchstat vs P5.6 (BenchmarkMatch8552595443, 10x, count=10): sec/op 580.5m +/- 0% -> 574.5m +/- 1% -1.03% (p=0.001) B/op 192.7Mi +/- 0% -> 194.4Mi +/- 0% +0.92% (p=0.000) allocs 2.615M +/- 0% -> 2.508M +/- 0% -4.10% (p=0.000) The small B/op cost is slab retention overhead, traded for object count (which drives GC scan work). go test ./... PASS (identical goldens) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every CMsgSource1LegacyGameEvent was fully unmarshaled (message plus a reflect.New per key) before the internal handler checked whether any handler is registered for the event name and usually returned nil. skipGameEvent peeks the eventid varint (field 2) with protowire; a known event with no registered handlers skips the unmarshal entirely. Unknown ids and absent fields fall back to the full path, preserving its error behaviour exactly. benchstat vs P5.7 (BenchmarkMatch8552595443, 10x, count=10): sec/op 574.5m +/- 1% -> 572.3m +/- 1% ~ (p=0.052) B/op 194.4Mi +/- 0% -> 193.8Mi +/- 0% -0.32% (p=0.000) allocs 2.508M +/- 0% -> 2.492M +/- 0% -0.63% (p=0.000) Modest on this bench because it registers a combat-log handler and combat-log events dominate the event stream; the skip is worth more to consumers that register fewer (or no) game-event handlers. go test ./... PASS (identical goldens) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
P5.9 (prefix cache for the set/decoder walk across sorted field paths) measured statistically flat on sec/op (p=0.481 vs a verified-clean P5.8 baseline) with identical allocs/B - most field paths in this workload are depth-1, where the cache does nothing. Reverted per the keep-only-if-confirmed rule. P5.10 (widen cell to 32 bytes with inline vec2/vec3 float lanes) cut allocs -17.9% but cost +12.0% B/op and +1.87% sec/op (p=0.000): the wider cell inflates every state array and clone slab, and the extra copy bytes outweigh the GC win, consistent with P3.1's earlier 32-byte-cell result. Reverted. Phase 5 final (verified-clean re-measure at P5.8): sec/op 573.9m +/- 1% (vs ~750-756m credible pre-phase level: ~-24%) B/op 193.8Mi (-50.2% vs P5.0) allocs 2.492M (-44.3% vs P5.0) Cumulative vs the original pre-Phase-1 baseline: sec -62%, B/op -75%, allocs -88%. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Phase 5 plan and results are preserved in the commit messages and kept locally; the tracking document does not belong in the repo. 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
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.
A second profile-driven performance round following #180. Fresh CPU/alloc profiles on master showed a new profile shape after the last round; this branch works through the new dominators one benchstat'd commit at a time, under the same rules as #180: no public API change, full golden-test suite green at every commit, one optimization per commit with its benchstat table in the commit message.
Results (BenchmarkMatch8552595443, M4 Pro, benchtime=10x, count=10)
Cumulative vs the pre-#180 baseline: sec −62%, B/op −75%, allocs −88%.
Changes
envelope_fast.godecodes the CDemoPacket / CSVCMsg_PacketEntities / CSVCMsg_UpdateStringTable / CNETMsg_Tick envelopes withprotowire, aliasing payloads instead of copying them through the reflective proto unmarshal. Only taken when a message type has exactly the internal handler registered (len(list) == 1); any user callback registration reverts that type to the full protobuf path, so user-visible messages own their copies exactly as before.nilinto the map andFilterEntitycould pass nil entities to callbacks.readBitsAsBytesexact prealloc + word fill (allocs −6.6%)readLeUint32/64read from the bit accumulator instead of allocating;readStringpreallocates.Two further ideas were implemented, measured, and rejected (a depth-1 prefix cache for the field-state walk: statistically flat; inline vec2/vec3 cell lanes: allocs −18% but B/op +12% and sec +1.9%) — details in the P5.9/P5.10 commit message.
Verification
go test ./...green at every commit with identical golden values (~48 replay scenarios).🤖 Generated with Claude Code