Skip to content

perf(core): halve allocations on the coalesced cache-miss path - #8

Merged
FranRuiz98 merged 1 commit into
masterfrom
claude/gallant-tesla-3b986c
Jul 25, 2026
Merged

perf(core): halve allocations on the coalesced cache-miss path#8
FranRuiz98 merged 1 commit into
masterfrom
claude/gallant-tesla-3b986c

Conversation

@FranRuiz98

Copy link
Copy Markdown
Owner

Core performance pass over the caching and coalescing layers, plus two correctness bugs found along the way.

Measured on the coalesced cache-miss path — 256 KB body, 8 concurrent waiters, median of 40 runs:

Allocated per miss Multiple of body
Before 8,948,560 B 34.1x
After 4,748,376 B 18.1x

-47%, and the saving scales with the number of waiters: each one used to pay its own full copy of the body.

What changed

Body handling

  • Reuse the buffer the coalescer already materialised. On a miss the body was buffered by CachedResponse, then read out again by StoreAsync via ReadAsByteArrayAsync (another full copy) and rebuffered into a second ByteArrayContent. The new internal BufferedByteArrayContent lets the caching layer tell an already-materialised body from a live network stream, so it reuses the array. This is the bulk of the 47%.
  • Enforce MaxResponseBodyBytes while reading, not after. A declared Content-Length over the limit is now rejected before a byte is read; a chunked body is read incrementally and abandoned as soon as it crosses the limit. Previously a 500 MB response was fully allocated and then rejected — exactly what the limit exists to prevent.
  • Skip buffering oversized responses entirely. A body over MaxBodySizeBytes was buffered into a byte[] just to be discarded. The caller now keeps its untouched stream.

Store behaviour

  • Stop retaining unusable entries. An entry with no freshness left, no stale window and no validator can neither be served nor revalidated. Without MaxCacheSize there is no SizeLimit and therefore no LRU, so those entries lived for the lifetime of the process. The distributed store now skips the write too, instead of issuing one that Redis deletes straight away via a negative TTL.
  • Drop the read that preceded every §4.4 invalidation. It existed only so the log and metric counted confirmed deletions, at the cost of fetching the whole stored body over the network to decide whether to log. Removal is idempotent: 2 -> 1 round-trips in the common case, 6 -> 3 in the worst.
  • Normalize Vary field names once at store time (lower-cased, sorted), so each lookup only concatenates instead of copying, sorting and lower-casing them again.

Correctness

  • HEAD served from cache dropped its content headers. HandleHeadAsync replaced Content wholesale to empty the body, losing Content-Type and reporting Content-Length: 0 — against RFC 9110 §9.3.2, which asks for the same header fields the equivalent GET would have sent.
  • stale-while-revalidate deduplication was per handler instance. It lived on CachingMiddleware, which IHttpClientFactory rotates every two minutes and can keep several of alive at once, so two live chains could revalidate the same key simultaneously — the duplicated origin load SWR exists to avoid. Moved to a per-client BackgroundRevalidationCoordinator, which also claims the key before starting the work: a refresh that finished before its own registration landed used to leave the key claimed forever, permanently blocking that entry.

Reviewer notes

Three deliberate behaviour changes worth a look:

  1. stampede_http.cache.invalidations now counts invalidations issued, not confirmed deletions. That is the price of dropping the read; the description and log level were updated to match. If the exact count matters, it can be recovered with a TryRemoveAsync on ICacheStore that only the in-memory store answers precisely.
  2. Two MemoryCacheStoreExpirationTests expectations changed. They asserted an expired entry with no validator stayed "for conditional revalidation" — but with no ETag or Last-Modified there is nothing to revalidate with. The legitimate case they described is covered by RevalidationGraceSeconds, which post-dates them.
  3. Distributed cache upgrades: Vary markers written by earlier versions carry unnormalized field names, so the first lookup of each misses and rewrites it. Self-healing, not corruption — but if that matters, the key prefix can be versioned.

Verification

  • 369 tests green (+20 new, covering bounded reads, buffer reuse, the coordinator's dedup and key-release contract, HEAD content headers, invalidation round-trip counts, and Vary casing/order independence).
  • Release build clean, 0 warnings, net8.0 and net10.0.

One caveat on the shape of this branch: it is a single commit covering seven distinct changes. CachingMiddleware.cs carries interleaved hunks from five of them, so splitting cleanly needed interactive hunk staging. Happy to re-cut it into per-change commits if that would help review.

🤖 Generated with Claude Code

Measured on a 256 KB body with 8 coalesced waiters (median of 40 runs):
8,948,560 B -> 4,748,376 B allocated per miss, 34.1x -> 18.1x the body
size. The saving scales with the number of waiters, since each one used
to pay its own full copy of the body.

Body handling:

- Reuse the buffer the coalescer already materialised instead of reading
  it out again and rebuffering into a second ByteArrayContent. The new
  internal BufferedByteArrayContent lets the caching layer tell an
  already-materialised body from a live network stream.
- Enforce MaxResponseBodyBytes while reading rather than after: a
  declared Content-Length over the limit is rejected before a byte is
  read, and a chunked body is abandoned as soon as it crosses it.
  Previously an oversized response was fully allocated, then rejected.
- Skip buffering entirely when the body exceeds MaxBodySizeBytes, so the
  caller keeps its untouched stream instead of paying for a copy that
  the cache then discards.

Store behaviour:

- Stop retaining entries with no freshness, no stale window and no
  validator: they can neither be served nor revalidated, and without
  MaxCacheSize there is no SizeLimit and therefore no LRU, so they lived
  for the lifetime of the process. The distributed store now skips the
  write too, rather than issuing one Redis deletes straight away via a
  negative TTL.
- Drop the read that preceded every §4.4 invalidation. It only made the
  log and metric count confirmed deletions, at the cost of fetching the
  whole stored body over the network: 2 -> 1 round-trips in the common
  case, 6 -> 3 in the worst. stampede_http.cache.invalidations now counts
  invalidations issued; its description says so.
- Normalize Vary field names once at store time (lower-cased, sorted) so
  each lookup only concatenates, instead of copying, sorting and
  lower-casing them again.

Correctness, found along the way:

- A HEAD served from cache dropped Content-Type and reported
  Content-Length: 0, against RFC 9110 section 9.3.2, because the content
  was replaced wholesale to empty the body.
- stale-while-revalidate deduplication lived on CachingMiddleware, which
  IHttpClientFactory rotates every two minutes and can keep several of
  alive at once, so two chains could revalidate the same key together.
  Moved to a per-client BackgroundRevalidationCoordinator, which also
  claims the key before starting the work: a refresh that finished before
  its own registration landed used to leave the key claimed forever.

Two MemoryCacheStoreExpirationTests expectations changed: they asserted
that an expired entry with no validator stayed "for conditional
revalidation", but there is nothing to revalidate with. The legitimate
case they described is covered by RevalidationGraceSeconds.

Note for distributed deployments: Vary markers written by earlier
versions carry unnormalized field names, so the first lookup of each
misses and rewrites it. Self-healing, not corruption.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@FranRuiz98
FranRuiz98 merged commit 182d564 into master Jul 25, 2026
3 checks passed
@FranRuiz98
FranRuiz98 deleted the claude/gallant-tesla-3b986c branch July 25, 2026 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant