[net11.0] Improve TypedBinding performance

[simonrozsival]

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!

Description of Change

This PR improves source-generated TypedBinding performance by changing how generated bindings describe and subscribe to property-change handlers.

The old generated path eagerly allocated a fixed Tuple<Func<TSource, object>, string>[] handler array. The new generated path uses:

• handlersCount - a compile-time count used to size listener storage.
• GetHandlers(TSource source) - a lazy iterator that yields only the INotifyPropertyChanged sources that actually exist for the current binding source.

This lets source-generated bindings avoid eager handler-array allocation and skip handler work for path parts that cannot raise PropertyChanged.

Key Technical Details

Runtime TypedBinding

• Adds a new internal TypedBinding<TSource, TProperty> constructor that accepts handlersCount and GetHandlers.
• Keeps the legacy constructor path for compatibility with existing manually-created typed bindings.
• Uses a PropertyChangeHandler/PropertyChangeListener path for generated bindings.
• Subscribes on each apply so changed intermediate objects are re-subscribed, while listener reuse keeps unchanged subscriptions idempotent.
• Avoids the weak delegate regression by having PropertyChangeListener subscribe directly and weakly reference the owning PropertyChangeHandler.

C# BindingSourceGen

• Emits the new TypedBinding constructor shape.
• Determines whether each path segment definitely, maybe, or cannot implement INotifyPropertyChanged.
• Emits direct handlers for definitely-INPC types and runtime is INotifyPropertyChanged checks for maybe-INPC types.
• Skips dead handler variables when no subsequent path part can yield a handler.
• Emits unsafe getters when a generated setter needs a non-last inaccessible property getter, even if no later handler is generated.

XAML SourceGen

• Emits the same handlersCount + GetHandlers constructor shape.
• Handles member, indexer, conditional-access, nullable, and cast paths consistently.
• Counts indexer handlers via CountIndexAccessHandlers(...) so both default member notifications (for example Item) and index-specific notifications (for example Item[2]) are subscribed when both are generated.

Review Fixes Included

The latest review feedback is addressed in this PR:

• Fixed under-counted XAML indexer handlers in both normal and conditional-access maybe-INPC indexer branches.
• Fixed missing UnsafeAccessor getter emission for writable bindings whose setter still needs a non-last inaccessible getter.
• Fixed GC-timing-sensitive listener loss by removing the weakly-held method-group delegate path.
• Fixed duplicate same-source listener application for indexer handlers that listen to both Item and Item[n].
• Kept PropertyChangeHandler using primary-constructor style.
• Fixed the benchmark protected readonly typo.

Why the Diff Is Large

The PR currently shows about +3990 / -871 lines across 29 files. Most of that churn is validation and benchmark coverage, not product implementation.

Category	Approx. diff	Main contributors
Tests	+2745 / -616	New TypedBinding2UnitTests.cs (+2094), source-generator integration/snapshot updates
Benchmarks	+331 / -83	Expanded typed-binding setup/steady-state benchmark coverage
Implementation + API	+914 / -172	TypedBinding, C# BindingSourceGen, XAML SourceGen, path analysis helpers, PublicAPI

The largest single file is TypedBinding2UnitTests.cs, which duplicates the existing typed-binding behavior matrix against the new generated-binding constructor path. Reducing that further would require a broader test harness refactor to share the old/new constructor matrix; that can be done separately, but this PR keeps the coverage explicit so the runtime behavior comparison is easy to review.

Benchmark Results

Latest local run:

dotnet run -c Release --project src/Core/tests/Benchmarks/Core.Benchmarks.csproj \
  -p:IncludeIosTargetFrameworks=false \
  -p:IncludeAndroidTargetFrameworks=false \
  -p:IncludeMacCatalystTargetFrameworks=false \
  -p:IncludeWindowsTargetFrameworks=false \
  -p:IncludeTizenTargetFrameworks=false \
  -- --filter '*BindingComparisonBenchmarker*' '*BindingBenchmark_*'

Environment: BenchmarkDotNet 0.13.10, macOS 26.5, Apple M1 Max, .NET SDK 11.0.100-preview.5.26302.115.

Steady-state (property value already applied)

Method	Mean	Ratio	Allocated
SetValue	37.82 ns	1.00	40 B
Binding	106.16 ns	2.91	128 B
TypedBinding (old)	78.23 ns	2.12	64 B
TypedBinding2 (new)	73.53 ns	2.03	64 B
SourceGeneratedBinding	73.63 ns	2.01	64 B

Setup path (binding creation + first apply)

