From a48dc32b8a2c307ee54c4b6e0a9d7cbdc29225bc Mon Sep 17 00:00:00 2001 From: Alex B <142788550+A13xB0@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:42:17 +0100 Subject: [PATCH] perf(store): build hop context without a map or lowered copies (#2186) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `buildHopContextPubkeys` (#1197) runs for every transmission in each topology and distance pass, inside `buildAggregateHopContextPubkeys` for every subpath query, and for each matched packet in subpath detail. On every call it: - made a de-dup map sized for 16 keys, to de-duplicate what's usually 2–5 keys; - lowered every hop into a new string just to look it up in the prefix map. Hops are upper-case hex on live data, so `strings.ToLower` always allocates; - lowered the observer ID twice, once to check it's hex and once to add it. ## Change - Duplicates are found by scanning the result slice. - Hops are lowered into a 64-byte stack buffer for the prefix-map lookup. `m[string(buf)]` doesn't allocate, and non-ASCII input falls back to `strings.ToLower`, so lookups are unchanged. - The observer ID is checked as hex in either case (`isHexAnyCase`) and lowered once. - The result is the same keys in the same order: sender, observer, then unambiguous hop anchors. ## Perf justification (AGENTS.md rule 0) - **Complexity.** The de-dup scan is O(k²) for the k keys of one transmission. k is at most 2 + path hops; MeshCore paths are under 64 hops and real values are 2–10. It replaces a map allocated on every call, and nothing grows with the number of transmissions or nodes. - **Scale.** It runs for every stored transmission in each analytics pass (topology and distance) and per request in subpath analytics, so the saving recurs on every recompute, not just at startup. Benchmarks, current master vs this branch, same machine: | | master | this PR | |---|---|---| | `BenchmarkBuildHopContextPubkeys` | 830 ns, 1192 B, 4 allocs | 507 ns, 128 B, 1 alloc | | `BenchmarkBuildAggregateHopContextPubkeys` | 5.6 ms, 6.65 MB, 20.4k allocs | 3.7 ms, 1.33 MB, 5.3k allocs | On a copy of a busy regional network's 7-day window (64k transmissions): | | master | this PR | |---|---|---| | this function's allocations over startup and the first analytics pass | 317 MB | 80 MB | | server CPU over the same run | 175.6 s | 165.6 s | ## Tests - `TestBuildHopContextPubkeysAllocatesOnlyItsResult` uses a fixture cased like live data (upper-case hops and observer ID): at most 2 allocations, the result and the lowered observer ID. Red commit first (9 allocations), then green. - `TestBuildHopContextPubkeysOrderAndDedup` pins the same keys in the same order. - The existing hop-context and disambiguation tests (#1198, #1202) pass. - `cd cmd/server && go test ./...` passes. - SonarQube: quality gate passed, no issues on added lines. Backend only: no UI, API shape or config change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H --------- Co-authored-by: Claude Opus 5.5 --- cmd/server/hop_context_allocs_test.go | 53 +++++++++++++++++++++++++ cmd/server/store.go | 56 ++++++++++++++++++++++----- 2 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 cmd/server/hop_context_allocs_test.go diff --git a/cmd/server/hop_context_allocs_test.go b/cmd/server/hop_context_allocs_test.go new file mode 100644 index 00000000..d052f033 --- /dev/null +++ b/cmd/server/hop_context_allocs_test.go @@ -0,0 +1,53 @@ +package main + +import "testing" + +func hopContextFixture() (*StoreTx, *prefixMap) { + pm := buildPrefixMap([]nodeInfo{ + {PublicKey: "aa11000000000000000000000000000000000000000000000000000000000001", Role: "repeater"}, + {PublicKey: "bb22000000000000000000000000000000000000000000000000000000000002", Role: "repeater"}, + {PublicKey: "bb33000000000000000000000000000000000000000000000000000000000003", Role: "repeater"}, + }) + tx := &StoreTx{ + DecodedJSON: `{"pubKey":"cc44000000000000000000000000000000000000000000000000000000000004"}`, + ObserverID: "DD55000000000000000000000000000000000000000000000000000000000005", + PathJSON: `["AA","BB","aa","AA11"]`, + } + tx.ParsedDecoded() + txGetParsedPath(tx) + return tx, pm +} + +// buildHopContextPubkeys runs per transmission in every topology and +// distance pass and per matched packet in subpath detail. Its de-dup map, +// sized for 16 keys, was allocated on every call (150MB of churn per +// analytics recompute on a busy week) to de-duplicate a handful of keys, +// and live hops and observer IDs are upper-case, so each was lowered into +// a new string just to look it up. The fixture is cased like live data. +func TestBuildHopContextPubkeysAllocatesOnlyItsResult(t *testing.T) { + tx, pm := hopContextFixture() + // The result slice, and the lowered observer ID it holds. + if n := testing.AllocsPerRun(200, func() { _ = buildHopContextPubkeys(tx, pm) }); n > 2 { + t.Fatalf("buildHopContextPubkeys allocates %.0f times per call, want at most 2", n) + } +} + +// Same keys, same order: sender, observer, then unambiguous hop anchors, +// lower-cased and de-duplicated. +func TestBuildHopContextPubkeysOrderAndDedup(t *testing.T) { + tx, pm := hopContextFixture() + got := buildHopContextPubkeys(tx, pm) + want := []string{ + "cc44000000000000000000000000000000000000000000000000000000000004", + "dd55000000000000000000000000000000000000000000000000000000000005", + "aa11000000000000000000000000000000000000000000000000000000000001", + } + if len(got) != len(want) { + t.Fatalf("got %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("got %v, want %v", got, want) + } + } +} diff --git a/cmd/server/store.go b/cmd/server/store.go index 00ddb553..9f55f252 100644 --- a/cmd/server/store.go +++ b/cmd/server/store.go @@ -5314,17 +5314,19 @@ func buildHopContextPubkeys(tx *StoreTx, pm *prefixMap) []string { if tx == nil || pm == nil { return nil } - seen := make(map[string]struct{}, 16) - out := make([]string, 0, 16) + // A handful of keys: de-duplicate by scanning out rather than with a + // map, which cost an allocation per call on every analytics pass. + out := make([]string, 0, 8) add := func(pk string) { if pk == "" { return } l := strings.ToLower(pk) - if _, ok := seen[l]; ok { - return + for _, have := range out { + if have == l { + return + } } - seen[l] = struct{}{} out = append(out, l) } @@ -5346,15 +5348,24 @@ func buildHopContextPubkeys(tx *StoreTx, pm *prefixMap) []string { // prefix. The full prefix-map lookup would also be acceptable, but the // hex+length check is O(len) and avoids one map probe per tx on a hot // path. See #1197 (adversarial r1 #4). - if obs := tx.ObserverID; obs != "" && len(obs) >= 4 && isHexLower(strings.ToLower(obs)) { + if obs := tx.ObserverID; obs != "" && len(obs) >= 4 && isHexAnyCase(obs) { add(obs) } // Unambiguous-prefix anchors: any hop in the path whose prefix has exactly - // one candidate is a strong context signal. + // one candidate is a strong context signal. Live hops are upper-case; + // they are lowered into a stack buffer for the lookup, which does not + // allocate. + var buf [64]byte for _, hop := range txGetParsedPath(tx) { - h := strings.ToLower(hop) - if cands, ok := pm.m[h]; ok && len(cands) == 1 { + var cands []nodeInfo + var ok bool + if b, ascii := lowerASCII(buf[:0], hop); ascii && len(hop) <= len(buf) { + cands, ok = pm.m[string(b)] + } else { + cands, ok = pm.m[strings.ToLower(hop)] + } + if ok && len(cands) == 1 { add(cands[0].PublicKey) } } @@ -5362,6 +5373,33 @@ func buildHopContextPubkeys(tx *StoreTx, pm *prefixMap) []string { return out } +// lowerASCII appends hop lower-cased to dst; ascii is false (and dst +// unusable) if hop has a non-ASCII byte, where strings.ToLower must be used. +func lowerASCII(dst []byte, hop string) ([]byte, bool) { + for i := 0; i < len(hop); i++ { + c := hop[i] + if c >= 0x80 { + return dst, false + } + if 'A' <= c && c <= 'Z' { + c += 'a' - 'A' + } + dst = append(dst, c) + } + return dst, true +} + +// isHexAnyCase reports whether s consists only of hex digits, either case. +func isHexAnyCase(s string) bool { + for i := 0; i < len(s); i++ { + c := s[i] + if !('0' <= c && c <= '9' || 'a' <= c && c <= 'f' || 'A' <= c && c <= 'F') { + return false + } + } + return s != "" +} + // isHexLower reports whether s consists only of [0-9a-f] (assumes already // lowercased by caller). Used to guard ObserverID before adding it to the // hop-disambiguation context, since ObserverID is a free-form observers.id