From a59e39f8664876656f541bbf668de801e3d59488 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Thu, 4 Jan 2024 14:58:47 +0200 Subject: [PATCH 1/3] perf: Avoid LOH allocations in RenderTargetBitmap --- .../Xaml/Media/Imaging/RenderTargetBitmap.cs | 103 ++++++++++++++++-- .../Media/Imaging/RenderTargetBitmap.iOS.cs | 8 +- .../Media/Imaging/RenderTargetBitmap.macOS.cs | 8 +- .../Media/Imaging/RenderTargetBitmap.skia.cs | 16 ++- 4 files changed, 106 insertions(+), 29 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs index 87e84aa858bf..9f1368a16875 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs @@ -14,14 +14,88 @@ using Uno.UI.Xaml.Media; using Buffer = Windows.Storage.Streams.Buffer; using System.Buffers; +using System.Runtime.InteropServices; namespace Microsoft.UI.Xaml.Media.Imaging { #if NOT_IMPLEMENTED [global::Uno.NotImplemented("IS_UNIT_TESTS", "__WASM__", "__NETSTD_REFERENCE__")] #endif - public partial class RenderTargetBitmap : ImageSource, IDisposable + public partial class RenderTargetBitmap : ImageSource { + // This is to avoid LOH array allocations + private unsafe struct UnmanagedArrayOfBytes : IDisposable + { + public nint Pointer; + public int Length { get; } + + public UnmanagedArrayOfBytes(int length) + { + Length = length; + Pointer = Marshal.AllocHGlobal(length); + } + + public byte this[int index] + { + get + { + return ((byte*)Pointer.ToPointer())[index]; + } + set + { + ((byte*)Pointer.ToPointer())[index] = value; + } + } + + public void Dispose() + { + Marshal.FreeHGlobal(Pointer); + } + } + + // https://stackoverflow.com/questions/52190423/c-sharp-access-unmanaged-array-using-memoryt-or-arraysegmentt + private sealed unsafe class UnmanagedMemoryManager : MemoryManager + where T : unmanaged + { + private readonly T* _pointer; + private readonly int _length; + + /// + /// Create a new UnmanagedMemoryManager instance at the given pointer and size + /// + public UnmanagedMemoryManager(T* pointer, int length) + { + if (length < 0) throw new ArgumentOutOfRangeException(nameof(length)); + _pointer = pointer; + _length = length; + } + /// + /// Obtains a span that represents the region + /// + public override Span GetSpan() => new Span(_pointer, _length); + + /// + /// Provides access to a pointer that represents the data (note: no actual pin occurs) + /// + public override MemoryHandle Pin(int elementIndex = 0) + { + if (elementIndex < 0 || elementIndex >= _length) + throw new ArgumentOutOfRangeException(nameof(elementIndex)); + return new MemoryHandle(_pointer + elementIndex); + } + + /// + /// Has no effect + /// + public override void Unpin() { } + + /// + /// Releases all resources associated with this object + /// + protected override void Dispose(bool disposing) { } + } + + #if NOT_IMPLEMENTED internal const bool IsImplemented = false; #else @@ -63,7 +137,7 @@ public int PixelHeight } #endregion - private byte[]? _buffer; + private UnmanagedArrayOfBytes? _buffer; private int _bufferSize; /// @@ -78,7 +152,7 @@ private protected override bool TryOpenSourceSync(int? targetWidth, int? targetH return false; } - image = Open(_buffer, _bufferSize, width, height); + image = Open(_buffer.Value, _bufferSize, width, height); InvalidateImageSource(); return image.HasData; } @@ -142,7 +216,12 @@ public IAsyncOperation GetPixelsAsync() { return Task.FromResult(new Buffer(Array.Empty())); } - return Task.FromResult(new Buffer(_buffer.AsMemory().Slice(0, _bufferSize))); + + unsafe + { + var mem = new UnmanagedMemoryManager((byte*)_buffer.Value.Pointer.ToPointer(), _bufferSize); + return Task.FromResult(new Buffer(mem.Memory.Slice(0, _bufferSize))); + } }); #if NOT_IMPLEMENTED @@ -150,26 +229,26 @@ public IAsyncOperation GetPixelsAsync() => throw new NotImplementedException("RenderTargetBitmap is not supported on this platform."); #endif - void IDisposable.Dispose() + ~RenderTargetBitmap() { - if (_buffer is not null) + if (_buffer.HasValue) { - ArrayPool.Shared.Return(_buffer); + _buffer.Value.Dispose(); } } #region Misc static helpers #if !NOT_IMPLEMENTED - private static void EnsureBuffer(ref byte[]? buffer, int length) + private static void EnsureBuffer(ref UnmanagedArrayOfBytes? buffer, int length) { if (buffer is null) { - buffer = ArrayPool.Shared.Rent(length); + buffer = new UnmanagedArrayOfBytes(length); } - else if (buffer.Length < length) + else if (buffer.Value.Length < length) { - ArrayPool.Shared.Return(buffer); - buffer = ArrayPool.Shared.Rent(length); + buffer.Value.Dispose(); + buffer = new UnmanagedArrayOfBytes(length); } } #endif diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.iOS.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.iOS.cs index cbcef9b5284b..cf8026d3fb10 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.iOS.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.iOS.cs @@ -23,11 +23,11 @@ partial class RenderTargetBitmap private protected override bool IsSourceReady => _buffer != null; /// - private static ImageData Open(byte[] buffer, int bufferLength, int width, int height) + private static ImageData Open(UnmanagedArrayOfBytes buffer, int bufferLength, int width, int height) { using var colorSpace = CGColorSpace.CreateDeviceRGB(); using var context = new CGBitmapContext( - buffer, + buffer.Pointer, width, height, _bitsPerComponent, @@ -44,7 +44,7 @@ private static ImageData Open(byte[] buffer, int bufferLength, int width, int he return default; } - private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIElement element, ref byte[]? buffer, Size? scaledSize = null) + private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIElement element, ref UnmanagedArrayOfBytes? buffer, Size? scaledSize = null) { var size = new Size(element.ActualSize.X, element.ActualSize.Y); if (size == default) @@ -85,7 +85,7 @@ private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIEle using var colorSpace = CGColorSpace.CreateDeviceRGB(); using var context = new CGBitmapContext( - buffer, + buffer!.Value.Pointer, width, height, _bitsPerComponent, diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.macOS.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.macOS.cs index d1bf2dc5776d..fd870cfd0771 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.macOS.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.macOS.cs @@ -21,11 +21,11 @@ partial class RenderTargetBitmap /// private protected override bool IsSourceReady => _buffer != null; - private static ImageData Open(byte[] buffer, int bufferLength, int width, int height) + private static ImageData Open(UnmanagedArrayOfBytes buffer, int bufferLength, int width, int height) { using var colorSpace = CGColorSpace.CreateDeviceRGB(); using var context = new CGBitmapContext( - buffer, + buffer.Pointer, width, height, _bitsPerComponent, @@ -42,7 +42,7 @@ private static ImageData Open(byte[] buffer, int bufferLength, int width, int he return default; } - private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIElement element, ref byte[]? buffer, Size? scaledSize = null) + private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIElement element, ref UnmanagedArrayOfBytes? buffer, Size? scaledSize = null) { var size = new Size(element.ActualSize.X, element.ActualSize.Y); @@ -87,7 +87,7 @@ private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIEle using var colorSpace = CGColorSpace.CreateDeviceRGB(); using var context = new CGBitmapContext( - buffer, + buffer!.Value.Pointer, width, height, _bitsPerComponent, diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs index 1c6131d3f9fc..18b493bff69a 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs @@ -15,16 +15,15 @@ partial class RenderTargetBitmap private const int _bitsPerComponent = 8; private const int _bytesPerPixel = _bitsPerPixel / _bitsPerComponent; - private static ImageData Open(byte[] buffer, int bufferLength, int width, int height) + private static ImageData Open(UnmanagedArrayOfBytes buffer, int bufferLength, int width, int height) { - var bufferHandle = GCHandle.Alloc(buffer, GCHandleType.Pinned); try { // Note: We use the FromPixelCopy which will create a clone of the buffer, so we are ready to be re-used to render another UIElement. // (It's needed also if we swapped the buffer since we are not maintaining a ref on the swappedBuffer) var bytesPerRow = width * _bytesPerPixel; var info = new SKImageInfo(width, height, SKColorType.Bgra8888, SKAlphaType.Premul); - var image = SKImage.FromPixelCopy(info, bufferHandle.AddrOfPinnedObject(), bytesPerRow); + var image = SKImage.FromPixelCopy(info, buffer.Pointer, bytesPerRow); return ImageData.FromCompositionSurface(new SkiaCompositionSurface(image)); } @@ -32,13 +31,9 @@ private static ImageData Open(byte[] buffer, int bufferLength, int width, int he { return ImageData.FromError(error); } - finally - { - bufferHandle.Free(); - } } - private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIElement element, ref byte[]? buffer, Size? scaledSize = null) + private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIElement element, ref UnmanagedArrayOfBytes? buffer, Size? scaledSize = null) { var renderSize = element.RenderSize; var visual = element.Visual; @@ -74,7 +69,10 @@ private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIEle var byteCount = bitmap.ByteCount; EnsureBuffer(ref buffer, byteCount); - bitmap.GetPixelSpan().CopyTo(buffer); + unsafe + { + bitmap.GetPixelSpan().CopyTo(new Span(buffer!.Value.Pointer.ToPointer(), byteCount)); + } bitmap?.Dispose(); return (byteCount, width, height); } From ee7b543b2962e18ba149a06b794838d261f51643 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Fri, 5 Jan 2024 10:12:20 +0200 Subject: [PATCH 2/3] chore: Adjust for Android --- .../Xaml/Media/Imaging/RenderTargetBitmap.cs | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs index 9f1368a16875..dde4f64702ce 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs @@ -23,6 +23,7 @@ namespace Microsoft.UI.Xaml.Media.Imaging #endif public partial class RenderTargetBitmap : ImageSource { +#if !__ANDROID__ // This is to avoid LOH array allocations private unsafe struct UnmanagedArrayOfBytes : IDisposable { @@ -94,7 +95,7 @@ public override void Unpin() { } /// protected override void Dispose(bool disposing) { } } - +#endif #if NOT_IMPLEMENTED internal const bool IsImplemented = false; @@ -137,7 +138,11 @@ public int PixelHeight } #endregion +#if !__ANDROID__ private UnmanagedArrayOfBytes? _buffer; +#else + private byte[]? _buffer; +#endif private int _bufferSize; /// @@ -146,19 +151,19 @@ private protected override bool TryOpenSourceSync(int? targetWidth, int? targetH var width = PixelWidth; var height = PixelHeight; - if (_buffer is null || _bufferSize <= 0 || width <= 0 || height <= 0) + if (_buffer is not { } buffer || _bufferSize <= 0 || width <= 0 || height <= 0) { image = default; return false; } - image = Open(_buffer.Value, _bufferSize, width, height); + image = Open(buffer, _bufferSize, width, height); InvalidateImageSource(); return image.HasData; } #if NOT_IMPLEMENTED - private static ImageData Open(byte[] buffer, int bufferLength, int width, int height) + private static ImageData Open(UnmanagedArrayOfBytes buffer, int bufferLength, int width, int height) => default; #endif @@ -217,18 +222,23 @@ public IAsyncOperation GetPixelsAsync() return Task.FromResult(new Buffer(Array.Empty())); } +#if !__ANDROID__ unsafe { var mem = new UnmanagedMemoryManager((byte*)_buffer.Value.Pointer.ToPointer(), _bufferSize); return Task.FromResult(new Buffer(mem.Memory.Slice(0, _bufferSize))); } +#else + return Task.FromResult(new Buffer(_buffer.AsMemory().Slice(0, _bufferSize))); +#endif }); #if NOT_IMPLEMENTED - private (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIElement element, ref byte[]? buffer, Size? scaledSize = null) + private (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIElement element, ref UnmanagedArrayOfBytes? buffer, Size? scaledSize = null) => throw new NotImplementedException("RenderTargetBitmap is not supported on this platform."); #endif +#if !__ANDROID__ ~RenderTargetBitmap() { if (_buffer.HasValue) @@ -236,9 +246,19 @@ public IAsyncOperation GetPixelsAsync() _buffer.Value.Dispose(); } } +#endif #region Misc static helpers #if !NOT_IMPLEMENTED +#if __ANDROID__ + private static void EnsureBuffer(ref byte[]? buffer, int length) + { + if (buffer is null || buffer.Length < length) + { + buffer = new byte[length]; + } + } +#else private static void EnsureBuffer(ref UnmanagedArrayOfBytes? buffer, int length) { if (buffer is null) @@ -251,6 +271,7 @@ private static void EnsureBuffer(ref UnmanagedArrayOfBytes? buffer, int length) buffer = new UnmanagedArrayOfBytes(length); } } +#endif #endif #endregion } From a9379369679da160c6610165d883bbeef77f175a Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Fri, 12 Jan 2024 15:15:42 +0200 Subject: [PATCH 3/3] chore: Address review comment --- .../Xaml/Media/Imaging/RenderTargetBitmap.cs | 25 +++++-------------- .../Media/Imaging/RenderTargetBitmap.iOS.cs | 2 +- .../Media/Imaging/RenderTargetBitmap.macOS.cs | 2 +- .../Media/Imaging/RenderTargetBitmap.skia.cs | 2 +- 4 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs index dde4f64702ce..9b90ed45eac0 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.cs @@ -25,7 +25,7 @@ public partial class RenderTargetBitmap : ImageSource { #if !__ANDROID__ // This is to avoid LOH array allocations - private unsafe struct UnmanagedArrayOfBytes : IDisposable + private unsafe class UnmanagedArrayOfBytes { public nint Pointer; public int Length { get; } @@ -34,6 +34,7 @@ public UnmanagedArrayOfBytes(int length) { Length = length; Pointer = Marshal.AllocHGlobal(length); + GC.AddMemoryPressure(length); } public byte this[int index] @@ -48,9 +49,10 @@ public byte this[int index] } } - public void Dispose() + ~UnmanagedArrayOfBytes() { Marshal.FreeHGlobal(Pointer); + GC.RemoveMemoryPressure(Length); } } @@ -225,7 +227,7 @@ public IAsyncOperation GetPixelsAsync() #if !__ANDROID__ unsafe { - var mem = new UnmanagedMemoryManager((byte*)_buffer.Value.Pointer.ToPointer(), _bufferSize); + var mem = new UnmanagedMemoryManager((byte*)_buffer.Pointer.ToPointer(), _bufferSize); return Task.FromResult(new Buffer(mem.Memory.Slice(0, _bufferSize))); } #else @@ -238,16 +240,6 @@ public IAsyncOperation GetPixelsAsync() => throw new NotImplementedException("RenderTargetBitmap is not supported on this platform."); #endif -#if !__ANDROID__ - ~RenderTargetBitmap() - { - if (_buffer.HasValue) - { - _buffer.Value.Dispose(); - } - } -#endif - #region Misc static helpers #if !NOT_IMPLEMENTED #if __ANDROID__ @@ -261,13 +253,8 @@ private static void EnsureBuffer(ref byte[]? buffer, int length) #else private static void EnsureBuffer(ref UnmanagedArrayOfBytes? buffer, int length) { - if (buffer is null) - { - buffer = new UnmanagedArrayOfBytes(length); - } - else if (buffer.Value.Length < length) + if (buffer is null || buffer.Length < length) { - buffer.Value.Dispose(); buffer = new UnmanagedArrayOfBytes(length); } } diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.iOS.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.iOS.cs index cf8026d3fb10..cf009efb1187 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.iOS.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.iOS.cs @@ -85,7 +85,7 @@ private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIEle using var colorSpace = CGColorSpace.CreateDeviceRGB(); using var context = new CGBitmapContext( - buffer!.Value.Pointer, + buffer!.Pointer, width, height, _bitsPerComponent, diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.macOS.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.macOS.cs index fd870cfd0771..28ed8615cb1c 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.macOS.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.macOS.cs @@ -87,7 +87,7 @@ private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIEle using var colorSpace = CGColorSpace.CreateDeviceRGB(); using var context = new CGBitmapContext( - buffer!.Value.Pointer, + buffer!.Pointer, width, height, _bitsPerComponent, diff --git a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs index 18b493bff69a..80281644a006 100644 --- a/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs +++ b/src/Uno.UI/UI/Xaml/Media/Imaging/RenderTargetBitmap.skia.cs @@ -71,7 +71,7 @@ private static (int ByteCount, int Width, int Height) RenderAsBgra8_Premul(UIEle EnsureBuffer(ref buffer, byteCount); unsafe { - bitmap.GetPixelSpan().CopyTo(new Span(buffer!.Value.Pointer.ToPointer(), byteCount)); + bitmap.GetPixelSpan().CopyTo(new Span(buffer!.Pointer.ToPointer(), byteCount)); } bitmap?.Dispose(); return (byteCount, width, height);