Do a performance and memory consumption optimization pass #87

New issue

Closed

opened 2026-08-16 00:21:31 +00:00 by Grandiras · 2 comments

Grandiras commented

2026-08-16 00:21:31 +00:00

Owner

No description provided.

claude-bot was assigned by Grandiras

2026-08-16 00:21:36 +00:00

claude-bot commented

2026-08-16 00:21:41 +00:00

Collaborator

🤖 On it — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/254); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-87/run-254.md) — updating every ~5 minutes while the task runs.

claude-bot referenced this issue from a commit

2026-08-16 00:57:38 +00:00

perf: optimize streaming, repository, and CID hot paths

claude-bot referenced this issue from a pull request that will close it,

2026-08-16 00:58:03 +00:00

Claude: changes for issue #87 #88

claude-bot commented

2026-08-16 00:58:04 +00:00

Collaborator

Pushed to claude/issue-87.

Did a performance and memory pass over the paths a running consumer actually spends time in: firehose/Jetstream frame parsing, MST lookup and construction, CAR block resolution, CID encoding. I built a benchmark harness first so the changes were driven by measurements rather than guesses.

The three structural wins

FirehoseEventParser — 6.6× faster, 3.5× less memory. It decoded DAG-CBOR to a JsonElement, re-serialized that into a JsonNode tree, rewrote the tree, copied it into a fresh discriminator-first object, then serialized again for the deserializer — five passes per frame. The rewrite also deep-cloned at every level, so a #commit's base64 blocks string was re-copied once per level of nesting above it. It now writes the CBOR into a Utf8JsonWriter in a single pass; WriteBase64StringValue means the CAR blob never becomes a UTF-16 string at all. The discriminator-first rebuild turned out to be unnecessary — AllowOutOfOrderMetadataProperties is already set on the shared options.

CarReader.FindBlock — was O(n) per call. Resolving a repository's MST is one lookup per node, so a repo walk was quadratic, and exports run to tens of thousands of blocks. It now builds a CID index on first use (deferred, so callers that only enumerate Blocks don't pay), keyed by a span-comparing comparer so lookups allocate nothing.

MerkleSearchTree. Get/TryUpdate called List.IndexOf on the entry they were iterating — a linear rescan from the front per comparison. Get and Remove also computed the key's depth (a SHA-256) on entry and never used the result, so every tree walk hashed the key once per node visited; lookups now allocate zero. Create recomputed every key's depth at every layer and copied the entry range into new lists as it partitioned — depths are hashed once and the recursion works on index bounds.

Smaller items: single-receive fast paths in both WebSocket read loops, an exact-sized splice buffer in FirehoseVerifier (WriteMapHeader now takes a Span<byte>), FromStreamAsync parsing the buffer it already filled instead of copying the whole CAR again onto the LOH, and a thread-safe + frozen AtProtoJsonDefaults.Options (the ??= could let concurrent first calls each build their own contract cache).

One public API addition: JetstreamEventParser.ParseFrame(ReadOnlyMemory<byte>, JetstreamProtocol). JsonDocument can't parse a span without copying it first, so the span overload duplicates every frame; the client's receive loop already holds an array and now uses the no-copy path — about 600 bytes per event off the live tail.

Two things worth flagging

I measured a regression and backed it out. My first Jetstream change used JsonDocument.ParseValue(ref Utf8JsonReader) to avoid the copy. In an isolated micro-benchmark that beat the existing code, but in the real ParseFrame it was consistently ~35% slower — element access and Clone() on such a document cost more than the copy saves. The span overload is now behaviorally unchanged from before, and the win moved to the new overload.

MST.Serialize is not a time win. The original run suggested 703 µs → 900 µs, i.e. a regression. Measuring it isolated showed the 703 µs baseline was an artifact of warm caches from the benchmarks that ran before it; matched, it's ~961 µs → ~903 µs, which is inside the noise. The real gain there is allocation (−12%). The chart labels it ≈1× rather than rounding it up to a win. For the same reason all the reported figures are medians of three runs with tiered compilation disabled — with it on, benchmark ordering moved individual numbers by 3–4×.

Verified: dotnet build -c Release clean (0 warnings, 0 errors); dotnet test tests/ATProtoNet.Tests/ passes 1296 tests, 0 failures (1291 before, plus 5 new). The new tests cover the CID index including the duplicate-CID case, and the ReadOnlyMemory overload — including that a parsed Record survives its source buffer being cleared, which matters because that overload reads the caller's buffer in place. CHANGELOG.md is updated under [Unreleased].

