From 18eaca20119805389ef2663453d92a33c093a56a Mon Sep 17 00:00:00 2001 From: "Claude (Initial Force WPF Bot)" Date: Fri, 15 May 2026 22:31:43 +0200 Subject: [PATCH 1/2] fix(AdornerLayer): re-entrancy-safe lease for zOrderMap snapshot pool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 7831813a ("wpf-perf(big-win T4): pool AdornerLayer._zOrderMap value snapshot") shipped a per-instance object[] snapshot buffer shared between MeasureOverride and ArrangeOverride to eliminate ~170 MB of per-pass DictionaryEntry[] allocations during MotionCatalyst take-open. Defect: Adorner.Measure / Adorner.Arrange callouts can re-enter the same AdornerLayer's MeasureOverride/ArrangeOverride via a nested layout pass. A naïve shared field lets the inner call's CopyTo overwrite the outer pass's snapshot, and its terminal Array.Clear nulls the slots the outer is still iterating — the outer then reads a null reference and the layout throws, leaving MotionCatalyst with a completely blank canvas on take-open. Fix: lease pattern. Each call captures the current field value into a local, immediately nulls the field (so any re-entrant call allocates its own buffer rather than aliasing), iterates on the local, and at end of pass restores its buffer to the field — keeping whichever buffer (own or the one a nested call left behind) is larger. Steady state on the non-re-entrant path remains zero-allocation: the field holds the grown buffer, every subsequent call leases-clears- copies-iterates-clears-restores in place. Re-entrant calls pay one object[] allocation per nesting level, matching the worst case of the pre-7831813a baseline. Validated end-to-end via MCP UI screenshots on MotionCatalyst: - HEAD before fix: take-open shows fully black canvas - HEAD + this fix: identical to vanilla upstream/release/10.0 (Carl Hansen golf swing, Frame 0/1240, both video viewports rendered, Pressure & Stance heatmap, Launch Monitor, all data boxes populated, playback toggles cleanly) All 358 perf commits in PRs #1-#4 preserved. --- .../System/Windows/Documents/AdornerLayer.cs | 58 ++++++++++++++----- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Documents/AdornerLayer.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Documents/AdornerLayer.cs index f548ed43c77..be55b75d3ec 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Documents/AdornerLayer.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Documents/AdornerLayer.cs @@ -458,15 +458,26 @@ protected override Size MeasureOverride(Size constraint) // Snapshot the values directly into a pooled object[] — SortedList.CopyTo(Array) // would otherwise allocate a fresh DictionaryEntry[] every layout pass // (~170 MB in the MotionCatalyst take-open profile). + // + // Re-entrancy: Adorner.Measure callouts can re-enter this method on the same + // AdornerLayer (nested layout passes). A naïve shared field would let the inner + // call's CopyTo overwrite the outer's snapshot and its Array.Clear null out + // slots the outer is still iterating. Lease the field into a local for the + // duration of the call; re-entrant calls allocate their own. IList valueList = _zOrderMap.GetValueList(); int count = valueList.Count; - if (_zOrderValuesSnapshotBuffer == null || _zOrderValuesSnapshotBuffer.Length < count) - _zOrderValuesSnapshotBuffer = new object[Math.Max(count, 8)]; - valueList.CopyTo(_zOrderValuesSnapshotBuffer, 0); + + object[] buffer = _zOrderValuesSnapshotBuffer; + _zOrderValuesSnapshotBuffer = null; // lease + + if (buffer == null || buffer.Length < count) + buffer = new object[Math.Max(count, 8)]; + + valueList.CopyTo(buffer, 0); for (int i = 0; i < count; i++) { - ArrayList adornerInfos = (ArrayList)_zOrderValuesSnapshotBuffer[i]; + ArrayList adornerInfos = (ArrayList)buffer[i]; Debug.Assert(adornerInfos != null, "No adorners found for element in AdornerLayer._zOrderMap"); int j = 0; @@ -477,7 +488,12 @@ protected override Size MeasureOverride(Size constraint) } } - Array.Clear(_zOrderValuesSnapshotBuffer, 0, count); + Array.Clear(buffer, 0, count); + + // Return the buffer to the pool. If a re-entrant call already restored a + // (possibly larger) buffer, keep the larger one. + if (_zOrderValuesSnapshotBuffer == null || _zOrderValuesSnapshotBuffer.Length < buffer.Length) + _zOrderValuesSnapshotBuffer = buffer; // Returning 0,0 prevents an invalidation of Measure for AdornerLayer from unnecessarily dirtying the parent. return new Size(); @@ -497,16 +513,22 @@ protected override Size MeasureOverride(Size constraint) protected override Size ArrangeOverride(Size finalSize) { // Not using an enumerator because the list can be modified during the loop when we call out. - // Snapshot the values directly into the same pooled object[] used by MeasureOverride. + // See MeasureOverride for the pooled-buffer rationale and the lease pattern + // that keeps the pool re-entrancy-safe when Arrange callouts nest. IList valueList = _zOrderMap.GetValueList(); int count = valueList.Count; - if (_zOrderValuesSnapshotBuffer == null || _zOrderValuesSnapshotBuffer.Length < count) - _zOrderValuesSnapshotBuffer = new object[Math.Max(count, 8)]; - valueList.CopyTo(_zOrderValuesSnapshotBuffer, 0); + + object[] buffer = _zOrderValuesSnapshotBuffer; + _zOrderValuesSnapshotBuffer = null; // lease + + if (buffer == null || buffer.Length < count) + buffer = new object[Math.Max(count, 8)]; + + valueList.CopyTo(buffer, 0); for (int i = 0; i < count; i++) { - ArrayList adornerInfos = (ArrayList)_zOrderValuesSnapshotBuffer[i]; + ArrayList adornerInfos = (ArrayList)buffer[i]; Debug.Assert(adornerInfos != null, "No adorners found for element in AdornerLayer._zOrderMap"); @@ -544,7 +566,12 @@ protected override Size ArrangeOverride(Size finalSize) } } - Array.Clear(_zOrderValuesSnapshotBuffer, 0, count); + Array.Clear(buffer, 0, count); + + // Return the buffer to the pool. If a re-entrant call already restored a + // (possibly larger) buffer, keep the larger one. + if (_zOrderValuesSnapshotBuffer == null || _zOrderValuesSnapshotBuffer.Length < buffer.Length) + _zOrderValuesSnapshotBuffer = buffer; return finalSize; } @@ -1189,10 +1216,11 @@ private GeneralTransform GetProposedTransform(Adorner adorner, GeneralTransform private UIElement[] _keysSnapshotBuffer; // Pooled snapshot buffer for MeasureOverride / ArrangeOverride iteration - // over _zOrderMap.GetValueList(). Avoids the per-pass DictionaryEntry[] - // allocation that SortedList.CopyTo(Array) would otherwise produce. - // Measure and Arrange share the buffer because they never overlap in a - // single layout pass (Measure runs to completion before Arrange begins). + // over _zOrderMap.GetValueList(). Avoids the per-pass DictionaryEntry[] + // allocation that SortedList.CopyTo(Array) would otherwise produce + // (~170 MB during MotionCatalyst take-open). Accessed via the lease pattern + // in each override so nested Adorner.Measure / Arrange callouts that + // re-enter this same AdornerLayer cannot clobber an outer pass's snapshot. private object[] _zOrderValuesSnapshotBuffer; // Dirty-bit gate for OnLayoutUpdated. Set on adorner add/remove and on any From e89e154913343dac514bf08fb80b72f6d28d28fd Mon Sep 17 00:00:00 2001 From: "Claude (Initial Force WPF Bot)" Date: Sat, 16 May 2026 11:30:28 +0200 Subject: [PATCH 2/2] fix(AdornerLayer): guard lease restore with try/finally on layout exceptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR review feedback (gpt-5.5-pro): the lease pattern in commit 18eaca20 restored _zOrderValuesSnapshotBuffer only on normal exit. If an Adorner.Measure / Adorner.Arrange callout (or GetDesiredTransform, the property assignments, etc.) throws, the layer's pool field stays null and subsequent layout passes allocate again, regressing the perf win. The original NRE root cause (re-entrant CopyTo + terminal Array.Clear nulling outer-pass slots) was already fixed by the lease itself — the outer pass owns its local buffer and is immune to nested clobber. This patch adds defensive resource restoration for the exception path: the iteration runs under try, and Array.Clear + restore happen in finally. No behavior change on the happy path. Defense-in-depth for the unhappy path so the pool field invariant survives layout callouts that throw. Co-Authored-By: Claude Opus 4.7 --- .../System/Windows/Documents/AdornerLayer.cs | 110 ++++++++++-------- 1 file changed, 60 insertions(+), 50 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Documents/AdornerLayer.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Documents/AdornerLayer.cs index be55b75d3ec..87e379aa155 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Documents/AdornerLayer.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Documents/AdornerLayer.cs @@ -473,27 +473,32 @@ protected override Size MeasureOverride(Size constraint) if (buffer == null || buffer.Length < count) buffer = new object[Math.Max(count, 8)]; - valueList.CopyTo(buffer, 0); - - for (int i = 0; i < count; i++) + try { - ArrayList adornerInfos = (ArrayList)buffer[i]; - Debug.Assert(adornerInfos != null, "No adorners found for element in AdornerLayer._zOrderMap"); + valueList.CopyTo(buffer, 0); - int j = 0; - while (j < adornerInfos.Count) + for (int i = 0; i < count; i++) { - AdornerInfo adornerInfo = (AdornerInfo)adornerInfos[j++]; - adornerInfo.Adorner.Measure(constraint); + ArrayList adornerInfos = (ArrayList)buffer[i]; + Debug.Assert(adornerInfos != null, "No adorners found for element in AdornerLayer._zOrderMap"); + + int j = 0; + while (j < adornerInfos.Count) + { + AdornerInfo adornerInfo = (AdornerInfo)adornerInfos[j++]; + adornerInfo.Adorner.Measure(constraint); + } } } + finally + { + Array.Clear(buffer, 0, count); - Array.Clear(buffer, 0, count); - - // Return the buffer to the pool. If a re-entrant call already restored a - // (possibly larger) buffer, keep the larger one. - if (_zOrderValuesSnapshotBuffer == null || _zOrderValuesSnapshotBuffer.Length < buffer.Length) - _zOrderValuesSnapshotBuffer = buffer; + // Return the buffer to the pool. If a re-entrant call already restored a + // (possibly larger) buffer, keep the larger one. + if (_zOrderValuesSnapshotBuffer == null || _zOrderValuesSnapshotBuffer.Length < buffer.Length) + _zOrderValuesSnapshotBuffer = buffer; + } // Returning 0,0 prevents an invalidation of Measure for AdornerLayer from unnecessarily dirtying the parent. return new Size(); @@ -524,54 +529,59 @@ protected override Size ArrangeOverride(Size finalSize) if (buffer == null || buffer.Length < count) buffer = new object[Math.Max(count, 8)]; - valueList.CopyTo(buffer, 0); - - for (int i = 0; i < count; i++) + try { - ArrayList adornerInfos = (ArrayList)buffer[i]; - - Debug.Assert(adornerInfos != null, "No adorners found for element in AdornerLayer._zOrderMap"); + valueList.CopyTo(buffer, 0); - int j = 0; - while (j < adornerInfos.Count) + for (int i = 0; i < count; i++) { - AdornerInfo adornerInfo = (AdornerInfo)adornerInfos[j++]; + ArrayList adornerInfos = (ArrayList)buffer[i]; - if (!adornerInfo.Adorner.IsArrangeValid) // optimization + Debug.Assert(adornerInfos != null, "No adorners found for element in AdornerLayer._zOrderMap"); + + int j = 0; + while (j < adornerInfos.Count) { - // We're dependent on Arrange to get the rendersize of the adorner, so Arrange before - // doing our transform magic. - adornerInfo.Adorner.Arrange(new Rect(new Point(), adornerInfo.Adorner.DesiredSize)); - GeneralTransform proposedTransform = adornerInfo.Adorner.GetDesiredTransform(adornerInfo.GetTransformForArrange()); - GeneralTransform adornerTransform = GetProposedTransform(adornerInfo.Adorner, proposedTransform); + AdornerInfo adornerInfo = (AdornerInfo)adornerInfos[j++]; + + if (!adornerInfo.Adorner.IsArrangeValid) // optimization + { + // We're dependent on Arrange to get the rendersize of the adorner, so Arrange before + // doing our transform magic. + adornerInfo.Adorner.Arrange(new Rect(new Point(), adornerInfo.Adorner.DesiredSize)); + GeneralTransform proposedTransform = adornerInfo.Adorner.GetDesiredTransform(adornerInfo.GetTransformForArrange()); + GeneralTransform adornerTransform = GetProposedTransform(adornerInfo.Adorner, proposedTransform); - int index = _children.IndexOf(adornerInfo.Adorner); + int index = _children.IndexOf(adornerInfo.Adorner); - if (index >= 0) + if (index >= 0) + { + // Get the matrix transform out, skip all non affine transforms + Transform transform = adornerTransform?.AffineTransform; + + ((Adorner)(_children[index])).AdornerTransform = transform; + } + } + if (adornerInfo.Adorner.IsClipEnabled) { - // Get the matrix transform out, skip all non affine transforms - Transform transform = adornerTransform?.AffineTransform; - - ((Adorner)(_children[index])).AdornerTransform = transform; + adornerInfo.Adorner.AdornerClip = adornerInfo.Clip; + } + else if (adornerInfo.Adorner.AdornerClip != null) + { + adornerInfo.Adorner.AdornerClip = null; } - } - if (adornerInfo.Adorner.IsClipEnabled) - { - adornerInfo.Adorner.AdornerClip = adornerInfo.Clip; - } - else if (adornerInfo.Adorner.AdornerClip != null) - { - adornerInfo.Adorner.AdornerClip = null; } } } + finally + { + Array.Clear(buffer, 0, count); - Array.Clear(buffer, 0, count); - - // Return the buffer to the pool. If a re-entrant call already restored a - // (possibly larger) buffer, keep the larger one. - if (_zOrderValuesSnapshotBuffer == null || _zOrderValuesSnapshotBuffer.Length < buffer.Length) - _zOrderValuesSnapshotBuffer = buffer; + // Return the buffer to the pool. If a re-entrant call already restored a + // (possibly larger) buffer, keep the larger one. + if (_zOrderValuesSnapshotBuffer == null || _zOrderValuesSnapshotBuffer.Length < buffer.Length) + _zOrderValuesSnapshotBuffer = buffer; + } return finalSize; }