perf: identity lookups over HTTP/2, with a handle answer cache #198

Grandiras · 2026-10-05T02:20:37Z

Grandiras commented

2026-10-05 02:20:37 +00:00

Closes #197.

What changed

HTTP/2 for identity fetches (IdentityFetch.GetAsync): every request asks for HTTP/2 with RequestVersionOrLower, so it falls back to 1.1 where the host doesn't offer h2. Concurrent lookups against one host now share a connection instead of each paying for a TCP+TLS handshake. The SSRF connect callback is unchanged: ALPN runs over the stream it returns.
Handle answer cache (HandleResolver): an LRU (capacity Cache.Capacity) keyed by the normalized handle, expiring after IdentityResolverOptions.HandleCacheTtl (new; default 5 min, TimeSpan.Zero disables). Only answers are cached, never "no answer" or a HandleConflict, so a newly set-up handle resolves at once. IdentityResolver.ResolveUncachedAsync, which OAuth uses for its post-login check, forgets the claimed handle before verifying it, so that path stays fully fresh.

Measurements (live network, this machine)

scenario	before	after
88 concurrent PLC lookups (one `DidResolver`), p50	~550 ms	~335 ms
Re-resolving a DID whose document is cached (handle re-verification), p50	~117 ms	cache hit, no request

Not included

I also tried starting the DID document fetch as soon as the first handle authority answers. It was within noise on real handles: *.bsky.social handles only answer over HTTPS, so there is nothing to overlap. I dropped it rather than add the code.

The DNS-first policy from #193 is left for the owner's decision.

🤖 Generated with Claude Code

Closes #197. ## What changed - **HTTP/2 for identity fetches** (`IdentityFetch.GetAsync`): every request asks for HTTP/2 with `RequestVersionOrLower`, so it falls back to 1.1 where the host doesn't offer h2. Concurrent lookups against one host now share a connection instead of each paying for a TCP+TLS handshake. The SSRF connect callback is unchanged: ALPN runs over the stream it returns. - **Handle answer cache** (`HandleResolver`): an LRU (capacity `Cache.Capacity`) keyed by the normalized handle, expiring after `IdentityResolverOptions.HandleCacheTtl` (new; default 5 min, `TimeSpan.Zero` disables). Only answers are cached, never "no answer" or a `HandleConflict`, so a newly set-up handle resolves at once. `IdentityResolver.ResolveUncachedAsync`, which OAuth uses for its post-login check, forgets the claimed handle before verifying it, so that path stays fully fresh. ## Measurements (live network, this machine) | scenario | before | after | |---|---|---| | 88 concurrent PLC lookups (one `DidResolver`), p50 | ~550 ms | ~335 ms | | Re-resolving a DID whose document is cached (handle re-verification), p50 | ~117 ms | cache hit, no request | ## Not included I also tried starting the DID document fetch as soon as the first handle authority answers. It was within noise on real handles: `*.bsky.social` handles only answer over HTTPS, so there is nothing to overlap. I dropped it rather than add the code. The DNS-first policy from #193 is left for the owner's decision. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit

2026-10-05 02:20:37 +00:00

perf: identity lookups over HTTP/2, with a handle answer cache (closes #197 )

CI / pds-integration (pull_request) Successful in 54s

Details

CI / build-and-test (pull_request) Successful in 1m35s

Details

64fb4039a5

- Identity fetches request HTTP/2 with fallback to 1.1, so concurrent lookups
  against the PLC directory or the DoH endpoint share one connection.
- HandleResolver remembers answers for IdentityResolverOptions.HandleCacheTtl;
  ResolveUncachedAsync forgets the claimed handle first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

opencode-bot commented

2026-10-05 02:20:38 +00:00

🤖 Reviewing this PR — the review will link the full session transcript.

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

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-198/run-2026-10-05T02-20-38-131Z.md) — updating every ~5 minutes while the review runs.

opencode-reviewer approved these changes

2026-10-05 02:22:32 +00:00

Dismissed

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Good to merge. The correctness-sensitive parts hold up:

