BindableObject property access micro-optimizations

[albyrock87]

Description of Change

I tried to improve the access time on BindableProperty given they're being accessed a lot during the application usage.
As we can see using Array + SIMD is actually performing better when the number of BindableProperty set on a BindableObject is less than ~10, though it becomes worse after that.
Array + SIMD also (obviously) allocates less memory.

Considering that SIMD may perform differently on different platforms I felt it would be better to simply improve the current Dictionary based implementation by leveraging:

• integer keys
• CollectionMarshal on GetOrAdd

Benchmarks

Strategy	PropertiesToSet	Mean	Error	StdDev	Gen0	Gen1	Allocated
Dictionary	1	65.22 ns	0.246 ns	0.218 ns	0.0889	0.0001	744 B
Array + SIMD	1	52.93 ns	0.750 ns	0.665 ns	0.0678	-	568 B
Dictionary<int,>	1	59.29 ns	0.563 ns	0.440 ns	0.0889	0.0001	744 B
Dictionary	3	150.70 ns	0.344 ns	0.322 ns	0.1500	0.0005	1256 B
Array + SIMD	3	122.09 ns	0.163 ns	0.127 ns	0.1290	0.0002	1080 B
Dictionary<int,>	3	133.50 ns	0.231 ns	0.180 ns	0.1500	0.0005	1256 B
Dictionary	8	426.76 ns	0.686 ns	0.641 ns	0.3662	0.0033	3064 B
Array + SIMD	8	318.26 ns	0.438 ns	0.409 ns	0.3028	0.0024	2536 B
Dictionary<int,>	8	338.98 ns	2.381 ns	1.988 ns	0.3662	0.0038	3064 B
Dictionary	15	773.49 ns	2.593 ns	2.298 ns	0.5798	0.0095	4856 B
Array + SIMD	15	665.18 ns	0.846 ns	0.791 ns	0.5531	0.0076	4632 B
Dictionary<int,>	15	581.04 ns	0.431 ns	0.360 ns	0.5798	0.0095	4856 B
Dictionary	30	1,542.20 ns	3.342 ns	3.126 ns	1.1692	0.0381	9784 B
Array + SIMD	30	1,419.83 ns	1.301 ns	1.154 ns	1.0796	0.0324	9032 B
Dictionary<int,>	30	1,165.86 ns	1.599 ns	1.496 ns	1.1692	0.0381	9784 B
Dictionary	50	2,648.69 ns	3.759 ns	2.935 ns	2.0828	0.1183	17448 B
Array + SIMD	50	2,587.54 ns	3.111 ns	2.429 ns	1.8196	0.0916	15224 B
Dictionary<int,>	50	1,997.17 ns	2.174 ns	2.033 ns	2.0828	0.1183	17448 B

[albyrock87]

Removed unused methods/classes

[albyrock87]

Here's the SIMD implementation I used for the test.

using System;
using System.Collections.Generic;

namespace Microsoft.Maui.Controls;

internal class BindablePropertiesContextStore
{
	private int _count;
	private long[] _propertyIds = [];
	private BindableObject.BindablePropertyContext[] _propertyContexts = [];
	
	public IReadOnlyList<BindableObject.BindablePropertyContext> Values => _propertyContexts;

	public BindableObject.BindablePropertyContext? Get(BindableProperty property)
	{
		var i = MemoryExtensions.IndexOf(_propertyIds, property.InternalId);
		return i < 0 ? null : _propertyContexts[i];
	}

	public void Add(BindableProperty property, BindableObject.BindablePropertyContext context)
	{
		EnsureCapacity(_count);
		_propertyIds[_count] = property.InternalId;
		_propertyContexts[_count++] = context;
	}

	private void EnsureCapacity(int targetIndex)
	{
		var length = _propertyIds.Length;
		
		if (length <= targetIndex)
		{

			if (length == 0)
			{
				_propertyIds = new long[4];
				_propertyContexts = new BindableObject.BindablePropertyContext[4];
				return;
			}

			var capacity = targetIndex * 2;
			Array.Resize(ref _propertyIds, capacity);
			Array.Resize(ref _propertyContexts, capacity);
		}
	}
}