Benchmark	TypedBinding2 (new)	TypedBinding (old)	Binding	New/Old Ratio
Name	838.2 ns / 736 B	2.039 us / 888 B	2.757 us / 1,064 B	0.43x
NameTwoWay	836.6 ns / 736 B	2.008 us / 888 B	2.482 us / 1,064 B	0.42x
Child	1.358 us / 1.08 KB	3.329 us / 1.43 KB	2.830 us / 1.61 KB	0.43x
ChildTwoWay	1.187 us / 1.08 KB	2.505 us / 1.43 KB	2.904 us / 1.61 KB	0.47x
ChildIndexed	1.599 us / 1.08 KB	3.499 us / 1.7 KB	7.620 us / 2.56 KB	0.47x
ChildIndexedTwoWay	1.117 us / 1.08 KB	3.096 us / 1.7 KB	5.628 us / 2.56 KB	0.36x

The new constructor path is about 2.1-2.8x faster on setup than the old TypedBinding constructor path and allocates less. In the steady-state property-change path, TypedBinding2 is about 6% faster than old TypedBinding with the same allocation, while source-generated binding is effectively tied with TypedBinding2 in this run.

Tests

• BindingSourceGen unit/integration coverage for generated handlers, unsafe accessors, nullable/cast paths, and source-generator interactions.
• XAML SourceGen coverage for compiled binding handler counts, indexers, nullable paths, and snapshots.
• Core typed-binding coverage for the new constructor path, including subscription lifetime after garbage collection.
• Benchmark coverage comparing old typed binding, new typed binding, regular binding, and source-generated binding paths.

Issues Fixed

No tracked issue.

[bcaceiro]

Just awesome. This will have a huge impact on all apps!

[Copilot]

Pull Request Overview

This PR refactors the TypedBinding<TSource, TProperty> class to introduce a new, more efficient constructor and internal architecture for handling property change notifications. The main changes include:

• Introduces a new constructor that accepts a handlers function instead of a pre-built array, improving performance by deferring handler evaluation
• Adds internal IPropertyChangeHandler interface with two implementations: LegacyPropertyChangeHandler (maintains backward compatibility) and PropertyChangeHandler (new efficient implementation)
• Updates benchmark code to test both old and new approaches
• Adds comprehensive unit tests for the new constructor in TypedBinding2UnitTests.cs
• Removes the TypedBinding.ForSingleNestingLevel factory method and updates its single usage

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
TypedBinding.cs	Core refactoring: removes static factory, adds new constructor with handler function parameter, introduces handler abstraction with two implementations
TypedBindingBenchmarker.cs	Expands benchmarks to compare old vs new binding approaches across multiple scenarios (simple, nested, indexed, two-way)
TypedBinding2UnitTests.cs	New comprehensive test file covering all binding scenarios with the new constructor
TypedBindingUnitTests.cs	Updates reflection code to access handlers through new _propertyChangeHandler field
TemplatedItemsList.cs	Updates single usage of removed factory method to use new constructor directly
BindingExpressionHelper.cs	Minor nullable annotation fix

Comments suppressed due to low confidence (1)

src/Controls/tests/Core.UnitTests/TypedBinding2UnitTests.cs:1

• This reflection code accesses fields on what appears to be the new PropertyChangeHandler class, but that class doesn't have a _handlers field - it has _listeners. This code will fail at runtime if the new constructor path is taken. The test should be updated to work with both handler implementations or check which implementation is in use.

using System;

[simonrozsival]

/azp run MAUI-UITests-public

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[simonrozsival]

@copilot there as a UI test failing and I am not sure if it can be related to these changes or not:

Failed VerifyHybridWebView_EvaluateJavaScriptWithDifferentHybridRoot [19 s]
  Error Message:
     JavaScript evaluation should return the correct title for HybridWebview1.
Assert.That(result, Is.EqualTo("EvaluateJavaScriptAsync Result: HybridWebView1"))
  Expected string length 46 but was 32. Strings differ at index 32.
  Expected: "EvaluateJavaScriptAsync Result: HybridWebView1"
  But was:  "EvaluateJavaScriptAsync Result: "
  -------------------------------------------^

  Stack Trace:
     at Microsoft.Maui.TestCases.Tests.HybridWebViewFeatureTests.VerifyHybridWebView_EvaluateJavaScriptWithDifferentHybridRoot() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/HybridWebViewFeatureTests.cs:line 80

1)    at Microsoft.Maui.TestCases.Tests.HybridWebViewFeatureTests.VerifyHybridWebView_EvaluateJavaScriptWithDifferentHybridRoot() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/HybridWebViewFeatureTests.cs:line 80

