Skip to content

perf(cache)!: read by the key's text wherever a local hit can answer - #221

Merged
cosmin-staicu merged 7 commits into
mainfrom
perf/span-reads-everywhere
Oct 6, 2026
Merged

cosmin-staicu merged 7 commits into
mainfrom
perf/span-reads-everywhere

Conversation

@cosmin-staicu

Copy link
Copy Markdown
Member

Reads by the key's text wherever a local hit can answer, so a warm read allocates nothing in two more places: the remaining read operations by span, and every read by a plain string key.

What changes

Span overloads for the remaining reads.

  • SpanKeyExtensions adds ContainsAsync and GetOrAddAsync (all expiration overloads) over ICache, ICache<T>, IHashCache and IHashCache<T>.
  • It adds GetCacheEntryAsync over ICache, IHashCache and IHashCache<T>.
  • They dispatch through new members on the four capability interfaces, as GetAsync does, so Moq and NSubstitute mocks keep answering from their CacheKey setups.

How a span GetOrAddAsync behaves.

  • A local hit is answered by span.
  • Anything else builds the key once and takes the CacheKey path. The local lookup never touches the distributed tier, so a miss costs no extra round trip.
  • A rehydrating policy goes straight to the CacheKey path, since rehydration keeps the key.

String keys read the local tier by their text (.NET 9+).

  • The multilayer caches' CacheKey reads try the local tier by CacheKey.Name first: GetAsync, GetItemAsync, ContainsAsync, GetCacheEntryAsync and GetOrAddAsync.
  • Cache<T> and HashCache<T> compose their strategy's key on the stack instead of through GetCacheKey.
  • So typed.GetAsync("user:42") over a prefix strategy no longer allocates on a local hit, with no call-site change.
  • A key built with a casing other than CacheKey.DefaultCasing stays on the key path, because the span path normalizes with the default.

Two closures off the hit path.

  • GetOrAddInternalAsync was an async method whose miss-path lock delegates capture its parameters, so every hit allocated their closure. It is now a sync wrapper over the async core.
  • TryRehydrate likewise built its closure before its early returns. The trigger now lives in its own method.
  • This was 208 bytes per GetOrAddAsync hit on the plain CacheKey path, even without rehydration.

Behaviour kept

Each fast path returns what the key path returns, or steps aside:

  • Disconnected tier, Trace logging, a declining strategy, an overlong key, a cancelled token: fall back to the key path, so exception and cancellation behaviour is unchanged.
  • .NET 8: has no span lookup on MemoryCache, so it takes the path it took before.

Breaking

The capability interfaces gain members. An implementation outside the library adds them, and forwarding to its CacheKey overloads keeps its behaviour.

Tests

SpanKeyOperationTests covers:

  • the new span operations against the key forms, over real in-memory tiers, prefix strategies, proxies and the null caches
  • case-sensitive keys
  • a declining strategy
  • rehydrate on a local hit, by span and by key
  • cancellation
  • zero allocation for warm string-key and span reads (.NET 9+)

Each guard was red-checked: dropping the casing checks (multilayer, typed, hash, typed hash), skipping rehydrate on either path, or turning off the typed stack composition fails at least one test.

Full suite: net10.0 2153 passed, net8.0 2122 passed.

SpanKeyExtensions gains ContainsAsync and GetOrAddAsync over ICache,
ICache<T>, IHashCache and IHashCache<T>, and GetCacheEntryAsync over
ICache and both hash surfaces, through new members on the four
capability interfaces. A span GetOrAddAsync answers a local hit by span
and otherwise builds the key and takes the CacheKey path, so a miss
costs no second round trip; under a rehydrating policy it takes that
path directly, since rehydration keeps the key.

On .NET 9 and later the CacheKey reads (GetAsync, GetItemAsync,
ContainsAsync, GetCacheEntryAsync, GetOrAddAsync) of the multilayer
caches look the local tier up by the key's text first, and Cache<T> and
HashCache<T> compose their strategy's key on the stack instead of
through GetCacheKey. A string key through a typed cache with a prefix
strategy no longer allocates on a local hit. A key built with a casing
other than CacheKey.DefaultCasing stays on the key path, since the span
path normalizes with the default.

GetOrAddAsync's hit path is no longer async, and TryRehydrate hands the
trigger to a separate method: both used to allocate their lambdas'
closure on every hit, rehydrating or not.

BREAKING CHANGE: ISpanKeyCache, ISpanKeyCache<T>, ISpanKeyHashCache and
ISpanKeyHashCache<T> gain members; an implementation outside the library
adds them, forwarding to its CacheKey overloads keeps its behaviour.

Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>
@cosmin-staicu
cosmin-staicu requested a balanced review from Copilot October 6, 2026 14:34
@github-actions github-actions Bot added the needs-cla-review A maintainer should assess whether a signed CLA is required (see CONTRIBUTING.md) label Oct 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

🔎 Maintainer heads-up: automated triage flagged this PR as potentially material, so it may need a signed CLA in addition to the DCO sign-off.

Strong signals

  • adds public API surface (PublicAPI.Unshipped.txt in src/UiPath.Caching.Abstractions, src/UiPath.Caching)

Other signals

  • large production change (+857 lines under src/)

This is advisory only — the bot does not decide. Please judge against the CLA criteria (material, product-critical, patent-sensitive, corporate contributor, broad commercial use). Note that thresholds can be gamed by splitting PRs, so use your judgement.

  • If a CLA is needed → add the cla-required label (a contributor comment with signing steps is posted automatically).
  • If it is not needed → replace needs-cla-review with cla-not-required so later pushes don't re-flag it.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Warm span get-or-add hits bypass validation of explicit durations and deadlines.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