HTTP/2: per-request Version/VersionPolicy.RequestVersionOrLower is the right shape — it downgrade-negotiates per host, works over the SDK's custom ConnectCallback (ALPN is performed by SocketsHttpHandler on the stream it returns), and is harmless where a caller-supplied handler (or a WASM HttpClientHandler) ignores it.
Cache correctness: the answer is only stored after the HTTPS/DNS conflict check has run, so a conflicting pair is never frozen into the cache; "no answer", conflicts, and unresolvable TLDs are never cached, so a newly set-up handle resolves immediately. ResolveUncachedAsync forgets the claimed handle before verifying, keeping the OAuth post-login check fully fresh — covered by a test.
LRU reuse, capacity, and TTL validation are wired through the existing LruCache, Cache.Capacity, and Validate() cleanly; public API file and docs/CHANGELOG updated.

Minor, non-blocking notes:

No TimeProvider for the TTL. Nearly every other timer in the SDK (CachingDidResolver, AuthorizationServerDiscovery, OAuthClient, …) takes a TimeProvider, which makes TTL expiry testable — here Environment.TickCount64 means expiry can't be exercised in tests. Also slightly less robust if a system clock skew mattered (TickCount64 is monotonic of course). Worth a follow-up ticket if the codebase cares about that seam.
Expired entries linger: there's no eviction of expired entries, so with a small Capacity a burst of one-shot handles can crowd out still-fresh answers. At the default 10 000 this is theoretical; flagged only in case someone tunesCapacity down sharply.
HandleCacheTtl capacity sharing with the DID-document cache (Cache.Capacity) is documented in the remark — just be aware tuning the DOC cache silently moves the handle cache's ceiling.

The DNS-first policy from #193 being left for the owner's decision is reasonable.

⏱ 1m 54s · 22 steps · $0.0296 · tokens: 48.9k in / 1.4k out (+452.0k cache-read) · Go: 5h 0% · wk 0% · mo 90%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Good to merge. The correctness-sensitive parts hold up: - **HTTP/2**: per-request `Version`/`VersionPolicy.RequestVersionOrLower` is the right shape — it downgrade-negotiates per host, works over the SDK's custom `ConnectCallback` (ALPN is performed by `SocketsHttpHandler` on the stream it returns), and is harmless where a caller-supplied handler (or a WASM `HttpClientHandler`) ignores it. - **Cache correctness**: the answer is only stored after the HTTPS/DNS conflict check has run, so a conflicting pair is never frozen into the cache; "no answer", conflicts, and unresolvable TLDs are never cached, so a newly set-up handle resolves immediately. `ResolveUncachedAsync` forgets the claimed handle before verifying, keeping the OAuth post-login check fully fresh — covered by a test. - **LRU reuse, capacity, and TTL validation** are wired through the existing `LruCache`, `Cache.Capacity`, and `Validate()` cleanly; public API file and docs/CHANGELOG updated. Minor, non-blocking notes: - **No `TimeProvider` for the TTL.** Nearly every other timer in the SDK (`CachingDidResolver`, `AuthorizationServerDiscovery`, `OAuthClient`, …) takes a `TimeProvider`, which makes TTL expiry testable — here `Environment.TickCount64` means expiry can't be exercised in tests. Also slightly less robust if a system clock skew mattered (TickCount64 is monotonic of course). Worth a follow-up ticket if the codebase cares about that seam. - **Expired entries linger**: there's no eviction of expired entries, so with a small `Capacity` a burst of one-shot handles can crowd out still-fresh answers. At the default 10 000 this is theoretical; flagged only in case someone tunesCapacity down sharply. - `HandleCacheTtl` capacity sharing with the DID-document cache (`Cache.Capacity`) is documented in the remark — just be aware tuning the DOC cache silently moves the handle cache's ceiling. The DNS-first policy from #193 being left for the owner's decision is reasonable. --- ⏱ 1m 54s · 22 steps · $0.0296 · tokens: 48.9k in / 1.4k out (+452.0k cache-read) · Go: 5h 0% · wk 0% · mo 90% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-198/run-2026-10-05T02-20-38-131Z.md)