[kubaflo]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[kubaflo]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[kubaflo]

Multimodal Review — BindableObject property access micro-optimizations

Overall Assessment: ✅ Looks good

Solid micro-optimization of a hot path. The benchmarks are convincing — Dictionary<int> + CollectionsMarshal wins clearly at ≥8 properties, and is competitive at lower counts. The approach is conservative and correct.

---

What works well

1. Dictionary<int, ...> keying — Switching from BindableProperty (reference-type key → virtual GetHashCode/Equals) to int keys eliminates that overhead entirely. Integer hashing in Dictionary is trivially cheap.

2. CollectionsMarshal.GetValueRefOrAddDefault in GetOrCreateContext — This is the highest-value change. The old GetContext() ?? CreateAndAddContext() did two dictionary lookups on every first access. The new code does a single probe. Correctly guarded with #if NETSTANDARD for netstandard2.0/netstandard2.1 TFMs where CollectionsMarshal is unavailable.

3. ref var result in GetValues<T> — Avoids repeated array indexing into the value-tuple array. Nice touch, and the removal of the now-redundant else branch (default values are already zero-initialized) was correctly identified by @MartyIX.

4. LocalValueEnumerator / LocalValueEntry removal — Confirmed zero usages across the repo. Dead code cleanup ✅

5. Interlocked.Increment for ID assignment — Thread-safe. And as @albyrock87 explained, BindableProperty instances are almost exclusively created as static fields during type initialization, so overflow is not a practical concern.

---

Suggestions (non-blocking)

1. Missing trailing newline in BindableObjectBenchmarker.cs
The new benchmark file doesn't end with a newline character. Most .NET tooling and dotnet format expect one. Minor but will show up as a diff artifact.

2. Consider [Benchmark(Baseline = true)]
Since the benchmark is meant to compare against the old implementation, marking one scenario as baseline would make future regression detection easier. Not required for this PR though.

3. Minor: _nextInternalId starting at int.MinValue
Functionally correct (keys just need uniqueness), but starting at 0 would be slightly more readable in debugger inspection. Non-blocking — the current approach maximizes the available ID space, which is a reasonable choice.

---

Correctness verification

• ✅ All _properties usages checked — .Values iteration at lines 376 and 636 still works since BindablePropertyContext.Property holds the BindableProperty reference.
• ✅ No public API surface changes — InternalId is internal, removed types were internal.
• ✅ #if NETSTANDARD guard is correct — Controls.Core.csproj targets netstandard2.0;netstandard2.1 alongside platform TFMs.
• ✅ No PublicAPI.Unshipped.txt changes needed.

[MauiBot]

🚦 Gate — Test Before and After Fix

👋 @albyrock87 — new gate results are available. Please review the latest session below.

🚦 Gate Session — 5e6e343 · Simplify value assignment logic in BindableObject · 2026-04-11 12:21 UTC

---

Gate Result: ⚠️ SKIPPED

No tests were detected in this PR.

Recommendation: Add tests to verify the fix using the write-tests-agent:

@copilot write tests for this PR

The agent will analyze the issue, determine the appropriate test type (UI test, device test, unit test, or XAML test), and create tests that verify the fix.

---

[albyrock87]

@copilot write tests for this PR

[MauiBot]

🤖 AI Summary

👋 @albyrock87 — new AI review results are available. Please review the latest session below.

📊 Review Session — 5e6e343 · Simplify value assignment logic in BindableObject · 2026-04-11 13:02 UTC

---

🔍 Pre-Flight — Context & Validation

---

Issue: N/A — no linked issue (standalone performance optimization)
PR: #33584 - BindableObject property access micro-optimizations
Platforms Affected: All (cross-platform core change — BindableObject is used everywhere)
Files Changed: 2 implementation, 1 benchmark

Key Findings