Analyze this scenario and correlate it to the changes in this PR. Suggest improvements.

[Copilot]

@simonrozsival I've opened a new pull request, #32642, to work on those changes. Once the pull request is ready, I'll request review from you.

[simonrozsival]

@StephaneDelcroix I think it would be better to introduce this only in .NET 11 and not in .NET 10 SR. We might need to wait until there's a net11 branch.

I also noticed I have not added the new public constructor to the Unshipped Public API documents... if this goes into .NET 11, I will do that in a separate PR.

[simonrozsival]

/rebase

[StephaneDelcroix]

I think it would be better to introduce this only in .NET 11 and not in .NET 10 SR. We might need to wait until there's a net11 branch.

I also noticed I have not added the new public constructor to the Unshipped Public API documents... if this goes into .NET 11, I will do that in a separate PR.

you can wrap this code in #if NET_11_0_OR_GREATER

[simonrozsival]

/azp run MAUI-UITests-public

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[simonrozsival]

/azp run

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

[simonrozsival]

/rebase

[lucastitus]

Hi Simon,

Thanks for the update! I wanted to highlight a regression introduced by this change that affects TypedBinding with nested property paths.

In previous MAUI versions, TypedBinding would re-establish INotifyPropertyChanged subscriptions whenever any object along the binding path changed, even if the binding had already been initialized once. This is essential for scenarios where the parent object starts as null, later becomes non-null, and then one of its nested objects changes.

With this PR, when the binding engine detects that a subscription has already been created during binding initialization, it skips re-subscribing to PropertyChanged on updated source objects. As a result:

• If the root or any intermediate object in the binding path starts as null,
• TypedBinding resolves the path once, does not subscribe to the deeper PropertyChanged watchers,
• and then never updates when nested properties change later.

Changes:
6ce4e0f#diff-bee8ab0ca9d7b2fdc63c4a340be85c32c8fc33bd437e1e331a93dd84c77d19f3R277

Thanks!

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 32382

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 32382"

[lucastitus]

@simonrozsival I think this should also be merged into main/inflight branches?

[simonrozsival]

@lucastitus I don't intend to backport this to .NET 10 servicing release unless there's a good reason to do so.

[lucastitus]

@simonrozsival You've already merged your initial commits: acc2476

However you did not merge the recent null intermediary binding fix I flagged. Any upcoming .NET 10 servicing releases will have the binding issues mentioned currently.

[lucastitus]

@simonrozsival As expected, this regression has made its way into release 10.0.50

[simonrozsival]

@lucastitus can you please open an issue with a repro project? or is there one open already?

/cc @StephaneDelcroix

[simonrozsival]

@StephaneDelcroix @jfversluis could you please have a look at this PR and let me know what you think about it? could we merge this into net11.0 soon?

[MauiBot]

⚠️ Merge Conflict Detected — This PR has merge conflicts with its target branch. Please rebase onto the target branch and resolve the conflicts.

[kubaflo]

/review -b feature/refactor-copilot-yml

[MauiBot]

⚠️ Merge Conflict Detected — This PR has merge conflicts with its target branch. Please rebase onto the target branch and resolve the conflicts.

[kubaflo]

AI code review for net11.0 target

Automated, non-approval review comment. This does not constitute a human approval and intentionally does not use GitHub's Approve/Request-changes.

Verdict: Needs changes (code itself looks good; blocked on rebase + CI re-validation)

What this PR does

Reworks TypedBinding<TSource,TProperty> property-change subscription to avoid the per-handler Tuple<Func<TSource,object>,string>[] allocations: a new public ctor takes (getter, setter, int handlersCount, Func<TSource, IEnumerable<(INotifyPropertyChanged?, string)>> handlers), backed by an IPropertyChangeHandler strategy (PropertyChangeHandler for the new path, LegacyPropertyChangeHandler for the old array path). BindingSourceGen now emits INPC-awareness (DefinitelyImplementsINPC/MaybeImplementsINPC) so subscriptions are only generated where needed.

Findings

