[bgen] Cache raw GetCustomAttributesData() results per provider - #25564
Conversation
MetadataLoadContext's GetCustomAttributesData() returns a fresh ReadOnlyCollection<CustomAttributeData> on every call. Since bgen queries multiple attribute types per provider (via GetCustomAttributes<T>, HasAttribute<T>, HasAttribute(string), IsNullable), the same provider's raw attribute list was being allocated and discarded many times over. Add a Dictionary<ICustomAttributeProvider, IList<CustomAttributeData>> that caches the raw result per provider. Changed GetAttributes() from static to instance to enable this cache. HasAttribute(string) also changed from static to instance (only one caller, already via instance). Baseline average (3 runs): 24.7s, 9164 MB total, gen0=1660, gen1=513 After average (3 runs): 22.1s, 6990 MB total, gen0=1265, gen1=417 Time: -2.6s (-10.5%) Memory: -2174 MB (-23.7%) GC: gen0 -395 (-23.8%), gen1 -96 (-18.7%) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR optimizes bgen’s reflection attribute access by caching GetCustomAttributesData() results per ICustomAttributeProvider, avoiding repeated allocations when multiple attribute queries are performed for the same provider during generation.
Changes:
- Add a per-
AttributeManagercache mappingICustomAttributeProvider→ rawCustomAttributeDatalist. - Convert
GetAttributes()from static to instance so it can use the cache. - Convert
HasAttribute (ICustomAttributeProvider, string)from static to instance (call sites already use anAttributeManagerinstance).
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
✅ [PR Build #5c70999] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #5c70999] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #5c70999] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #5c70999] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 193 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
MetadataLoadContext's
GetCustomAttributesData()returns a freshReadOnlyCollection<CustomAttributeData>on every call. Since bgen queries multiple attribute types per provider (viaGetCustomAttributes<T>,HasAttribute<T>,HasAttribute(string),IsNullable), the same provider's raw attribute list was being allocated and discarded many times over.Add a
Dictionary<ICustomAttributeProvider, IList<CustomAttributeData>>that caches the raw result per provider. ChangedGetAttributes()from static to instance to enable this cache.HasAttribute(string)also changed from static to instance (only one caller, already via instance).