Skip to content

Parser performance & correctness improvements#180

Merged
jcoene merged 39 commits into
masterfrom
jcoene/goal-perf
Jun 22, 2026
Merged

Parser performance & correctness improvements#180
jcoene merged 39 commits into
masterfrom
jcoene/goal-perf

Conversation

@jcoene

@jcoene jcoene commented Jun 22, 2026

Copy link
Copy Markdown
Member

Summary

Overhauls the Source 2 entity/replay decode hot path for performance and closes
several correctness gaps. On a representative recent replay it is ~2× faster,
uses ~half the memory, and makes ~78% fewer allocations — with the full golden
test suite green at every commit and the public API unchanged except for a few
deliberate, documented value-representation fixes (see Behavior changes below).

Developed as one focused change per commit, each with its own benchstat table
in the message, so it can be reviewed incrementally.

Benchmarks

BenchmarkMatch8552595443 (build 6601, ~70 MB, parsed in-memory; M4 Pro,
-benchmem -benchtime=10x -count=10):

metric before after change
sec/op 1.523 s ~0.74 s −51%
B/op 791.5 MiB 389.4 MiB −51%
allocs/op 20.75 M 4.48 M −78%

(Streaming callers — NewStreamParser on an *os.File — get an additional
~6% from buffered reads; the table above is the in-memory parse path.)

Performance changes

  • Reusable field-path buffer — eliminates the single largest allocation
    source (a freshly grown []*fieldPath per entity per tick, ~56–60% of all
    allocations); field paths are now value types in a reused buffer.
  • Word-at-a-time bit reader — refills the accumulator a 64-bit word at a
    time instead of byte-by-byte, with a realign() so byte reads stay zero-copy.
  • Flattened Huffman tree + 8-bit op lookup table — replaces the per-bit
    interface-dispatch walk; most field-path ops resolve in a single table index.
  • Decode class baseline once, clone per entity — instead of re-decoding the
    baseline bitstream on every entity creation.
  • Typed entity state — stores decoded values in a 24-byte tagged-union cell
    (scalars inline, no per-field interface{} boxing on the write path); values
    box lazily on Get.
  • Plus smaller wins: value-slice/buffer reuse for pending messages, entity
    tuples, and snappy scratch; outerMessage by value; modifier/debug fast-outs;
    bufio for non-byte-reader streams.

Correctness fixes

  • String-table additive index — the non-increment delta was decoded as an
    absolute index, mis-indexing delta-updated tables (e.g. ActiveModifiers);
    now additive, matching manta's own entity decoder.
  • Fail-loud string-table parsing; outer-message size bound; truncated bitstream
    now errors cleanly instead of looping.
  • Combat-log type resolved via the event descriptor instead of a hardcoded key.
  • Forward-compat decoders: CUtlBinaryBlock, Quaternion, QAngle
    precise/noscale/pitch-yaw raw-float cases, full 64-bit int64.
  • getEventKey off-by-one; skip empty/deleted modifier entries; mana/runetime
    field-patch sentinel guards.

⚠️ Behavior changes — please review

A few fields now decode to a more correct representation. These change the
dynamic type/value returned by Entity.Get for those specific fields, so a
caller doing a concrete type assertion on them may need an update. They're locked
by tests (TestDecoderRepresentations / TestValueChangingDecoderWiring):

field before after
int64 int32 (truncated >32 bits) int64 (full width)
HeroID_t uint32 int32 (signed)
HSequence uint32 int32, value − 1 (−1 = none)
BloodType uint64 uint64 (encoding fixed-8; value unchanged)

These ship in v1.5.0; pin to an earlier release if you depend on the previous
representation of these fields.

Robustness fixes

  • Baseline clone() deep-copies mutable leaf slices ([]float32/[]byte) so a
    caller mutating a Get/Map result can't corrupt the baseline template or
    sibling entities.
  • Reused per-packet buffers are cleared on all dispatch paths (including error),
    so they don't retain deleted entities or packet data.

Testing

  • Full golden suite (~48 real replays asserting exact entity-event counts,
    combat-log counts, hero mana floats, and specific entity field values) stays
    green with identical assertions across the whole branch.
  • New tests: decoder representation/wiring locks, Huffman flat≡tree and
    lookup≡walk equivalence, truncated field-path stream handling, and a
    zero-copy readBytes invariant.

Thanks to clarity, which served as a
reference for some of these improvements.

🤖 Generated with Claude Code

jcoene and others added 30 commits June 19, 2026 09:48
@jcoene @claude
docs: parser improvement plan
048d252 
Ordered correctness+performance plan for the Source 2 replay parser, derived
from a subsystem review against the Clarity (Java) reference parser. Executed
one goal per commit on jcoene/goal-perf.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(bench): in-memory benchmark rig + recent canonical replay
e3870da 
The benchmark streamed from *os.File inside the timed loop, so ~80% of CPU
was per-byte read syscalls, masking parser changes. Read the replay into
memory once, ResetTimer, and parse via in-memory NewParser in the loop. Add
BenchmarkMatch8552595443 (build 6601, more representative of current replays)
as the canonical bench and repoint the Makefile profile targets to it.

No parser code changed; go test ./... green (identical golden values).

Baseline (8552595443, M4 Pro, -benchmem -benchtime=10x -count=10):
  sec/op  1.523 ± 1%  |  B/op  791.5Mi  |  allocs/op  20.75M
Alloc profile reconfirms readFieldPaths dominates at 56.6% of allocations.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(packet): reuse pendingMessage value-slice, stable sort
f968107 
onCDemoPacket allocated a *pendingMessage per embedded message (~0.7M
allocs/parse) and sorted via sort.Sort's reflection path. Switch
pendingMessages to a value slice reusing a parser-level buffer, give
priority() a value receiver, and use typed sort.Stable (deterministic
file-order within each priority bucket, no reflection allocs). The priority
reordering is preserved; onCDemoPacket is non-re-entrant so the reused
backing array is safe.

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P0:
  sec/op    1.523 -> 1.514     -0.62% (p=0.000)
  B/op      791.5Mi -> 763.8Mi -3.51%
  allocs/op 20.75M -> 20.06M   -3.33%

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(parser): return outerMessage by value
638122a 
readOuterMessage allocated &outerMessage{} per outer message and returned it
as a pointer, forcing a heap escape. Its only caller (Start) consumes the
message immediately and never retains it, so return it by value to keep it on
the stack.

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P1.1:
  allocs/op 20.06M -> 20.03M  -0.19% (-30K)
  B/op      763.8Mi -> 762.6Mi -0.15%
  sec/op    1.514 -> 1.519     +0.32% (within run-to-run noise)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(parser): reuse snappy scratch buffer for outer messages
1b00c34 
snappy.Decode(nil, ...) allocated a fresh output buffer for every compressed
outer message. Pass a reused parser-level scratch buffer as the destination;
snappy reuses it when large enough, amortizing decompression to ~the largest
compressed message. Safe because the decoded buffer is consumed within this
message's dispatch and never retained across outer messages.

go test ./... green, including the full 48-replay golden suite (the gate for
this reuse-aliasing change).

benchstat (8552595443, 10x x10) vs P1.2:
  B/op      762.6Mi -> 700.6Mi  -8.14% (p=0.000)
  sec/op    1.519 -> 1.511       -0.49%
  allocs/op 20.03M -> 20.00M     -0.12%

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(modifier): skip emission entirely when no handler registered
3116d5b 
emitModifierTableEvents allocated a proto message and unmarshalled every
ActiveModifiers entry on every create/update even when no OnModifierTableEntry
handler was registered (the common case). Early-return when there are no
handlers. Behavior is unchanged for consumers that register one.

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P1.3:
  allocs/op 20.00M -> 18.74M  -6.32% (p=0.000, -1.26M)
  B/op      700.6Mi -> 647.9Mi -7.51%
  sec/op    1.511 -> 1.479      -2.15%

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(entity): reuse per-packet tuple buffer and entity-data reader
a29739f 
onCSVCMsg_PacketEntities allocated a new reader and a fresh []tuple (sized to
the update count) on every PacketEntities message. Hoist the tuple type to a
package-level entityOpTuple, add reader.reset, and reuse parser-level buffers
for both. The handler still sees identical entities/ops in identical order
(append order preserved), so expectEntityEvents is unchanged. Not re-entrant,
so reuse is safe.

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P1.4:
  B/op      647.9Mi -> 571.0Mi  -11.87% (p=0.000)
  allocs/op 18.74M -> 18.66M     -0.40%
  sec/op    1.479 -> 1.494       +1.03% (run variance)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(entity): share field-path name caches on the class
729d63c 
name -> field-path resolution depends only on the class serializer, yet each
entity allocated its own fpCache/fpNoop maps and recomputed the identical
mapping. Move both caches to *class so a name is resolved once per class
instead of once per entity, and drop two map allocations per entity creation.
Parsing is single-goroutine, so plain maps are safe.

go test ./... green (identical golden values; expectPlayer6Name /
expectHeroEntityName exercise Get-by-name across many entities).

benchstat (8552595443, 10x x10) vs P1.5:
  allocs/op 18.66M -> 18.63M  -0.16% (-30K)
  B/op      571.0Mi -> 569.5Mi -0.27%
  sec/op    ~ (p=0.190)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(reader): hoist debug verbosity check out of the field loop
16bc0aa 
readFields called v(6) twice per field path in the hot loop (millions of
times per parse). Evaluate it once per readFields call into a local. readFields
is invoked fresh per entity update, so a mid-parse debug-level change still
takes effect on the next call.

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P1.6:
  sec/op    1.509 -> 1.476  -2.18% (p=0.002)
  allocs/op unchanged (CPU-only)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(stream): buffer non-byte-reader inputs (e.g. *os.File)
a354e5f 
The outer loop reads 3 varints + payload per message, one byte at a time;
against a raw *os.File that is one read syscall per byte (~80% of CPU on the
streaming path). Wrap the reader in a bufio.Reader when it does not already
implement io.ByteReader, so in-memory readers (*bytes.Reader, used by
NewParser) are left unwrapped and incur no extra copy.

go test ./... green (identical golden values).

Canonical in-memory bench flat (bytes.Reader unwrapped). Streaming
NewStreamParser(os.File) path, measured separately:
  sec/op  1.612 -> 1.507  -6.47% (p=0.002)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(fieldpath): reuse a value-type field-path buffer (eliminate 56%-…
769aad1 
…alloc slice)

readFieldPaths allocated a fresh []*fieldPath per entity update and grew it via
append(fp.copy()), which the profile attributed to ~56% of all allocations.
Make fieldPath a value type with a fixed [7]int array and decode the op stream
into a reused parser-level []fieldPath buffer; readFields consumes it in place.
The two-pass order (all ops, then all values) and the field-path values are
unchanged, so decoding is identical. The op accumulator is borrowed from the
existing sync.Pool so taking its address for the op functions does not force a
per-call heap escape (a value-typed local would, regressing B/op).

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P1.8:
  allocs/op 18.63M -> 10.65M  -42.86% (p=0.000, -7.98M)
  B/op      569.5Mi -> 398.6Mi -30.00%
  sec/op    1.483 -> 1.198      -19.22%

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(reader): refill the bit accumulator a word at a time
54c074f 
readBits refilled the accumulator one byte at a time; it is the hottest
function in the parser. Load a full 64-bit little-endian word when >= 8 bytes
remain, masking it to the whole bytes of headroom before merging so no stale
partial-byte bits are left above bitCount, and fall back to byte-at-a-time only
for the buffer tail. Every read is <= 32 bits (now asserted for quantized
floats), so one refill always suffices.