• No linked GitHub issue; community-contributed performance PR by @albyrock87
• Replaces Dictionary<BindableProperty, BindablePropertyContext> with Dictionary<int, BindablePropertyContext> using per-property integer InternalId keys — simpler hash computation
• BindableProperty.InternalId assigned via Interlocked.Increment(ref _nextInternalId) starting from int.MinValue — thread-safe, assigned once per static field initialization (not per instance)
• Uses CollectionsMarshal.GetValueRefOrAddDefault in GetOrCreateContext to eliminate double-lookup pattern (check existence, then add), with #if NETSTANDARD fallback
• Removes LocalValueEnumerator, LocalValueEntry, and GetLocalValueEnumerator — confirmed unused (grep confirms zero callers outside the removed code)
• Renames CreateAndAddContext to CreateContext — dictionary insertion now handled in GetOrCreateContext
• GetValues<T> uses ref var result = ref resultArray[i] to avoid repeated array bounds checks and removes redundant else-branch (array already zero-initialized)
• Overflow concern for InternalId raised by @MartyIX and resolved: BindableProperty instances are static readonly fields incremented once per type load, making overflow practically impossible
• Prior agent review at same commit SHA (5e6e343, 2026-04-05): all 4 try-fix alternatives passing; recommended REQUEST CHANGES
• No unit tests added — Gate skipped; only benchmarks are included
• Correctness concern: CollectionsMarshal.GetValueRefOrAddDefault ref is valid only until next dictionary mutation; if DefaultValueCreator(this) triggers reentrant GetOrCreateContext, dictionary resize could invalidate the ref
• PR rebased on 2026-04-08 (triggered by @PureWeen), same HEAD SHA 5e6e343
• Benchmarks show 10–25% throughput improvement across 1–50 properties set per object

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #33584	Use int InternalId keys + CollectionsMarshal.GetValueRefOrAddDefault + remove LocalValueEnumerator	⏳ PENDING (Gate skipped — no tests)	BindableObject.cs, BindableProperty.cs	Original PR; backed by BenchmarkDotNet data showing 10–25% improvement

---

🔧 Fix — Analysis & Comparison

---

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix (claude-opus-4.6)	Override GetHashCode()/Equals() on BindableProperty with cached sequential int; keep BindableProperty as dict key	✅ PASS (96/96)	BindableProperty.cs, BindableObject.cs	Requires public API changes (GetHashCode/Equals overrides on sealed class); no key type change
2	try-fix (claude-sonnet-4.6)	ReferenceEqualityComparer.Instance as dictionary comparer + CollectionsMarshal.GetValueRefOrAddDefault	✅ PASS (96/96)	BindableObject.cs only	Simplest approach; no BindableProperty.cs changes; no new API entries needed
3	try-fix (gpt-5.3-codex)	Replace dictionary with List<BindablePropertyContext> + linear ReferenceEquals scan	✅ PASS (96/96)	BindableObject.cs only	O(n) for large property sets; correct for typical few-properties cases; more complex refactor
4	try-fix (gpt-5.4)	2-entry per-instance MRU cache as fast-path before dictionary lookup + CollectionsMarshal fallback	✅ PASS (96/96)	BindableObject.cs only	Novel; adds per-instance overhead (~2 fields); benefit depends on access locality
PR	PR #33584	int InternalId keys + CollectionsMarshal.GetValueRefOrAddDefault + remove LocalValueEnumerator	⚠️ SKIPPED (no tests)	BindableObject.cs, BindableProperty.cs	Backed by BenchmarkDotNet data; most comprehensive optimization

Cross-Pollination

Model	Round	New Ideas?	Details
claude-opus-4.6	2	No	Simple solution space fully covered by 4 attempts + PR
claude-sonnet-4.6	2	Yes	Sorted array + binary search using InternalId — still requires InternalId, complex insertion
gpt-5.3-codex	2	Yes	Hybrid list+dictionary with threshold promotion — variant of Attempt 3, more complex
gpt-5.4	2	Yes	Custom IEqualityComparer<BindableProperty> using InternalId hash — hybrid of Attempts 1+2

Exhausted: Yes — round 2 shows only complex/speculative variants remain; claude-opus confirms no new simple ideas

