[Perf] Lazy-initialize _triggerSpecificity dictionary on BindableObject - #34133
Conversation
Fixes #34131 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
|
||
| internal ushort _triggerCount = 0; | ||
| internal Dictionary<TriggerBase, SetterSpecificity> _triggerSpecificity = new Dictionary<TriggerBase, SetterSpecificity>(); | ||
| internal Dictionary<TriggerBase, SetterSpecificity> _triggerSpecificity; |
There was a problem hiding this comment.
Would it work to add
#nullable enable
internal Dictionary<TriggerBase, SetterSpecificity>? _triggerSpecificity;
#nullable disable?
Or would it still cascade to more and more changes?
There was a problem hiding this comment.
That should work, but I don't think it has high value right now. We should let copilot annotate the whole class/codebase at some point.
There was a problem hiding this comment.
Pull request overview
This PR optimizes memory allocation in BindableObject by lazy-initializing the _triggerSpecificity dictionary, which is only used by the small fraction of objects (<5%) that actually have triggers attached. This reduces per-instance allocations for the vast majority of MAUI UI elements.
Changes:
- Removed eager initialization of
_triggerSpecificitydictionary inBindableObject - Added null-safe operations at all three access points in
TriggerBase(attach, detach, condition changed) - Added comprehensive benchmarks to measure allocation improvements for objects without triggers
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Controls/src/Core/BindableObject.cs | Removed eager = new Dictionary<...>() initializer from _triggerSpecificity field |
| src/Controls/src/Core/Interactivity/TriggerBase.cs | Added null-coalescing assignment on attach, conditional access on detach and condition query |
| src/Core/tests/Benchmarks/Benchmarks/BindableObjectAllocBenchmarker.cs | New benchmark file covering multiple performance issues including trigger dictionary allocation (#34131) |
StephaneDelcroix
left a comment
There was a problem hiding this comment.
LGTM from my review. I don't see blocking issues; this looks good to merge as-is.
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Fixes #34131
Description
The
Dictionary<TriggerBase, SetterSpecificity>field_triggerSpecificitywas eagerly allocated on everyBindableObjectinstance, but fewer than 5% of objects ever use triggers. This PR makes it lazy-initialized on first use.Changes
BindableObject.cs— remove eager= new Dictionary<...>()initializerTriggerBase.cs— null-safe access at 3 call sites (??=on attach,?.on detach,?.TryGetValueon condition changed)