src/ATProtoNet/Identity/HandleResolver.cs Outdated

					
				@ -82,6 +86,9 @@ public sealed class HandleResolver : IHandleResolver, IDisposable

				            return null;

				        }

				        if (_answers is not null && _answers.TryGetValue(handle, out var cached) && Environment.TickCount64 < cached.ExpiresAt)

Non-blocking: Environment.TickCount64 makes the TTL untestable and diverges from the rest of the SDK, which uses TimeProvider everywhere (CachingDidResolver, AuthorizationServerDiscovery, OAuthStateStore...). Suggest accepting a TimeProvider? (defaulting to TimeProvider.System) so expiry can be exercised — the rest of the codebase already has the pattern. Not a merge blocker; filing as a follow-up candidate.

Non-blocking: `Environment.TickCount64` makes the TTL untestable and diverges from the rest of the SDK, which uses `TimeProvider` everywhere (CachingDidResolver, AuthorizationServerDiscovery, OAuthStateStore...). Suggest accepting a `TimeProvider?` (defaulting to `TimeProvider.System`) so expiry can be exercised — the rest of the codebase already has the pattern. Not a merge blocker; filing as a follow-up candidate.

Grandiras commented

2026-10-05 02:47:48 +00:00

@claude /review

Grandiras force-pushed issue-197-identity-latency from 64fb4039a5

CI / pds-integration (pull_request) Successful in 54s

Details

CI / build-and-test (pull_request) Successful in 1m35s

Details

to 6fc044e563

CI / pds-integration (pull_request) Failing after 34m40s

Details

CI / build-and-test (pull_request) Failing after 40m21s

Details

2026-10-05 02:58:16 +00:00

Compare

opencode-bot commented

2026-10-05 03:29:30 +00:00

🤖 Reviewing this PR — the review will link the full session transcript.

🤖 Reviewing this PR — the review will link the full session transcript.

Grandiras force-pushed issue-197-identity-latency from 6fc044e563

CI / pds-integration (pull_request) Failing after 34m40s

Details

CI / build-and-test (pull_request) Failing after 40m21s

Details

to 08264614ee

CI / pds-integration (pull_request) Successful in 26s

Details

CI / build-and-test (pull_request) Successful in 50s

Details

Sync Closures to GitHub / sync-closure (pull_request) Successful in 6s

Details

2026-10-05 04:00:22 +00:00

Compare

opencode-bot commented

2026-10-05 04:01:25 +00:00

🤖 Reviewing this PR — the review will link the full session transcript.

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

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-198/run-2026-10-05T04-01-24-437Z.md) — updating every ~5 minutes while the review runs.

opencode-reviewer approved these changes

2026-10-05 04:02:00 +00:00

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Re-review: the implementation is unchanged since my previous approval — I re-read HandleResolver.cs, IdentityFetch.cs, IdentityResolver.cs, and the tests, and they match the reviewed diff exactly. The correctness-sensitive parts still hold: HTTP/2 is requested per-request with RequestVersionOrLower (safe downgrade, works over the custom connect callback, harmless where a caller-supplied handler ignores it); the handle cache stores only positive answers after the HTTPS/DNS conflict check, never "no answer" or a conflict; and ResolveUncachedAsync forgets the claimed handle before verifying, keeping the OAuth post-login path fresh — all covered by tests.

The two non-blocking notes from my last pass stand as follow-up candidates, unchanged:

No TimeProvider seam for HandleCacheTtl (Environment.TickCount64 directly), so TTL expiry isn't testable and the class diverges from the rest of the SDK's timer pattern.
Expired entries are not evicted until overwritten, so a burst of one-shot handles can crowd out still-fresh answers at small Cache.Capacity values (theoretical at the default 10 000).

Both are worth a follow-up ticket, neither blocks merging. Good to merge.