Selected Fix: PR's fix — The only approach backed by actual BenchmarkDotNet data showing 10–25% throughput improvement. Attempt 2 (ReferenceEqualityComparer.Instance + CollectionsMarshal) is the most compelling simpler alternative — only modifies BindableObject.cs, no BindableProperty changes, no new API surface. However, it lacks benchmark evidence. PR's int-key approach is the most impactful per benchmarks.

---

📋 Report — Final Recommendation

---

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase	Status	Notes
Pre-Flight	✅ COMPLETE	Performance optimization PR; no linked issue; community contribution
Gate	⚠️ SKIPPED	No tests detected in PR
Try-Fix	✅ COMPLETE	4 attempts, all passing; 2 cross-pollination rounds; PR's fix selected as best
Report	✅ COMPLETE

Summary

PR #33584 is a well-benchmarked community contribution that optimizes BindableObject property storage by switching from Dictionary<BindableProperty, BindablePropertyContext> to Dictionary<int, BindablePropertyContext> (using per-property integer IDs) and applying CollectionsMarshal.GetValueRefOrAddDefault to eliminate the double-lookup in GetOrCreateContext. Benchmarks show 10–25% throughput improvement. The approach is sound, but the PR has one correctness concern and needs unit tests before merge.

Root Cause

BindableObject uses Dictionary<BindableProperty, BindablePropertyContext> for per-instance property storage. Every GetValue/SetValue call triggers virtual dispatch on BindableProperty.GetHashCode() (delegating to RuntimeHelpers.GetHashCode()). Additionally, GetOrCreateContext performed a double-lookup: first TryGetValue (check existence), then Add (insert). Both hot paths are called on every property read/write in a MAUI application.

Fix Quality

Strengths:

• Backed by real BenchmarkDotNet data showing 10–25% throughput improvement across 1–50 properties set
• Integer key dictionary: int.GetHashCode() is a single identity operation — no virtual dispatch, no RuntimeHelpers call
• CollectionsMarshal.GetValueRefOrAddDefault eliminates double-lookup in GetOrCreateContext
• Removal of LocalValueEnumerator/LocalValueEntry is correct — confirmed unused in codebase
• InternalId uses Interlocked.Increment (thread-safe), assigned once per static field init; overflow is effectively impossible
• #if NETSTANDARD fallback for platforms without CollectionsMarshal
• GetValues<T> ref var optimization removes repeated array bounds checks and eliminates redundant else branch

Concerns requiring changes:

1. No unit tests (blocker): Gate skipped — no tests added. PR modifies GetValue/SetValue/GetOrCreateContext (critical hot paths). Unit tests verifying correctness of the new int-keyed lookup and GetValues<T> behavior should be added to BindableObjectUnitTests.cs in src/Controls/tests/Core.UnitTests/.

2. CollectionsMarshal.GetValueRefOrAddDefault ref safety (concern): The ref context returned by GetValueRefOrAddDefault is only valid until the next dictionary mutation. If property.DefaultValueCreator(this) triggers reentrant GetOrCreateContext (a property whose default value reads another property on the same object), the dictionary may resize and the ref context becomes invalid. A comment documenting this assumption (no reentrant access) should be added.

3. Counter starting at int.MinValue (minor): Starting from int.MinValue means the first property gets ID -2147483647. Starting from 0 (i.e., first Increment returns 1) is more conventional and readable.

4. Benchmark file missing trailing newline (trivial): BindableObjectBenchmarker.cs lacks a final newline.

Try-Fix comparison:
All 4 try-fix alternatives pass tests. Attempt 2 (ReferenceEqualityComparer.Instance + CollectionsMarshal) is the most compelling simpler alternative — achieves both optimizations with only BindableObject.cs changes and zero changes to BindableProperty.cs. However, it has no benchmark data to compare against the PR's approach. The PR's int-key approach is the most impactful per benchmarks and the approach the author has validated end-to-end.

Selected Fix: PR's fix — backed by benchmark data; s/agent-fix-pr-picked

---

[PureWeen]

/rebase