Cut extension-adapter allocations on the save and load hot paths - #184
Conversation
Saving a 100-item extension-bearing RSS feed was the one scenario where v4000 regressed against v3001 (338 KB vs 258 KB allocated, 0.89x the speed): 25 write-path sites constructed a syndication extension purely to read its XML namespace back, and the modernised extension contexts made each construction ~3x dearer (ten eagerly-initialised collections, plus the Version and parsed Uri every construction always bought). Per-family isolation put the entire regression on YahooMedia, whose write path pays the idiom twice per item. - Add the NamespaceUri constant the Podcast and GeoRSS families already use to the six legacy families that lacked it, referenced from each base(...) call so the constant and the instance property cannot drift, and rewrite all 25 write-path sites to use it. The 8 load-path sites (CreateNamespaceManager) are deliberately untouched. - Stop WriteXmlNamespaceDeclarations running Activator.CreateInstance per supported type per save: framework types reuse the cached probe instances via a FrozenDictionary keyed by concrete type. Safe because WriteXmlNamespaceDeclaration is non-virtual and reads only XmlPrefix and XmlNamespace. - Add the missing extension-bearing save arm to FeedSaveBenchmarks; the existing arms save extension-free feeds, which is exactly where this regression hid. Saved output is byte-identical to the 4000.0.4 package for both the RSS and Atom corpora (cmp). Allocation for the regressed scenario: 338.09 KB to 224.35 KB (-33.6%), now 12.9% below v3001. Full detail and measurement provenance: .endjin/build-warnings.md section 17. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CBG52K8XTzAPk4Vat1pnhp
Extension auto-detection paid its candidate-selection cost per entity even when the answer was "none": a GetNamespacesInScope dictionary (368 B, measured), a candidate list, and a scan of all 27 framework probes against both — 26% of the whole load of a 50,000-URL sitemap that declares no extension namespace at all (build-warnings.md §2.40, recorded there as measured-not-fixed). FillCore now ORs per-key bitmasks — namespace URI -> probe mask and prefix -> probe mask, as lazy FrozenDictionary maps over FrameworkProbes indices — while walking the entity's namespace axis with mutate-and- restore, and consumes matches in ascending bit order, which is FrameworkProbes order, which is the old attachment order and therefore save order. Zero matches allocates nothing; the candidate list is gone; user-supplied SupportedExtensions follow exactly as before. More than 64 framework probes refuses loudly rather than truncating. Verified the way §2.40 demanded: a per-entity extension inventory over the whole 76.2 MiB real corpus plus the synthetic corpora — 115,417 records of attached extension types in attachment order — is byte- identical across the change. Sitemap.Load of the real 50,000-URL document: 77.99 MB to 63.49 MB (-18.6%); plain RSS load -16%; Atom load -18%. A cheaper-looking two-tier walk was built, measured slower, and removed; the experiment and the remaining ~5.7 MB ceiling are recorded in build-warnings.md §18. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CBG52K8XTzAPk4Vat1pnhp
There was a problem hiding this comment.
Pull request overview
Reduces allocations in extension save/load hot paths by eliminating throwaway SyndicationExtension constructions during save and by making load-side auto-detection avoid per-entity namespace dictionaries/candidate lists when no extension namespaces are in scope.
Changes:
- Cache framework probe instances by type for reuse during namespace declaration writing; add namespace/prefix → probe-bitmask lookup for allocation-light auto-detection.
- Introduce
NamespaceUriconstants on multiple extension families and update element writers to use them instead of instantiating an extension just to read.XmlNamespace. - Extend save benchmarks to include an RSS corpus where every item has extensions, so extension-bearing save costs are measured.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Solutions/Argotic.Extensions/SyndicationExtensionAdapter.cs | Adds probe-by-type caching and bitmask-based auto-detection to reduce per-entity allocations. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaUtility.cs | Removes per-call extension instantiation; uses NamespaceUri constant. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaThumbnail.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaTextConstruct.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaText.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaSyndicationExtension.cs | Introduces NamespaceUri constant and uses it in base constructor call. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaRestriction.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaRating.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaPlayer.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaHash.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaGroup.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaCredit.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaCopyright.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaContent.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/YahooMedia/YahooMediaCategory.cs | Uses YahooMediaSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/SiteSummaryContent/SiteSummaryContentSyndicationExtension.cs | Introduces NamespaceUri constant and uses it in base constructor call. |
| Solutions/Argotic.Extensions/Core/SiteSummaryContent/SiteSummaryContentItem.cs | Uses SiteSummaryContentSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/SimpleList/SimpleListSyndicationExtension.cs | Introduces NamespaceUri constant and uses it in base constructor call. |
| Solutions/Argotic.Extensions/Core/SimpleList/SimpleListSort.cs | Uses SimpleListSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/SimpleList/SimpleListGroup.cs | Uses SimpleListSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/Podcast/PodcastSyndicationExtension.cs | Documentation wording tweak (no functional change). |
| Solutions/Argotic.Extensions/Core/LiveJournal/LiveJournalUserPicture.cs | Uses LiveJournalSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/LiveJournal/LiveJournalSyndicationExtension.cs | Introduces NamespaceUri constant and uses it in base constructor call. |
| Solutions/Argotic.Extensions/Core/LiveJournal/LiveJournalSecurity.cs | Uses LiveJournalSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/LiveJournal/LiveJournalMood.cs | Uses LiveJournalSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/iTunes/ITunesSyndicationExtension.cs | Introduces NamespaceUri constant and uses it in base constructor call. |
| Solutions/Argotic.Extensions/Core/iTunes/ITunesOwner.cs | Uses ITunesSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/iTunes/ITunesCategory.cs | Uses ITunesSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/FeedSync/FeedSynchronizationSyndicationExtension.cs | Introduces NamespaceUri constant and uses it in base constructor call. |
| Solutions/Argotic.Extensions/Core/FeedSync/FeedSynchronizationSharingInformation.cs | Uses FeedSynchronizationSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/FeedSync/FeedSynchronizationRelatedInformation.cs | Uses FeedSynchronizationSyndicationExtension.NamespaceUri when writing XML attributes/elements. |
| Solutions/Argotic.Extensions/Core/FeedSync/FeedSynchronizationItem.cs | Uses FeedSynchronizationSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Extensions/Core/FeedSync/FeedSynchronizationHistory.cs | Uses FeedSynchronizationSyndicationExtension.NamespaceUri when writing XML. |
| Solutions/Argotic.Benchmarks/Saving/FeedSaveBenchmarks.cs | Adds RSS-with-extensions benchmark arm and validates corpus actually attaches extensions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| XPathNavigator navigator = this.Navigator; | ||
| if (!navigator.MoveToFirstNamespace(XPathNamespaceScope.ExcludeXml)) | ||
| { | ||
| return 0; | ||
| } | ||
|
|
||
| foreach (ISyndicationExtension extension in SyndicationExtensionAdapter.FrameworkProbes.Value) | ||
| (FrozenDictionary<string, ulong> byNamespace, FrozenDictionary<string, ulong> byPrefix) = ProbeMasks.Value; | ||
| ulong matched = 0; |
There was a problem hiding this comment.
Addressed in c96d71d on the stacked #185 (this method was rewritten there, so the fix lands where the code lives going forward). Rather than a try/finally, both namespace walks now resolve ProbeMasks.Value before the first navigator move — the lazy initializer was the only throwing operation on the path, so hoisting it makes the documented "nothing that can throw runs between move and restore" invariant true again at zero cost, and keeps the no-namespaces early return. Verified behaviour-identical: the 115,417-record extension inventory is byte-for-byte unchanged.
Test Results3 574 tests 3 574 ✅ 30s ⏱️ Results for commit 1ea18be. ♻️ This comment has been updated with latest results. |
…ransport Review on #184 found that MatchFrameworkProbes read ProbeMasks.Value after moving the navigator onto the namespace axis - the lazy initializer is the one thing on that path that can throw (the >64-probe guard), and a throw there would strand the navigator on a namespace node, contradicting the method's restore contract. Rather than the suggested try/finally, both walks (MatchFrameworkProbes and FilterByPresentContent, which inherited the shape) now resolve the lazy BEFORE the first move, making the documented "nothing that can throw runs between move and restore" invariant true again at zero cost. Review on #185 found AsyncLoadBenchmarks.Setup replacing the field-initializer HttpClient/handler instances without disposing them; they are now disposed before reassignment. Verified: extension inventory byte-identical (115,417 records), wikihow 50K and plain-RSS allocations unchanged, all five gates green, async class smoke clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CBG52K8XTzAPk4Vat1pnhp
Benchmarks: four census-calibrated corpus generators and nine new classes (category "gapfill") grow the suite from 261 to 333 cases, covering everything §19's coverage analysis found dark: legacy formats (RSS 0.90-1.0, Atom 0.3, RSD 0.6, incl. the real ISO-8859-1+DOCTYPE wire shape), Podcasting 2.0 at census element ratios, the declared-but-unused production shape, a maximal every-family ceiling, Google sitemap extensions (load and save), BlogML/APML scaling, save breadth for OPML/APML/BlogML/RSD, the per-format CreateAsync surface, and opt-in real-corpus calibration arms. Setup guards encode arm meaning, not just size; benchmark line coverage moves Data 21->57%, Extensions 17->31%, Syndication 29->40%, with the remaining darkness documented. Optimisation: the new instruments' top finding was that declaration-matched extension families ran a full context Load per entity even when no in-namespace content exists - costlier than parsing real payloads on the ~96%-of-production declared-but-unused shape. FillCore now filters candidates through one walk of the entity's child elements (InlineArray stack tracking preserves prefix-rebinding tolerance; a content-map override covers FeedHistory's RFC 5005 atom:links). Declared-but-unused @100 items: 3,105 KB -> 880 KB (-72%), 2,188 us -> 981 us (-55%); a real 18.5 MB podcast feed -40% via childless sub-entities; extension-free paths unchanged. Behaviour proven identical across a 115,417-record extension inventory of the full real corpus. Also: review fixes (lazy mask reads hoisted ahead of navigator moves; benchmark stub transport disposed) and a raw-string-literal rewrite of the corpus generators, proven byte-identical by a 187-hash sweep across every shape-cycle branch.
Summary
Two allocation fixes to the extension machinery's hot paths, produced by re-benchmarking the one regression the v3001↔v4000 package comparison recorded (RSS save with extensions) and then collecting the largest known load-side cost (§2.40's empty-probe finding). Outputs and detection behaviour are proven unchanged; only the allocations moved.
Commit 1 — the save path. 25 write-path sites constructed a throwaway
SyndicationExtensionpurely to read.XmlNamespaceback, and the modernised contexts made each construction ~3× dearer than in v3001 (ten eagerly-initialised collections plus aVersionand parsedUri). All 25 sites now use aNamespaceUriconstant (the idiom the Podcast/GeoRSS families already used), referenced from eachbase(...)call so constant and property cannot drift.WriteXmlNamespaceDeclarationsalso stops runningActivator.CreateInstanceper type per save, reusing the cached probe instances.Commit 2 — the load path. Extension auto-detection paid per entity for a
GetNamespacesInScopedictionary, a candidate list, and a 27-probe scan even on documents declaring no extension namespace at all.FillCorenow ORs per-keyFrozenDictionarybitmasks while walking the entity's namespace axis; zero matches allocates nothing, and matches are consumed inFrameworkProbesorder, preserving attachment order (and therefore save order) exactly.Measured results (allocation; GC-precise instruments, corroborated by BenchmarkDotNet)
Sitemap.Load, real 50,000-URL documentRssFeed.Load, 100 itemsAtomFeed.Load, 100 entriesVerification
cmp).FeedSaveBenchmarksgains the extension-bearing save arm whose absence let the save regression hide..endjin/build-warnings.md§17–§18 (local engineering log).Deliberately not included
CreateNamespaceManagerthrowaway sites and the eager-context-collections design question (§18.4).🤖 Generated with Claude Code
https://claude.ai/code/session_01CBG52K8XTzAPk4Vat1pnhp