⏱ 36m 10s · 151 turns · tokens: 301.1k in / 108.2k out (+28917.0k cache-read)
📜 Full session transcript
→ Opened PR: #88

Pushed to `claude/issue-87`. Did a performance and memory pass over the paths a running consumer actually spends time in: firehose/Jetstream frame parsing, MST lookup and construction, CAR block resolution, CID encoding. I built a benchmark harness first so the changes were driven by measurements rather than guesses. ![Dumbbell chart: benchmark time and allocation before and after, log scale](https://git.grandiras.net/attachments/28121de4-396b-4979-bc8a-fb1f099c6181) ## The three structural wins **`FirehoseEventParser` — 6.6× faster, 3.5× less memory.** It decoded DAG-CBOR to a `JsonElement`, re-serialized *that* into a `JsonNode` tree, rewrote the tree, copied it into a fresh discriminator-first object, then serialized again for the deserializer — five passes per frame. The rewrite also deep-cloned at every level, so a `#commit`'s base64 `blocks` string was re-copied once per level of nesting above it. It now writes the CBOR into a `Utf8JsonWriter` in a single pass; `WriteBase64StringValue` means the CAR blob never becomes a UTF-16 string at all. The discriminator-first rebuild turned out to be unnecessary — `AllowOutOfOrderMetadataProperties` is already set on the shared options. **`CarReader.FindBlock` — was O(n) per call.** Resolving a repository's MST is one lookup per node, so a repo walk was quadratic, and exports run to tens of thousands of blocks. It now builds a CID index on first use (deferred, so callers that only enumerate `Blocks` don't pay), keyed by a span-comparing comparer so lookups allocate nothing. **`MerkleSearchTree`.** `Get`/`TryUpdate` called `List.IndexOf` on the entry they were iterating — a linear rescan from the front per comparison. `Get` and `Remove` also computed the key's depth (a SHA-256) on entry and never used the result, so every tree walk hashed the key once per node visited; lookups now allocate zero. `Create` recomputed every key's depth at *every layer* and copied the entry range into new lists as it partitioned — depths are hashed once and the recursion works on index bounds. Smaller items: single-receive fast paths in both WebSocket read loops, an exact-sized splice buffer in `FirehoseVerifier` (`WriteMapHeader` now takes a `Span<byte>`), `FromStreamAsync` parsing the buffer it already filled instead of copying the whole CAR again onto the LOH, and a thread-safe + frozen `AtProtoJsonDefaults.Options` (the `??=` could let concurrent first calls each build their own contract cache). One public API addition: `JetstreamEventParser.ParseFrame(ReadOnlyMemory<byte>, JetstreamProtocol)`. `JsonDocument` can't parse a span without copying it first, so the span overload duplicates every frame; the client's receive loop already holds an array and now uses the no-copy path — about 600 bytes per event off the live tail. ## Two things worth flagging **I measured a regression and backed it out.** My first Jetstream change used `JsonDocument.ParseValue(ref Utf8JsonReader)` to avoid the copy. In an isolated micro-benchmark that beat the existing code, but in the real `ParseFrame` it was consistently ~35% *slower* — element access and `Clone()` on such a document cost more than the copy saves. The span overload is now behaviorally unchanged from before, and the win moved to the new overload. **`MST.Serialize` is not a time win.** The original run suggested 703 µs → 900 µs, i.e. a regression. Measuring it isolated showed the 703 µs baseline was an artifact of warm caches from the benchmarks that ran before it; matched, it's ~961 µs → ~903 µs, which is inside the noise. The real gain there is allocation (−12%). The chart labels it `≈1×` rather than rounding it up to a win. For the same reason all the reported figures are medians of three runs with tiered compilation disabled — with it on, benchmark ordering moved individual numbers by 3–4×. **Verified:** `dotnet build -c Release` clean (0 warnings, 0 errors); `dotnet test tests/ATProtoNet.Tests/` passes 1296 tests, 0 failures (1291 before, plus 5 new). The new tests cover the CID index including the duplicate-CID case, and the `ReadOnlyMemory` overload — including that a parsed `Record` survives its source buffer being cleared, which matters because that overload reads the caller's buffer in place. `CHANGELOG.md` is updated under `[Unreleased]`. --- ⏱ 36m 10s · 151 turns · tokens: 301.1k in / 108.2k out (+28917.0k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-87/run-254.md) → Opened PR: https://git.grandiras.net/Grandiras/ATProto.NET/pulls/88

perf-before-after.svg

8.1 KiB

Grandiras closed this issue

2026-08-16 01:18:09 +00:00

Grandiras referenced this issue from a commit

2026-08-16 01:18:11 +00:00

Claude: changes for issue #87 (#88)