mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-10-12 01:17:17 +00:00
## Summary
The subpath index keeps every contiguous run of 2-8 hops from every
stored transmission's path, in two maps:
- `spIndex`: key → count
- `spTxIndex`: key → transmissions containing that subpath, one entry
per occurrence
Both were always updated together (the only production callers were
`addTxToSubpathIndexFull` / `removeTxFromSubpathIndexFull`, which touch
both), so every count equalled `len(spTxIndex[key])`. The existing
`TestSubpathTxIndexPopulated` asserted exactly that. The index therefore
held its 1M+ keys twice.
This PR removes `spIndex`. The two analytics consumers read `len(txs)`,
and the add/remove helpers take only the transaction map. No API
response changes.
No existing upstream issue covers this; it came out of profiling a
production instance where garbage collection was 87% of CPU (30 s CPU
profile: `gcDrain` 39.9 s of 45.9 s sampled) with an 805 MB live heap
against a 1 GiB `GOMEMLIMIT`. That heap profile put 186 MB in this
index; on that instance's 7-day window there are 1.2M distinct subpath
keys (79% seen once) and 3.07M entries.
## Performance
### Go benchmark
`BenchmarkBuildSubpathIndex` builds the index for a synthetic,
production-shaped window: 41k transmissions with path lengths drawn from
the production hop-count histogram, and hops from random walks over a
400-node graph with 3-byte prefixes. That gives 1.48M keys, 85% seen
once (production: 1.2M, 79%). No real data is used. 10 runs each:
```
cpu: AMD Ryzen 5 3600XT 6-Core Processor
│ master │ this PR │
│ sec/op │ sec/op vs base │
BuildSubpathIndex-12 2.157 ± 1% 1.610 ± 5% -25.33% (p=0.000 n=10)
│ retained-B │ retained-B vs base │
BuildSubpathIndex-12 238.5Mi ± 0% 187.7Mi ± 0% -21.31% (p=0.000 n=10)
│ B/op │ B/op vs base │
BuildSubpathIndex-12 572.0Mi ± 0% 465.6Mi ± 0% -18.61% (p=0.000 n=10)
```
(`retained-B` is the live heap the index keeps after a forced GC: 250.1
MB → 196.8 MB.)
### Staging heap
Live heap after a forced GC (`/debug/pprof/heap?gc=1`), on a staging
host running a copy of a production database and its live MQTT feed,
attributed to the subpath index builder:
| | master | this PR |
|---|---|---|
| subpath index (`buildSubpathIndex`, cum) | 172.4 MB | 116.1 MB |
| packets in memory at the time | 27,521 | 24,086 |
| per packet | 6.27 KB | 4.82 KB (**-23%**) |
The two snapshots were taken minutes apart with different numbers of
packets loaded, hence the per-packet figure; it agrees with the
benchmark's -21%.
The same A/B replay used for the other PRs (30 minutes of real
production API traffic) showed no latency change (p50 10.3 ms vs 10.7
ms, p95 59.5 ms vs 52.9 ms, 0 errors). GC count and CPU were lower in
this run (44 → 28 collections, 13.0% → 9.6% CPU), but runs on this
shared host vary by that much on their own, so no claim is made from
them.
## Tests
- Red commit `ded359ff`: the fixture, `TestSubpathFixtureShape` (keeps
the fixture production-like: 60-95% singleton keys),
`BenchmarkBuildSubpathIndex`, and `TestSubpathIndexRetainedHeap`, which
allows at most 225 MB for the 41k fixture. It fails on `master` with
250.1 MB.
- Green commit `e399c561`: the change. The heap test passes at about 197
MB.
- `make test`: 16 of 16 modules pass; `go test -race` over the subpath,
eviction, path-hop and resolved-index tests passes.
Test changes in the green commit are mechanical, as required for a
structural change:
- struct literals stop setting `spIndex`; where a literal did not set
`spTxIndex` it now does, because appending to a nil map panics where the
old code skipped the transaction map when passed `nil`;
- count assertions read `len(spTxIndex[key])`;
- the old "spIndex and spTxIndex agree" check becomes "no key is kept
with an empty list", which is what keeps `len()` a correct count.
## Review notes
- Removal still takes out one occurrence per subpath occurrence and
deletes a key whose list becomes empty, so `len()` stays equal to the
old count through eviction.
- The store's memory estimate (`perSubpathEntryBytes`) is unchanged
here; accounting accuracy is a separate item.
- No new `map[string]interface{}`.
## SonarLint
SonarQube Community rules (the SonarLint engine) were run on the files
this PR touches, and only issues on lines it adds are listed.
- **Fixed** in `8af9582f`: S3776 (cognitive complexity 16 > 15) in the
test fixture. Path generation is split into graph and hop-count helpers;
the random draws stay in the same order, so the fixture is unchanged.
## Checked against the repo rules
- Rebased onto `master` (`673442cc`). Each red commit was re-run and
still fails on its assertions; each green commit passes (TDD.md).
- `make test`: all 16 modules pass. Frontend suites: all pass except
`test-issue-1956-release-routing.js`, which needs CI's `GITHUB_REF_NAME`
and fails the same way on `master` locally (#2160 addresses it).
- No private data: test fixtures and examples use the codes the existing
tests use (SJC/SFO/LAX, `#be`-style scopes).
- No new `map[string]interface{}`, no hard-coded colours. `git add` was
explicit, and there is one logical change per commit.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>