• ✅ API surface is additive. New ctor added to Microsoft.Maui.Controls.Internals.TypedBinding<,> Unshipped; the old ctor is retained (kept for back-compat / generated assemblies) and merely added to eng/BannedSymbols.txt so internal callers prefer the allocation-free overload. Good migration hygiene.
• ✅ Dispatcher null-safety is correct. PropertyChangeHandler.OnPropertyChanged uses (sender as BindableObject)?.Dispatcher then dispatcher.DispatchIfRequired(...), which is an extension on IDispatcher? that funnels through EnsureDispatcher — same established pattern as the legacy PropertyChangedProxy. Non-BindableObject INPC sources (the common case) won't NRE.
• ✅ Subscription reuse logic (TryGetSource + ReferenceEquals short-circuit, trailing Unsubscribe(startIndex: index)) correctly avoids redundant re-subscription and cleans stale listeners when the source yields fewer parts. Weak listeners (WeakPropertyChangedProxy) preserve the no-leak behavior.
• ✅ Strong test coverage updated (TypedBinding/TypedBinding2 unit tests, BindingSourceGen unit tests, benchmarks) and the Clone() path is preserved through both handler strategies.
• ⚠️ Merge state is CONFLICTING/DIRTY against net11.0. Needs a rebase before it can merge.
• ⚠️ Minor: _handlers(source) is invoked per Subscribe, allocating an enumerator each time — acceptable since subscribe is not a steady-state hot path, but worth a benchmark sanity check vs. the array path for shallow bindings.

CI note

Last build (1344637) is stale and predates current net11.0; Pack macOS and Helix Windows Unit Tests (Debug/Release) are red. Pack-macOS failing is almost certainly infra (unrelated to binding code), but the Helix unit-test legs touch exactly what this PR changes and the timeline has expired, so I can't confirm them as unrelated. Please rebase and require a fresh green run — especially the Helix unit tests — before merge.

Confidence: Medium-high on the code; low on CI (stale/conflicting).

[kubaflo]

AI cross-model review — PR #32382 ([net11.0] Improve TypedBinding performance)

Verdict: NEEDS_CHANGES (non-approval, automated synthesis of 4 independent model reviews; does not use GitHub Approve/Request-changes)