What changed in this PR

Optimizes warm local cache reads on .NET 9+ while expanding span-based read APIs.

Changes:

  • Adds span overloads for contains, entry, and get-or-add operations.
  • Uses stack-composed keys and allocation-free local-hit paths.
  • Adds tests, API declarations, and documentation.
File Description
tests/​UiPath.Caching.Tests/​SpanKeyOperationTests.cs Tests behavior and allocations.
tests/​UiPath.Caching.Tests/​Fakes/​SpanReads.cs Adds span test helpers.
src/​UiPath.Caching/​PublicAPI.Unshipped.txt Records concrete public APIs.
src/​UiPath.Caching/​MultilayerHashCache.SpanRead.cs Adds hash local-read helpers.
src/​UiPath.Caching/​MultilayerHashCache.cs Optimizes hash reads and get-or-add.
src/​UiPath.Caching/​MultilayerCache.SpanRead.cs Adds cache local-read helpers.
src/​UiPath.Caching/​MultilayerCache.cs Optimizes cache reads and get-or-add.
src/​UiPath.Caching/​HashCacheOfT.cs Adds typed hash span composition.
src/​UiPath.Caching/​CacheOfT.cs Adds typed cache span composition.
src/​UiPath.Caching.Abstractions/​SpanKeyExtensions.cs Exposes new span extensions.
src/​UiPath.Caching.Abstractions/​PublicAPI.Unshipped.txt Records abstraction APIs.
src/​UiPath.Caching.Abstractions/​NullHashCache.cs Implements hash span capabilities.
src/​UiPath.Caching.Abstractions/​NullCache.cs Implements cache span capabilities.
src/​UiPath.Caching.Abstractions/​ISpanKeyHashCacheOfT.cs Expands typed hash capability.
src/​UiPath.Caching.Abstractions/​ISpanKeyHashCache.cs Expands hash capability.
src/​UiPath.Caching.Abstractions/​ISpanKeyCacheOfT.cs Expands typed cache capability.
src/​UiPath.Caching.Abstractions/​ISpanKeyCache.cs Expands cache capability.
docs/​reference/​interfaces.md Documents span-read behavior.
docs/​how-to/​telemetry-and-strategies.md Updates strategy guidance.
CHANGELOG.md Records API and performance changes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/UiPath.Caching/MultilayerCache.cs
Comment thread src/UiPath.Caching/MultilayerHashCache.cs
…lookup

Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The broad public API expansion and version-specific cache fast paths warrant final human validation despite comprehensive tests.

Review effort: Balanced
Findings: None

Resolved since last review (2)

Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The broad public API and runtime-specific cache-path changes warrant final human validation despite strong test coverage.

Review effort: Balanced
Findings: None

Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

It changes breaking public capability interfaces and allocation-sensitive behavior across several cache layers, warranting final human validation.

Review effort: Balanced
Findings: None

…their tests

Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>
@cosmin-staicu cosmin-staicu added cla-not-required Maintainer reviewed: no CLA required for this contribution and removed needs-cla-review A maintainer should assess whether a signed CLA is required (see CONTRIBUTING.md) labels Oct 6, 2026
@cosmin-staicu cosmin-staicu self-assigned this Oct 6, 2026
…ches explicitly

Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Warm GetOrAdd fast paths can bypass non-cacheable-type validation.

Review effort: Balanced
Findings: None

Previously missed (4)

In code that hasn't changed since last review

Medium severity Validate cacheability before span local-cache lookup

src/​UiPath.Caching/​MultilayerCache.SpanRead.cs:49

The span local-hit branch can return before NotCacheableException.ThrowIfNotCacheable<T>(), unlike the corresponding CacheKey operation. With a custom IMemoryCacheFactory retaining a MemoryCache that contains an ICacheEntry<int>, GetOrAddAsync<int>(Span<char>, ...) now returns that entry instead of rejecting the non-cacheable type. Validate T before probing L1.

Medium severity Validate non-cacheable types before warm-hit return

src/​UiPath.Caching/​MultilayerCache.cs:363

Moving the type validation into GetOrAddCoreAsync lets this newly added warm-hit return bypass it. That changes the ICache contract for non-cacheable value types whenever a custom memory-cache factory exposes a pre-populated local entry. Move the validation into this synchronous wrapper before the local probe; the async core no longer needs to repeat it.

Medium severity Run type validation before span fast-path lookup

src/​UiPath.Caching/​MultilayerHashCache.SpanRead.cs:64

This span fast path can return a warm hash entry without running the non-cacheable-type validation performed by the CacheKey path. A retained/custom MemoryCache can therefore make GetOrAddAsync<int>(Span<char>, ...) succeed even though hash-cache operations reject int. Validate T before the local lookup.

Medium severity Move validation before CacheKey warm-cache probe

src/​UiPath.Caching/​MultilayerHashCache.cs:396

The warm CacheKey branch now precedes the NotCacheableException check left in GetOrAddCoreAsync, so a local entry can bypass the hash cache's type contract. Move that validation into this wrapper before probing L1, then remove it from the core.

… the local tier

Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The fast paths preserve validation, cancellation, casing, rehydration, and fallback behavior with comprehensive coverage.

Review effort: Balanced
Findings: None

@sonarqubecloud

sonarqubecloud Bot commented Oct 6, 2026

Copy link
Copy Markdown

@cosmin-staicu
cosmin-staicu merged commit 6379e45 into main Oct 6, 2026
10 checks passed
@cosmin-staicu
cosmin-staicu deleted the perf/span-reads-everywhere branch October 6, 2026 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-not-required Maintainer reviewed: no CLA required for this contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants