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

📊 Expand Full Gate — 5e6e343 · Simplify value assignment logic in BindableObject

---

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

📊 Expand Full Review — 5e6e343 · Simplify value assignment logic in BindableObject

---

🔍 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)
Files Changed: 2 implementation, 1 benchmark

Key Findings

• No linked GitHub issue; this is a community-contributed performance PR
• Optimizes BindableObject._properties dictionary lookup by replacing BindableProperty reference keys with int (InternalId) keys — simpler hash code computation
• BindableProperty.InternalId is assigned via Interlocked.Increment(ref _nextInternalId) starting from int.MinValue — effectively a process-wide static counter (thread-safe, assigned once per static field initialization)
• Uses CollectionsMarshal.GetValueRefOrAddDefault in GetOrCreateContext to avoid the double-lookup pattern (check-then-add), with #if NETSTANDARD fallback
• Removes LocalValueEnumerator, LocalValueEntry, and GetLocalValueEnumerator internal classes — confirmed unused (no callers found in codebase)
• Renames CreateAndAddContext to CreateContext — dictionary insertion now happens in GetOrCreateContext rather than inside the creation method
• GetValues<T> uses ref var result = ref resultArray[i] to avoid repeated array bounds checks
• Removes the redundant else branch in GetValues<T> (array is zero-initialized, so defaults are already set)
• Reviewer @MartyIX raised overflow concern for InternalId — resolved by author: BindableProperty instances are static readonly fields (initialized once per class load), so overflow is practically impossible
• A separate reviewer thread about removed code is unresolved but is just the author's own annotation
• No unit tests added — Gate was skipped; benchmarks added but no correctness tests
• Potential concern: CollectionsMarshal.GetValueRefOrAddDefault returns a ref that is only valid until the next dictionary mutation; if DefaultValueCreator(this) triggers reentrant GetOrCreateContext calls, the ref context could be invalidated by a dictionary resize

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #33584	Use int InternalId keys + CollectionsMarshal.GetValueRefOrAddDefault	⚠️ SKIPPED (Gate — no tests)	BindableObject.cs, BindableProperty.cs	Original PR

---

🔧 Fix — Analysis & Comparison

---

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix	Override GetHashCode() on BindableProperty with cached sequential int; keep Dictionary<BindableProperty,...> unchanged	✅ PASS (96/96)	BindableProperty.cs only	Minimal diff; doesn't get CollectionsMarshal benefit
2	try-fix	Replace dict with parallel arrays (_propKeys[] + _propValues[]) + linear scan	✅ PASS (96/96)	BindableObject.cs	O(n) for large sets; removes .Values enumeration cleanly
3	try-fix	ReferenceEqualityComparer.Instance + CollectionsMarshal.GetValueRefOrNullRef in GetContext/GetValues<T>; keep BindableProperty keys	✅ PASS (96/96)	BindableObject.cs	No InternalId needed; ref already reference-equal by default
4	try-fix	Single-entry inline cache (_cacheKey, _cacheCtx) as fast path before dict lookup	✅ PASS (96/96)	BindableObject.cs	Adds per-instance memory; uncertain real-world benefit
PR	PR #33584	int InternalId keys + CollectionsMarshal.GetValueRefOrAddDefault + removed LocalValueEnumerator	⚠️ SKIPPED (no tests)	BindableObject.cs, BindableProperty.cs	Supported by benchmarks; most comprehensive

Cross-Pollination

Model	Round	New Ideas?	Details
claude-opus-4.6	2	No	NO NEW IDEAS
claude-sonnet-4.6	2	Yes	Per-type static slot indices with direct array indexing (WPF-style); complex to implement, not pursued
gpt-5.3-codex	2	Yes	Hybrid small-map: inline array for first 4–8 entries, lazy dict promotion on overflow; complex, not pursued
gpt-5.4	2	Yes	Same hybrid small-store idea; already surfaced above

Exhausted: Yes — all models queried, new ideas are complex/risky and space is well-covered by 4 passing alternatives

Selected Fix: PR's fix — Has real benchmark data, handles all sizes in O(1), most comprehensive improvement. All 4 try-fix alternatives pass tests but trade off: Attempt 1 changes GetHashCode semantics, Attempt 2 is O(n) for large sets, Attempt 3 may offer minimal gain (reference equality is already default), Attempt 4 adds per-instance memory with uncertain benefit.

---

📋 Report — Final Recommendation

---

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase	Status	Notes
Pre-Flight	✅ COMPLETE	Performance optimization PR, no linked issue
Gate	⚠️ SKIPPED	No tests detected in PR
Try-Fix	✅ COMPLETE	4 attempts, 4 passing; 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. The benchmarks show 10–25% improvement across typical property counts. The approach is sound, but the PR needs unit tests and has one correctness concern worth addressing before merge.

Root Cause

BindableObject uses a Dictionary<BindableProperty, BindablePropertyContext> for per-instance property storage. The default GetHashCode() on reference types incurs RuntimeHelpers.GetHashCode() overhead on every lookup. Additionally, GetOrCreateContext previously performed a double-lookup (check existence, then add), which is inefficient.

Fix Quality

Strengths:

• Backed by real BenchmarkDotNet data showing 10–25% throughput improvement
• Integer key dictionary is faster due to trivial hash computation (int.GetHashCode() is a single identity operation)
• CollectionsMarshal.GetValueRefOrAddDefault eliminates double-lookup in GetOrCreateContext — a single atomic dictionary operation
• Removal of LocalValueEnumerator/LocalValueEntry is correct — grep confirms zero usages in the codebase
• InternalId is assigned via Interlocked.Increment (thread-safe), incremented only once per BindableProperty (static field), so overflow is effectively impossible
• #if NETSTANDARD fallback is included for platforms without CollectionsMarshal

Concerns requiring changes:

1. No unit tests (blocker): Gate was skipped because no tests were added. The PR modifies critical hot-path code (GetValue/SetValue/GetOrCreateContext) used everywhere in MAUI. Unit tests verifying correctness of the new integer-keyed lookup, GetValues<T>, and GetOrCreateContext should be added to BindableObjectUnitTests.cs.

2. CollectionsMarshal.GetValueRefOrAddDefault ref safety (concern): The ref context obtained from GetValueRefOrAddDefault is only valid until the next dictionary mutation. If property.DefaultValueCreator(this) triggers reentrant access to GetOrCreateContext (e.g., a property whose default value reads another property), the dictionary may resize and the ref context becomes invalid — pointing to freed/relocated memory. This was safe in the old CreateAndAddContext pattern because the property was added after the creator ran. A comment or guard should be added to document this assumption.

3. Starting counter at int.MinValue (minor): private static int _nextInternalId = int.MinValue; means the first property gets ID -2147483647. Starting from 0 (i.e., initializing to -1 so the first Interlocked.Increment returns 0) would be more readable and conventional. This is cosmetic but worth noting.

4. Benchmark file missing trailing newline (trivial): BindableObjectBenchmarker.cs is missing a final newline — minor style issue.

Try-Fix comparison:
All 4 alternative approaches pass unit tests, but the PR's approach is the best overall: it's O(1) for all sizes (unlike parallel arrays), avoids changing GetHashCode semantics (unlike Attempt 1), and is more impactful than the ReferenceEqualityComparer approach (Attempt 3, which may offer minimal gain since reference equality is already the default for these objects).

Verdict: Request changes to add unit tests and address the CollectionsMarshal ref safety concern with documentation. The optimization strategy is sound and should be approved once those are addressed.

---