Content & Media: Batch cold read-through for Children()/Descendants() traversal - #23358
Open
AndyButland wants to merge 11 commits into
Open
Content & Media: Batch cold read-through for Children()/Descendants() traversal#23358AndyButland wants to merge 11 commits into
AndyButland wants to merge 11 commits into
Conversation
Materialising a set of child/descendant keys previously went one key at a time: FilterAvailable -> per-key GetById -> GetNodeAsync -> a single-row GetContentSourceAsync/GetMediaSourceAsync in its own scope on an L1 miss. For a large, mostly-uncached set (e.g. a flat media library exceeding the seed count) that is one database round trip and one scope per item. Add GetByKeysAsync to IDocumentCacheService/IMediaCacheService (default impl loops the single-key path for back-compat): it does the same per-key L0 and L1/L2 probe as today, then reads only the genuine misses from the database in a single batched query within one scope, applying the published-ancestor guard (documents) and the generation guard exactly as GetNodeAsync does before populating L1/L0. The content/media status filtering services now materialise via a shared slow-start chunked iterator (ChunkedPublishedContentEnumerator): keys are drawn in growing chunks and an all-L0-hit chunk is served fully synchronously (no async, no batch) so the warm path and short-circuiting consumers (FirstChild/FirstOrDefault) are unchanged, while a cold full enumeration collapses its database access into a handful of batched reads. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extends SyntheticPublishedTreeFixture with an unseeded (cold) mode backed by a latency-injecting, round-trip-counting repository that implements both the single and batched reads, plus a ResetColdAsync for per-iteration cold measurement. HybridCacheColdNavigationBenchmarks measures cold Children()/ Descendants()/FirstOrDefault() and reports SingleFetchCount/BatchFetchCount — the hardware-independent signal: a cold traversal issues zero per-item reads and only a handful of batched reads. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds MediaHybridCacheMockTests (MediaCacheService.GetByKeysAsync batches the database read and never calls the single-item read per item, and populates the memory cache) and a media cold-navigation benchmark (SyntheticPublishedMediaTreeFixture + HybridCacheColdMediaNavigationBenchmarks) mirroring the document coverage — the reported workload is a large, flat media library. Confirmed the cold traversal issues zero per-item reads and only a handful of batched reads. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add direct unit tests for ChunkedPublishedContentEnumerator (all-hit, all-miss, FirstOrDefault/Take laziness, predicate, mixed-order, missing-omitted, empty). - Reduce Enumerate cognitive complexity by extracting FillChunk / ResolveChunk / PlaceMisses helpers, and add XML docs to the public method. - Document Document/MediaCacheService.GetByKeysAsync with inheritdoc and route the L0 fast-path check through the shared TryGetCached (removes the complex conditional). - HybridCacheColdMediaNavigationBenchmarks: make the fixed tree shape constants rather than single-value [Params] (clears the analyzer warning). - MediaHybridCacheMockTests: type the media field as Media. - Mark the parameterless-of-instance-state SetupCounting / SetupDocumentCacheService test helpers static. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n benchmarks Brings the document cold-navigation and the original navigation benchmarks in line with the media one: the fixed 50 x 100 tree dimensions are constants rather than single-value [Params], clearing the "single value to [Params] is unnecessary" warning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Return concrete PublishedContentType from BuildTestMediaType / BuildTestContentType (CA1859, concrete return type for performance). - SingleFetchCount / BatchFetchCount doc summaries begin with "Gets" (SA1623). - Hold the CollectionAssert expected arrays as static readonly fields rather than inline literals (CA1861). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets the remaining major perf bottleneck in Children() / Descendants() traversal on large, cold caches by batching the database read-through beneath HybridCache (L1/L2), while preserving lazy enumeration semantics for short-circuiting consumers.
Changes:
- Add additive
GetByKeysAsyncAPIs toIDocumentCacheService/IMediaCacheService, and implement batched miss materialisation in the HybridCache-backed services. - Introduce
ChunkedPublishedContentEnumerator(slow-start, sync-L0-first chunked iterator) and route status-filtering materialisation through it. - Add/extend unit + integration tests plus cold navigation benchmarks + fixtures to validate batching, laziness, and cache-population behavior.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/PublishStatus/PublishedMediaStatusFilteringServiceTests.cs | Updates laziness tests to assert chunked materialisation and “no batching on all-L0 hits”. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/PublishStatus/PublishedContentStatusFilteringServiceTests.cs | Refactors laziness tests to validate chunking/batching and publish-status short-circuiting. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/PublishStatus/ChunkedPublishedContentEnumeratorTests.cs | New unit tests for chunk growth, ordering, omission of missing keys, and predicate behavior. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs | Updates construction of PublishedContentStatusFilteringService for the new dependency. |
| tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/MediaHybridCacheMockTests.cs | New integration tests proving batched repo access and L0 population for media. |
| tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheMockTests.cs | Adds integration tests proving batched repo access and L0 population for documents. |
| tests/Umbraco.Tests.Benchmarks/HybridCacheNavigationBenchmarks.cs | Adjusts benchmark parameterization to fixed constants for the reported workload shape. |
| tests/Umbraco.Tests.Benchmarks/HybridCacheColdNavigationBenchmarks.cs | New cold-cache benchmark for content traversal, including latency modeling and fetch counters. |
| tests/Umbraco.Tests.Benchmarks/HybridCacheColdMediaNavigationBenchmarks.cs | New cold-cache benchmark for media traversal, mirroring the content benchmark. |
| tests/Umbraco.Tests.Benchmarks/Fixtures/SyntheticPublishedTreeFixture.cs | Extends fixture to support seeded vs cold runs and to count single vs batched repo fetches. |
| tests/Umbraco.Tests.Benchmarks/Fixtures/SyntheticPublishedMediaTreeFixture.cs | New media fixture supporting cold/seeded runs with fetch counters and latency injection. |
| src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs | Implements GetByKeysAsync batching for media with generation guard and L0/L1 population rules. |
| src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs | Implements GetByKeysAsync batching for documents, including published-ancestor guard parity. |
| src/Umbraco.Core/Services/PublishStatus/PublishedMediaStatusFilteringService.cs | Switches media key materialisation to chunked enumerator + batched cache-service reads. |
| src/Umbraco.Core/Services/PublishStatus/PublishedContentStatusFilteringService.cs | Switches content key materialisation to chunked enumerator + batched cache-service reads with culture predicate. |
| src/Umbraco.Core/Services/PublishStatus/ChunkedPublishedContentEnumerator.cs | New shared chunked enumerator enabling sync warm path + batched cold materialisation. |
| src/Umbraco.Core/PublishedCache/IMediaCacheService.cs | Adds additive GetByKeysAsync default implementation for back-compat. |
| src/Umbraco.Core/PublishedCache/IDocumentCacheService.cs | Adds additive GetByKeysAsync default implementation for back-compat (with optional preview). |
AndyButland
marked this pull request as ready for review
July 13, 2026 12:01
This comment was marked as outdated.
This comment was marked as outdated.
AndyButland
marked this pull request as draft
July 13, 2026 12:04
AndyButland
marked this pull request as ready for review
July 13, 2026 19:03
|
Claude finished @AndyButland's task in 1s —— View job I'll analyze this and get back to you. |
…iss-for-content-collections
…iss-for-content-collections
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Background
#22742 (closing #22646, shipped in 17.5) improved
IPublishedContent.Children()/Descendants()traversal, and a follow-up (#23356) ported the two media-specific gaps across. This PR addresses a remaining cause of the originally reported slowness on large document collections or media libraries: the per-item database read-through when the cache is cold.Seeding the cache on start-up already uses batched updates (see #19890). At runtime, when the particular key isn't in the case (i.e. wasn't seeded), look-ups are one-at-a-time.
The problem
Materialising a set of child/descendant keys goes one key at a time:
FilterAvailable→ per-keyGetById→GetNodeAsync→ on an L1/L2 miss, a single-rowGetContentSourceAsync/GetMediaSourceAsyncin its own scope. For a large, mostly-uncached set — e.g. a flat media library well past the seed count, on first traversal or after a restart / cache clear — that is one database round trip and one scope per item. In the report with 1,300 media items ≈ 1,300 sequential round trips; on a latency-bound database that is reported as minutes.Why this approach
The obvious fix — "ask the cache for many keys at once" — isn't available:
Microsoft.Extensions.Caching.Hybrid(10.7.0) exposes no multi-key/batch read and no batch factory;GetOrCreateAsyncis strictly per-key. So there is no in-architecture way to batch the L1/L2 lookup, and HybridCache (L1/L2) rightly stays the cache of record. The only lever for the cold path is the database fetch beneath HybridCache.In this PR we do:
GetNodeAsyncno-DB probe — same probe count, nothing new against the cache).GetContentSourcesAsync/GetMediaSourcesAsync), with the published-ancestor guard (documents) and the stale-set generation guard applied exactly as the single-item path does.Laziness is preserved with a slow-start chunked iterator (
ChunkedPublishedContentEnumerator): keys are drawn in growing chunks (1, 2, 4, … capped), and an all-L0-hit chunk is served fully synchronously — no async state machine, no batch call — so the warm path and short-circuiting consumers (FirstChild(),Descendant().FirstOrDefault()) behave exactly as before. A cold full enumeration collapses its database access into a handful of batched reads; cold over-fetch on a predicate short-circuit (e.g. via a.Take(...)is bounded to ~2× of what is drawn.Changes
GetByKeysAsyncadded toIDocumentCacheService/IMediaCacheService(additive, default implementation loops the single-key path for back-compat), implemented in the two cache services mirroringGetNodeAsync.ChunkedPublishedContentEnumerator— the shared slow-start, sync-L0-first iterator.Benchmark results
BenchmarkDotNet, InProcessEmit toolchain, .NET 10.0.9, Intel Core i5-10300H, synthetic tree of 50 × 100 ≈ 5,051 nodes, cache cold. Baseline = pre-change per-key path; After = this PR. Benchmarks and the cold fixture are included undertests/Umbraco.Tests.Benchmarks(*ColdNavigation*).Database round trips per cold traversal (deterministic; this is the figure that maps to the report):
Descendants()(~5,050 items)Children()(50 items)FirstChild()/Descendants().FirstOrDefault()At a database round-trip latency L, cold
Descendants()drops from ≈5,050·L to ≈27·L (~187× fewer round trips) — identical for documents and media.Cold allocation (latency 0, pure CPU/allocation): cold
Descendants()allocation drops ≈25–39% (media 25.3 → 15.4 MB; document 27.4 → 20.6 MB);Children()similarly (media 266 → 186 KB).Warm path — no regression: warm (seeded-cache)
Children(),Descendants(),FirstOrDefault()and the recursive traversal are all within measurement noise on time; the large warmDescendants()case allocates ~40% less.FirstChild()/FirstOrDefault()still materialise exactly one item.End-to-end verification (running site)
Beyond the micro-benchmarks, the change was verified against a running Umbraco instance backed by a local SQL Server database, on a node with ≈ 69,895 children.
Paged listing (1000 per page)
Aa template that renders one page of
Model.Children()(1,000 per page) was setup with timing for the page retrieval. Each set is a fresh site restart (cold converted cache), so load 1 is a cold page retrieval and loads 2–5 are warm; three restart-sets were recorded.Warm loads (2–5) measured ~1–2 ms on both branches — unchanged.
Conclusion
Rendering a cold 1,000-item page dropped from ~1.4 s to ~0.15 s — roughly 9× faster (−89%), with the warm path unaffected. The measured gain is the eliminated per-item round trips: on the per-key path the 1,000 page rows are read one at a time, cold from disk, whereas the batched path replaces them with a few
WHERE INreads (the "Ordered listing" section below isolates and confirms this with a controlled cold/warm buffer pool). The saved round trips dominate whenever each is expensive — a cold buffer pool, or a remote / higher-latency SQL Server — which is the "1,300 media items ≈ 10 minutes" case that prompted this work. Net: the cold read-through is materially faster, while the warm path and short-circuiting laziness are preserved.Ordered listing (all children by name, first 10)
A second template variant orders all children by name and lists the first 10 (
Model.Children().OrderBy(c => c.Name).Take(10)). Unlike paging, this deliberately forces the entire child set (~69,895 items) to be materialised and sorted — there is no short-circuit, so it is the worst case for this change: every child must be fetched regardless.A first pass showed the two branches as roughly equal here, which turned out to be a measurement artefact. Restarting the site clears Umbraco's own caches (the converted-content L0 and HybridCache's in-memory L1) but not SQL Server's buffer pool — a separate process that is never restarted. Earlier full-tree reads had already paged all of
cmsContentNuinto that buffer pool, so both branches were reading ~70k rows from memory inside SQL Server (~0.1 ms/row) and there was almost no round-trip cost left for the batching to remove.Re-run with the buffer pool explicitly flushed (
CHECKPOINT; DBCC DROPCLEANBUFFERS) after boot but before the cold load, so load 1 is genuinely cold at both the app caches and the database — the state a server is in after a cold start, or permanently for a working set larger than the buffer pool. Local SQL Server, ~69,895 children, Debug, fresh restart + buffer flush per set:Warm loads (2–5, served from L0) measured ~150 ms on both branches — unchanged. (One baseline cold set was discarded as
ERR— the one-off Razor view compilation on the very first request after a fresh build.)Conclusion
With a genuinely cold buffer pool the per-key path pays ~70,000 random single-row reads from disk (~1 ms each → over a minute), while the batched path collapses them into a handful of
WHERE INreads and lands at ~10.4 s — roughly 6.3× faster (−84%) on the full-materialisation worst case. The two are equal only when the database is already serving from memory. The batching's advantage is precisely the eliminated round trips, and it scales with how expensive each round trip is: large under cold-disk I/O (above) and under network latency to a remote database (the reporter's "1,300 items ≈ 10 minutes"), negligible when the working set is hot in the buffer pool. The warm path (L0/L1 hit) is unchanged, so this is a pure win with no regression.Testing
Added unit + component tests (batched read verified, single-item read never called per item, laziness and warm short-circuit preserved) for documents and media; solution builds and CI checks should pass.