Because the refill reads bytes ahead into the accumulator, add realign() so the
byte-aligned readBytes/readByte fast path rewinds those buffered bytes and stays
zero-copy (aliasing the underlying buffer) at any byte boundary, not just when
bitCount==0. This makes byte reads zero-copy more often than before.

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P1.9:
  sec/op    1.198 -> 1.058  -11.64% (p=0.000)
  B/op      flat   allocs/op flat

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
docs: skip P1.11 (varints-from-accumulator breaks reader pos contract)
142f22a 
Routing readByte/readLeUintX through readBits would consume into the word
reader's read-ahead accumulator, leaving r.pos ahead of the logical position.
The reader contract (r.pos, remBytes) and the reader unit tests rely on r.pos
being the logical position. The marginal gain wasn't worth it; P1.10's realign
already keeps these byte reads fast and zero-copy. Reverted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(fieldpath): flatten the field-path huffman tree into int arrays
59f33d6 
readFieldPaths walked an interface-based huffman tree one bit at a time, with
~2-3 interface-method dispatches plus a pointer chase per bit. Flatten the tree
(derived from manta's own huffTree, so codes are identical) into int32 child
arrays where a negative entry -(op+1) marks a leaf, and walk those instead.
Adds TestHuffmanFlatMatchesTree to lock the flat arrays to the interface tree.

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P1.10:
  sec/op    1.058 -> 0.960  -9.28% (p=0.000)
  B/op / allocs/op unchanged

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(fieldpath): resolve field-path ops via an 8-bit lookup table
b11b3c2 
Replace the per-bit flat-tree walk with a 256-entry lookup that resolves most
ops (codes <= 8 bits) in a single index, falling back to the flat-tree walk for
longer codes. Add reader.peekBits (zero-pads, never over-reads past the buffer)
and skipBits so the lookup can inspect a fixed window even at the final
FieldPathEncodeFinish near the end of the stream. The table is built once from
manta's immutable huffTree (swapNodes/addNode are never called).
TestHuffmanLookupMatchesWalk checks the lookup matches the walk (op + bits
consumed) over 5000 random streams.

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P1.12:
  sec/op    0.960 -> 0.796  -17.10% (p=0.000)
  B/op / allocs/op unchanged

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
perf(entity): decode each class baseline once and clone per entity
0673269 
Every entity creation re-decoded the raw class baseline bitstream from scratch
(~4x the cost of a delta update). Decode each class baseline once into a
template fieldState and clone it for new entities instead. clone() shares
immutable leaf values and deep-copies nested fieldStates, so it is cheaper than
re-decoding (no huffman/bit decode, no re-boxing) while remaining independent
per entity. Templates are invalidated when the instancebaseline string table
updates, so a changed baseline is re-decoded.

go test ./... green (identical golden values; the cloned+overlaid state is
value-identical to a fresh decode).

benchstat (8552595443, 10x x10) vs P1.13:
  sec/op    0.796 -> 0.766  -3.71% (p=0.000)
  B/op      398.6Mi -> 378.1Mi -5.14%
  allocs/op 10.65M -> 10.26M   -3.61%

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
test(reader): guard zero-copy readBytes invariant
a0eeecf 
Add TestReaderReadBytesZeroCopy asserting a byte-aligned readBytes returns a
slice aliasing the underlying buffer (not a copy), both when bitCount==0 and
through the word reader's realign path. CDemoPacket pendingMessage parsing and
sendtables rely on this aliasing; the test prevents a future reader change from
silently turning it into a copy.

go test ./... green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(stringtable): additive non-increment index (was absolute)
5d02c35 
The non-increment branch computed an absolute index (= varuint+1) where the
Source 2 format (and clarity's S2StringTableEmitter, and manta's own entity
decoder) use an additive delta (+= varuint+2). The absolute form produced
wrong, non-monotonic indices for delta-updated tables such as ActiveModifiers
(thousands of mis-indexed entries on a typical replay). This was invisible to
the existing tests because none resolve a delta-updated table by index.

go test ./... green (identical golden values; init stays -1 so the increment
path is unchanged).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(stringtable): surface parse errors instead of swallowing them
cd44c63 
parseStringTable deferred a recover() that logged and returned a partially
populated table, so callers saw success and applied incomplete state. Return
the error and propagate it from onCSVCMsg_Create/UpdateStringTable, matching
clarity's fail-loud behaviour. recover never fires on healthy replays.
Adds TestParseStringTableTruncated.

go test ./... green (identical golden values).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(gameevent): correct off-by-one bound in getEventKey
e5e3881 
getEventKey guarded with `f.i > len(keys)`, allowing f.i == len(keys) to fall
through to GetKeys()[f.i] and panic. Use >= so a version-skewed event that
advertises more fields than it carries returns the intended error instead.
Well-formed events never hit this.

go test ./... green (identical golden values).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(modifier): skip empty/deleted ActiveModifiers entries
66d7cd4 
emitModifierTableEvents unmarshalled every entry including empty (deleted)
ones, raising an all-zero CDOTAModifierBuffTableEntry to handlers. Skip entries
with no value, matching clarity. A real modifier is never zero-length on the
wire.

go test ./... green (identical golden values).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(gameevent): resolve combat-log type via descriptor, not key[0]
eaf131f 
Type(), TypeName() and String() hardcoded keys[0].GetValByte() for the combat
log type, assuming it is always the first key and always byte-encoded. Resolve
it via the descriptor (GetInt32("type")) instead, which tolerates a different
key order or int encoding across game versions. On current descriptors 'type'
is index 0, so output is unchanged; the combat-log test already asserts
GetInt32("type") succeeds on every event.

go test ./... green (identical golden values).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
feat(decoder): forward-compat decoders + 64-bit int64
ca3fade 
Add decoders for base types clarity handles that manta lacked and that would
otherwise fall through to the wrong default decoder (and desync) if a future
build networked them: CUtlBinaryBlock (length-prefixed bytes), Quaternion
(4-component vector), ResourceId_t (unsigned 64), CGlobalSymbol (string). None
occur in the test corpus, so behavior is unchanged today.

Also decode int64 with a full 64-bit varint (was 32-bit, which truncates large
values). Only m_nTotalDamageTaken is int64 today and is <=1 byte, so its value
is unchanged; note Get on an int64 field now returns int64 rather than int32.

go test ./... green (identical golden values).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(decoder): align HSequence and HeroID_t with clarity
ec1cb0f 
HSequence had no decoder and fell through to an unsigned varint; clarity
decodes it as varint-1 (a sequence handle, -1 for none). HeroID_t likewise fell
through to unsigned; clarity uses a signed varint. Both consume the same bits as
before (no desync) but produce the clarity-correct values. Neither field is
asserted by the golden tests, which stay green (incl. builds 6600/6601 where
these fields appear). HSequence is decoded as a signed int32 so wire value 0
becomes -1 rather than wrapping.

go test ./... green (identical golden assertions).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(decoder): decode BloodType as a fixed 8-bit value
fa4170d 
m_nBloodType was decoded as an unsigned varint; clarity reads a fixed 8-bit
value. These agree bit-for-bit only while the value is < 128. The full golden
suite (including builds 6600/6601 where m_nBloodType is decoded ~109k times)
stays green, confirming every value in the corpus is < 128, so this is a safe
forward-compat alignment that also reads correctly for future values >= 128.

go test ./... green (identical golden values).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
feat(decoder): handle QAngle precise/noscale/pitch-yaw raw-float cases
647a5ba 
Rewrite qangleFactory to match clarity's QAngle decoders: qangle_pitch_yaw with
bit count 0 or 32 carries raw 32-bit floats (not bit angles); qangle_precise
reads three flagged 20-bit angles; and a general bit count of 32 reads three raw
floats. Fields present in the corpus (bit counts 8/13, and the no-bit-count
coord form) keep identical behavior, so these are dormant forward-compat paths
for newer engine variants.

go test ./... green (identical golden values).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(patch): guard mana/runetime patches on the sentinel bounds
d4e8f57 
The mana and runetime field patches applied unconditionally. Guard them on the
synthetic full-float-range bounds (+/-MaxFloat32) the engine emits when no real
range is set, matching clarity. The affected fields always carry that sentinel
today, so behavior is unchanged; the guards just stop the patches from clobbering
a field that already has real bounds. Manta keeps its bespoke 4-bit runetime
decoder and /30 simtime handling.

go test ./... green (identical golden values).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(parser): bound outer-message size before allocating
8bf31ce 
readOuterMessage passed the size varint straight to stream.readBytes, which
does make([]byte, n) with no upper bound, so a corrupt or truncated stream with
a huge varint could attempt a multi-gigabyte allocation. Reject sizes above a
generous 256 MiB cap (far above any legitimate message) with an error instead.

go test ./... green (identical golden values).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
docs(fieldpath): name the max field-path depth invariant
6eb5c2c 
Introduce maxFieldPathDepth (= 7) with a comment citing clarity's
S2LongFieldPathFormat, and use it for the fixed path array. The fixed-size
array already enforces the bound via a recovered bounds-check panic; this just
documents the invariant. No behavior change.

go test ./... green (identical golden values).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
docs: phase 2 (correctness) complete
cfcc6a8 
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jcoene and others added 9 commits June 20, 2026 08:16
@jcoene @claude
perf(entity): store field values in a typed cell, box lazily on read
fbca7ed 
Entity state stored every decoded value boxed in a []interface{}, so each
float/int/handle heap-allocated on the write path — per field, per entity, per
tick. Replace it with a 24-byte tagged-union cell {ref interface{}; num uint32;
kind}: scalars (float32, int32, uint32, bool, and <=32-bit uint64 handles) live
inline in num with zero write-path allocation; reference values (string,
[]float32, []byte) and the rare genuinely-64-bit ints (CStrongHandle, fixed64,
int64, steamids) go in ref; nested tables are a *fieldState in ref. Decoders now
return a cell; values are boxed into interface{} lazily in cell.iface(), only on
read (Entity.Get), which is far rarer than the write path.

Public API is unchanged: Entity.Get still returns interface{} with the exact
same dynamic type and value as before (a pure storage refactor). go test ./...
passes with identical golden assertions, including entity_test's exact-type
checks (int32/uint64/bool/string, plus a >2^32 steamid proving 64-bit values are
not truncated) and expectHeroEntityMana's float32 round-trip.

benchstat (8552595443, 10x x10) vs end of Phase 2:
  allocs/op 10.264M -> 4.410M  -57.03% (p=0.000)
  sec/op    0.766 -> 0.734      -4.23%
  B/op      378.1Mi -> 388.2Mi  +2.65% (qangle/Vector []float32 left boxed to
                                 preserve the []float32 Get contract)

Cumulative vs P0 baseline: allocs -78.8%, sec -51.8%, B/op -51.0%.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(entity): deep-copy mutable baseline leaves in clone
40da8cc 
clone() shallow-copied cells and only deep-copied nested fieldStates, so cells
whose ref holds a []float32 vector or []byte blob were shared across every
entity cloned from the cached class baseline (introduced when baselines became
decode-once + cloned). A caller mutating a slice returned from Entity.Get/Map
could corrupt the baseline template and sibling entities; before the branch,
baselines were re-decoded per entity so this aliasing did not exist.

Deep-copy []float32/[]byte leaves in clone(); strings and boxed integers are
immutable and stay shared. Only fires for baseline vector/blob leaves.

go test ./... green (identical golden values; deep-copy yields equal slices).

benchstat (8552595443, 10x x10) vs P4 baseline:
  allocs/op 4.410M -> 4.478M  +1.54% (+68K, the per-entity leaf copies)
  B/op      388.2Mi -> 389.4Mi +0.33%
  sec/op    ~ (p=0.105)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(parser): clear reused tuple/pending buffers after dispatch
ff0e2a1 
p.entityTuples and p.pendingMsgBuf were stored back with their full length and
stale [len:cap] entries, so *Entity pointers (and their de-boxed state) and
inner-packet buffers were retained an extra packet — stale slots from a larger
prior packet could pin deleted entities. After dispatch, clear() the used
entries and keep the slice at length zero; clearing the full written length each
packet keeps [len:cap] zero across packets, so no stale references accumulate.

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P4.1:
  allocs/op flat, B/op flat (clear is alloc-free)
  sec/op    +1.4% nominal, consistent with thermal drift across the run

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(reader): guard skipBits against underflow on truncated streams
69b3aaa 
peekBits zero-pads at end of buffer and skipBits blindly subtracted, so on a
truncated or corrupt field-path op stream a lookup entry could consume more bits
than were buffered, underflowing bitCount (uint32 wraps huge) and looping
forever appending field paths. Make skipBits panic when n > bitCount; the
parser's top-level recover turns it into an error, restoring the fail-fast
behaviour the per-bit walk had. Cannot affect well-formed replays (the fast path
never under-runs on valid streams).

Adds TestReadFieldPathsTruncated (an all-zero buffer is an unbounded PlusOne
run) asserting a clean panic instead of a hang.

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P4.2: all metrics flat (guard is free;
sec p=0.631, B/op p=0.971, allocs p=0.057).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(reader): correct debug position() after word-at-a-time refill
4e9a173 
position() assumed bitCount <= 8 (one byte buffered), but the word reader can
leave 56/48/... bits buffered, making the byte/bit split wrong. Derive the
logical bit position from pos*8 - bitCount. Debug-only (called under the v(6)
guard); no parsing impact.

go test ./... green; benchstat flat vs P4.3.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
test(decoder): lock value-changing decoder representations (Decision A)
1b3d8ab 
The branch deliberately changes what Entity.Get returns for a few fields, to
match clarity's correct representation, and we are owning these as intentional
API changes (Decision A, documented in IMPROVEMENTS.md):
  - int64 fields: int32 (truncated) -> int64
  - HeroID_t: uint32 -> int32 (signed)
  - HSequence: uint32 -> int32, value - 1 (-1 = none)
  - BloodType: stayed uint64; only the fixed-8 encoding changed (value identical)

Add decoder-level tests asserting the exact dynamic type AND value each produces
(deterministic, not replay-dependent), plus a wiring test locking the
fieldTypeDecoders map entries, so these intended downstream-visible values can't
silently regress. Covers the inline-vs-boxed uint64 split including a >2^32
value (no truncation).

go test ./... green; benchstat flat vs P4.4 (test-only).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
docs: phase 4 cumulative cost (cooled re-measure)
9d530d5 
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
fix(parser): clear reused buffers on error paths too
4fbc1fe 
P4.2 cleared p.pendingMsgBuf / p.entityTuples only after successful dispatch;
on a handler/callByPacketType error the parser field still pointed at the
just-refilled backing array, so packet buffers / entity pointers stayed live
(bounded to one packet, freed at parser GC since the parse aborts, but a real
inconsistency). Dispatch into a result variable and break on error, then run a
single clear() + [:0] cleanup before a single return, so cleanup runs on success
and error alike.

Uses the result-variable pattern rather than a deferred closure on purpose: a
defer closure captures the slice header (also stored on the Parser), forcing it
to escape to the heap — a per-packet allocation in a path that runs thousands of
times. The result-variable form is allocation-free.

go test ./... green (identical golden values).

benchstat (8552595443, 10x x10) vs P4.5: alloc- and B/op-neutral (4,478,290
allocs / 408.36M B identical), sec flat.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene @claude
chore: remove IMPROVEMENTS.md planning doc
7f3f403 
The working plan/benchmark log lived in the branch during development; the
per-commit benchstat tables and the PR summary capture the durable record, so
drop the scratch doc before merge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jcoene jcoene changed the title Parser performance & correctness overhaul (~2x faster, -78% allocations) Parser performance & correctness improvements Jun 22, 2026
@jcoene jcoene merged commit 0efe7e1 into master Jun 22, 2026
1 check passed
@jcoene jcoene deleted the jcoene/goal-perf branch June 22, 2026 18:35
@jcoene jcoene mentioned this pull request Jul 1, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jcoene