All four models independently returned NEEDS_CHANGES. The PR reworks TypedBinding<TSource,TProperty> to a new (getter, setter, handlersCount, GetHandlers) constructor backed by a lazy PropertyChangeHandler, and teaches both source generators (BindingSourceGen C# and CompiledBindingMarkup XAML) to emit INPC-aware handlers — a real 2–3× steady-state win. Synthesis kept 4 code-confirmed correctness bugs and dropped 2 lower-value items (a per-notification Action allocation in OnPropertyChanged and a _listeners[i] = null micro-opt) as non-blocking.

Key findings (all validated against HEAD b687e4c)

• error · CompiledBindingMarkup.cs:695 (consensus 3/4) — MaybeImplementsINPC index branch emits two yield returns but increments handlersCount once → _listeners undersized → index-specific (Item[n]) listener silently dropped. Parallel DefinitelyImplementsINPC branch increments twice. Precise fix suggested.
• error · CompiledBindingMarkup.cs:729 (consensus 3/4) — same undercount in the ConditionalAccess→IndexAccess branch. Precise fix suggested.
• error · BindingCodeWriter.cs:696 (consensus 1/4) — unsafe-getter emission gated solely on hasSubsequentHandler; a writable two-way binding to a non-last inaccessible-getter property whose type isn't INPC still references GetX(...) from Setter.From→ExtendExpression, but the accessor is omitted → uncompilable generated code (value getter reuses the user lambda, so one-way bindings hide it).
• error · TypedBinding.cs:667 (consensus 1/4) — Subscribe(part, OnPropertyChanged) passes a method-group delegate stored only as WeakReference by WeakEventProxy (WeakEventProxy.cs:50); nothing roots it, so after GC the proxy self-Unsubscribe()s and the binding silently stops updating. Existing BindingExpressionPart caches _changeHandler in a field to avoid exactly this.

Context (not posted inline)

A user (lucastitus) reported a related null-intermediary re-subscription regression; it could not be statically confirmed at this SHA and may predate the current diff, so it is noted here rather than as an inline finding. Re-verify with a repro before merge.

CI

PR-failing and not mergeable: maui-pr, Pack macOS, and Helix Unit Tests Windows (Debug & Release) are red, plus Build Analysis fails and the branch is in merge conflict. The failing Windows unit-test legs exercise the binding/source-generator code this PR changes, so they must be treated as potentially PR-caused (consistent with the handler-undercount and generated-code bugs above) until a fresh green run proves otherwise — do not LGTM over them. Rebase, fix the findings, and require a green Helix unit-test run before merge.

[kubaflo]

AI cross-model re-review — PR #32382 ([net11.0] Improve TypedBinding performance)

Verdict: NEEDS_CHANGES (non-approval automated synthesis of 4 independent model re-reviews; does not use GitHub Approve/Request-changes)

Re-review after your update: thanks for the new commit — all three prior code errors are verified fixed against HEAD f11ef00. The remaining reasons for NEEDS_CHANGES are one newly-introduced behavioral redundancy and the still-pending binding-critical CI legs. Two of the four models re-flagged already-fixed issues against stale line numbers; those are false positives and are not reposted.

✅ Fixed (verified in code)

• Prior #1 — handlersCount undercount (CompiledBindingMarkup.cs). Both the direct IndexAccess and the ConditionalAccess→IndexAccess maybe-INPC branches now do handlersCount += CountIndexAccessHandlers(...), which returns 2 when DefaultMemberName is set (matching the two yield returns) and 1 otherwise. _listeners is correctly sized; the Item[n] listener is no longer dropped.
• Prior #2 — missing unsafe getter for a two-way setter (BindingCodeWriter.cs). The emit condition now includes needsGetterForSetter = binding.SetterOptions.IsWritable && !isLastPart, so a writable binding to a non-last inaccessible-getter property emits GetX(...) regardless of hasSubsequentHandler. The generated setter compiles (integration test covers it).
• Prior #3 — weakly-rooted OnPropertyChanged delegate (TypedBinding.cs). The subscription path was redesigned: PropertyChangeListener subscribes its own instance method (source.PropertyChanged += OnPropertyChanged), is strongly rooted by _listeners[] and by the source event, and only weakly references the owning handler. The binding strongly holds the handler (readonly IPropertyChangeHandler _propertyChangeHandler). No weakly-stored delegate remains; the added GC unit test exercises this.
• Prior #4 (context note) — null-intermediary re-subscription. The new Subscribe breaks on a null part and trims trailing listeners via Unsubscribe(startIndex), then re-subscribes on the next apply. Not reposted.

⚠️ Still-open / new — 1 inline comment

• warning · TypedBinding.cs:675 — duplicate source subscription → Apply runs twice. An indexer with a DefaultMemberName yields the same INPC object twice ("Item" + "Item[n]"), so two listeners subscribe to the same PropertyChanged. One notification invokes the handler twice and ShouldApplyChanges (l.690) returns true for each, dispatching Apply twice. The bound value stays correct (BindableProperty dedups the redundant write), so it is not a correctness break — but it is a new behavioral regression that re-evaluates the getter/converter twice on every indexer change, at odds with the PR's perf goal. Coalesce duplicate subscriptions into one listener with multiple expected names, or filter per-listener.

Corrected false positives (not posted)

• gemini re-flagged the handlersCount undercount at CompiledBindingMarkup.cs:695/729 (the old line numbers) — already fixed by CountIndexAccessHandlers; stale.
• opus-4.8 re-flagged the weak Subscribe(part, OnPropertyChanged) delegate at TypedBinding.cs:667 — that signature no longer exists (the listener now subscribes directly). Its CI claim (Pack macOS / Helix failing, merge conflict) is also stale.
• gemini's _listeners[i] = null micro-opt was already considered and dropped as non-blocking in the prior review.

CI

Not PR-failing, but not yet green. macOS Build (Debug/Release) and Pack macOS pass; Windows Build (Debug/Release), Pack Windows, Helix Unit Tests Windows (Debug & Release), and Build Analysis are still pending. The Helix Windows unit-test legs exercise exactly the binding/source-generator code this PR reworks, so merge-readiness cannot be confirmed until they report green. No leg is currently red.

Confidence: medium — the three prior fixes are verified with high confidence; the verdict turns on the new double-apply behavior plus the still-pending binding-critical CI.

[kubaflo]

Duplicate subscription to the same source → Apply runs twice per notification. For an indexer with a non-empty DefaultMemberName, the generated GetHandlers yields the same INotifyPropertyChanged instance twice — (obj, "Item") and (obj, "Item[n]"). This loop therefore creates two separate PropertyChangeListeners (line 663) and subscribes both to obj.PropertyChanged (this line). A single PropertyChanged("Item[n]") then invokes the handler twice (once per listener), and ShouldApplyChanges (line 690) returns true for each invocation because it scans all listeners for a matching (sender, propertyName) — so Apply(fromTarget: false) is dispatched twice instead of once.

The final bound value stays correct (the redundant SetValue is deduped by BindableProperty equality), so this is not a correctness break, but it is a new behavioral regression in a perf-focused PR: every indexer notification re-evaluates the getter/converter twice, whereas the old TypedBinding yielded a single indexer handler and applied once.

Consider coalescing duplicate source subscriptions into one listener that carries multiple expected property names, or moving the per-listener name filter into PropertyChangeListener so only the listener whose expected name matches invokes ApplyChanges.