Repository navigation
perf(store): in-place resolved-path pubkey scanning - #575
Conversation
decodePersistedRelayPath (Load, chunk merge, live ingest) and the eviction prefetch decoded every observations.resolved_path value with encoding/json. The ingestor writes it as compact json.Marshal output of []*string, so read that form directly and keep the JSON decoder as the fallback for anything else (whitespace, escapes, non-ASCII, non-string values, trailing data). The result is unchanged: strings verbatim, same order, duplicates kept, null and "" dropped (differential + fuzz test against extractResolvedPubkeys(unmarshalResolvedPath)). Retained pubkeys are copies, never substrings of the row: the load path clones each kept hop, and the eviction prefetch clones each distinct pubkey per transmission, since evictStale re-assigns s.byNode[pk] and a string-key assignment stores the new key. 2.5M observations / 121K transmissions: Load allocations 140.4M -> 127.9M, allocated 5958 -> 5131 MiB, retained heap unchanged; full eviction prefetch 2.95 s -> 1.67 s, 46.4M -> 24.8M allocations. Relates to #533 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rapport — CS-MacBook PR#575 #533 — head 5199a44Status: Implemented and measured, draft. CI is green except E2E shard 3/3, which failed once in Evidence tags: [T] = test/benchmark run locally or in CI, [A] = architecture/code reading, [K] = known or pre-existing, not caused by this PR. Requirements
All mutants: 12/12 killed [T]. Invariants
Local suites
CI per job (head 5199a44)
Leftovers
|
Review — CS-pve-agent3 PR#575 — head a26c43eDom: APPROVE with nits Independent, read-only review. Head Evidence tags: [T] = I ran it, [A] = code/architecture reading, [K] = known or pre-existing, not caused by this PR. Findings
No blocking findings. Answers to the review pointsIn-place scan equivalent to the JSON decoder (#533)
Validation, canonicalisation, hash width, ordering, duplicates
Safe fallback for escaped or unusual JSON
No pinning of large input buffers
Measurement, and whether the gain justifies the complexity
#489 pool reused, not duplicated
#495 lock rule and
Combination with #534/#573 and #490/#555 after the sync
Invariants
Acceptance criteria → tests
Tests and mutants I ran
Not verified
🤖 Generated with Claude Code |
Relates to #533
Summary
decodePersistedRelayPath(Load, chunk merge, live ingest) and the eviction prefetch (resolvedPubkeysForEvictionBatch) decoded everyobservations.resolved_pathvalue withencoding/json. The ingestor writes that column as compactjson.Marshaloutput of[]*string, so this PR reads that form in place and keeps the JSON decoder as the fallback for everything else.Plan (as executed)
-race, E2E, before/after benchmarks.Inventory
decodePersistedRelayPathalready decoded straight into[]stringand compacted in place (fix(store): traffic_share_score inflates with uptime — late observations re-append the same tx to byPathHop #158 follow-up), and the live paths call it before takings.mu. Its contract isextractResolvedPubkeys(unmarshalResolvedPath(s))pluspresent(NULL column vs. persisted path).byNode/nodeHashes/byPathHopverbatim, so the scanner keeps them verbatim too. Hash width is irrelevant at this layer: hops are full pubkeys ornull.unmarshalResolvedPath(a*stringper hop) and then deduplicated per transmission.What changed
resolved_path_scan.go:scanResolvedPathPubkeysaccepts only[, then"…"ornullseparated by,, then], where string bytes are printable ASCII (0x20–0x7e) with no backslash. That is exactly the set thatjson.Unmarshalwould return unchanged. Whitespace, escapes, non-ASCII bytes (which JSON would reject or rewrite to U+FFFD), numbers, nested values, top-levelnulland trailing data all report "not compact", and the caller falls back to the JSON path. Order and duplicates are kept;nulland""are dropped.decodePersistedRelayPath: scans into a stack buffer, then clones each kept hop, because these strings become retained map keys.resolvedPubkeysForEvictionBatch: appends substrings with no allocation, then clones each distinct pubkey per transmission in the existing dedup loop. This matters becauseevictStalere-assignss.byNode[pk] = filtered, and assigning to an existing string key stores the new key header. A substring there would pin its whole row (TestEvictionBatchDoesNotPinRows_533caught this during development).byNode/nodeHashes/byPathHopmap key, and those maps already hold one copy per distinct pubkey. A pool would add a lock and a lookup per hop and save no retained bytes. So there is no new cache, no saturation case, no lock, and no change totrackedBytescharges.Benchmarks (Apple M5, Go 1.26.3; base =
origin/master273c216, same benchmark file copied in)Micro,
-count 6, medians:DecodePersistedRelayPath_533(realistic 1–3 hop values)EvictionBatch_533(10K tx ≈ 150K rows)Load_Obs489(250K obs,-benchtime 3x)Load_Obs489heapMiB / trackedMiB¹
-benchmemtruncates to an integer. The mean is just under 3 (one slice plus one copy per kept hop).Macro,
TestResolvedPathScan533_Measureat 2.5M observations / 121,166 transmissions, 3 alternating runs per side:Sysafter load (peak-RSS proxy)Attribution before the change (pprof on the 2.5M run): the decoder was 4.6 % of Load CPU but 21 % of Load's allocated bytes (17 % of objects). JSON decoding was 48 % of the eviction prefetch's bytes (59 % of objects). The rest of the prefetch is SQL row scanning. These are synthetic measurements on a laptop, not measurements of a deployed instance.
Sysis the Go runtime's figure, not OS max-RSS.Perf justification (hot path): the scan is O(len(value)) per observation with no locks, maps or global state. It runs where the JSON decode ran: outside
s.muon the live paths, and in the unlocked phase ofRunEviction.Tests
TestDecodePersistedRelayPathMatchesJSON_533: about 60 edge cases (empty,null,[], nulls,"", duplicates, odd but valid strings, malformed/trailing/whitespace, numbers, nested values, every escape kind, control bytes, DEL, valid UTF-8, invalid bytes, surrogates). Each is checked againstextractResolvedPubkeys(unmarshalResolvedPath)for both entry points.FuzzPersistedRelayPath_533: the same differential. Run locally for 120 s, about 4.06M executions, no divergence.TestScanResolvedPathPubkeysGrammar_533: accepts and rejects the right inputs, and leavesdstuntouched on reject.TestDecodePersistedRelayPathDoesNotPinInput_533andTestEvictionBatchDoesNotPinRows_533: retained strings are copies.TestPersistedRelayPathAllocs_533: at most 3 allocs for a 2-hop decode, 0 for the eviction append. On the JSON-only code this was 9 and 11 (red).TestEvictionBatchMatchesJSON_533: compact and fallback rows mixed per transmission, first-seen dedup order.presentlost, dedup removed, scan disabled). All 12 were killed.cd cmd/server && go test ./...✅,cd cmd/ingestor && go test ./...✅,go test -raceon load/chunk/evict/fix(store): traffic_share_score inflates with uptime — late observations re-append the same tx to byPathHop #158/perf(store): evaluate in-place resolved-path pubkey scanning and bounded sharing #533/TestNoSQLUnderStoreLock✅,sh test-all.sh267/267 ✅,node test-frontend-helpers.js719/0 ✅.test-e2e-playwright.jsagainst a local server on the preparede2e-fixture.db(CI steps): 154/157, 3 built-in skips ✅.Scope and invariants
cmd/serverremains read-only. No newmap[string]interface{}. No frontend change, andcheck-xss-sinks.sh --diff origin/masteris clean. Workflows are untouched.🤖 Generated with Claude Code