⏱ 0m 36s · 12 steps · $0.0088 · tokens: 32.6k in / 918 out (+106.4k cache-read) · Go: 5h 1% · wk 0% · mo 91%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Re-review: the implementation is unchanged since my previous approval — I re-read `HandleResolver.cs`, `IdentityFetch.cs`, `IdentityResolver.cs`, and the tests, and they match the reviewed diff exactly. The correctness-sensitive parts still hold: HTTP/2 is requested per-request with `RequestVersionOrLower` (safe downgrade, works over the custom connect callback, harmless where a caller-supplied handler ignores it); the handle cache stores only positive answers after the HTTPS/DNS conflict check, never "no answer" or a conflict; and `ResolveUncachedAsync` forgets the claimed handle before verifying, keeping the OAuth post-login path fresh — all covered by tests. The two non-blocking notes from my last pass stand as follow-up candidates, unchanged: - No `TimeProvider` seam for `HandleCacheTtl` (`Environment.TickCount64` directly), so TTL expiry isn't testable and the class diverges from the rest of the SDK's timer pattern. - Expired entries are not evicted until overwritten, so a burst of one-shot handles can crowd out still-fresh answers at small `Cache.Capacity` values (theoretical at the default 10 000). Both are worth a follow-up ticket, neither blocks merging. Good to merge. --- ⏱ 0m 36s · 12 steps · $0.0088 · tokens: 32.6k in / 918 out (+106.4k cache-read) · Go: 5h 1% · wk 0% · mo 91% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-198/run-2026-10-05T04-01-24-437Z.md)

Grandiras commented

2026-10-05 11:49:29 +00:00

@claude /review

claude-bot commented

2026-10-05 11:49:32 +00:00

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

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

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/728); 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/pr-198/run-728.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-10-05 11:49:59 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

The change is small, matches the description, and I found nothing blocking.

HTTP/2: RequestVersionOrLower falls back to 1.1, and the SSRF connect callback is untouched.
Cache: it reuses the thread-safe LruCache. HandleCacheTtl is validated and zero disables it. Only answers are cached, never "no answer" or a conflict.
Uncached path: ResolveUncachedAsync calls Forget on the claimed handle. This is a no-op for a custom IHandleResolver, which is acceptable.
Tests, changelog, docs, PublicAPI.Unshipped.txt: all updated, and the tests cover the new behaviour.

Minor, not blocking:

Race: a lookup already in flight when Forget runs can re-insert its older answer, so ResolveUncachedAsync is not strictly fresh under concurrency. Its own verification still uses a fresh result in that call.
Expired entries: they are only overwritten or LRU-evicted, never removed on read. That is harmless given the bounded capacity.

🧠 claude-sonnet-5-5 · ⏱ 0m 9s · 3 turns · tokens: 30.4k in / 844 out (+29.1k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. The change is small, matches the description, and I found nothing blocking. - **HTTP/2:** `RequestVersionOrLower` falls back to 1.1, and the SSRF connect callback is untouched. - **Cache:** it reuses the thread-safe `LruCache`. `HandleCacheTtl` is validated and zero disables it. Only answers are cached, never "no answer" or a conflict. - **Uncached path:** `ResolveUncachedAsync` calls `Forget` on the claimed handle. This is a no-op for a custom `IHandleResolver`, which is acceptable. - **Tests, changelog, docs, `PublicAPI.Unshipped.txt`:** all updated, and the tests cover the new behaviour. Minor, not blocking: - **Race:** a lookup already in flight when `Forget` runs can re-insert its older answer, so `ResolveUncachedAsync` is not strictly fresh under concurrency. Its own verification still uses a fresh result in that call. - **Expired entries:** they are only overwritten or LRU-evicted, never removed on read. That is harmless given the bounded capacity. --- 🧠 claude-sonnet-5-5 · ⏱ 0m 9s · 3 turns · tokens: 30.4k in / 844 out (+29.1k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-198/run-728.md)

Grandiras merged commit ed3a8dfbcf into main

2026-10-05 12:06:43 +00:00

Grandiras deleted branch issue-197-identity-latency

2026-10-05 12:06:43 +00:00

Grandiras referenced this pull request from a commit

2026-10-05 12:06:44 +00:00

perf: identity lookups over HTTP/2, with a handle answer cache (#198)

opencode-bot referenced this pull request

2026-10-05 12:35:12 +00:00

refactor: debloat the auth and server layers #206

Sign in to join this conversation.

No reviewers