Merge pull request #1901 from SixLabors/af/faster-getrowspan

Speed up DangerousGetRowSpan(y)

Author: antonfirsov

src/ImageSharp/Common/Helpers/Numerics.cs

@@ -970,6 +970,7 @@ public static uint RotateRightSoftwareFallback(uint value, int offset)
         /// <param name="value">Value.</param>
         /// <param name="min">Mininum value, inclusive.</param>
         /// <param name="max">Maximum value, inclusive.</param>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool IsOutOfRange(int value, int min, int max)
             => (uint)(value - min) > (uint)(max - min);
     }

src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs

@@ -179,7 +179,7 @@ private void ConvertStride(int spectralStep)
                 // PackFromRgbPlanes expects the destination to be padded, so try to get padded span containing extra elements from the next row.
                 // If we can't get such a padded row because we are on a MemoryGroup boundary or at the last row,
                 // pack pixels to a temporary, padded proxy buffer, then copy the relevant values to the destination row.
-                if (this.pixelBuffer.TryGetPaddedRowSpan(yy, 3, out Span<TPixel> destRow))
+                if (this.pixelBuffer.DangerousTryGetPaddedRowSpan(yy, 3, out Span<TPixel> destRow))
                 {
                     PixelOperations<TPixel>.Instance.PackFromRgbPlanes(this.configuration, r, g, b, destRow);
                 }

src/ImageSharp/ImageFrame{TPixel}.cs

@@ -443,12 +443,12 @@ internal void Clear(TPixel value)
         [MethodImpl(InliningOptions.ShortMethod)]
         private void VerifyCoords(int x, int y)
         {
-            if (x < 0 || x >= this.Width)
+            if ((uint)x >= (uint)this.Width)
             {
                 ThrowArgumentOutOfRangeException(nameof(x));
             }
 
-            if (y < 0 || y >= this.Height)
+            if ((uint)y >= (uint)this.Height)
             {
                 ThrowArgumentOutOfRangeException(nameof(y));
             }

src/ImageSharp/Image{TPixel}.cs

@@ -452,12 +452,12 @@ private static Size ValidateFramesAndGetSize(IEnumerable<ImageFrame<TPixel>> fra
         [MethodImpl(InliningOptions.ShortMethod)]
         private void VerifyCoords(int x, int y)
         {
-            if (x < 0 || x >= this.Width)
+            if ((uint)x >= (uint)this.Width)
             {
                 ThrowArgumentOutOfRangeException(nameof(x));
             }
 
-            if (y < 0 || y >= this.Height)
+            if ((uint)y >= (uint)this.Height)
             {
                 ThrowArgumentOutOfRangeException(nameof(y));
             }

src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs

@@ -13,34 +13,35 @@ internal class SharedArrayPoolBuffer<T> : ManagedBufferBase<T>, IRefCounted
         where T : struct
     {
         private readonly int lengthInBytes;
-        private byte[] array;
         private LifetimeGuard lifetimeGuard;
 
         public SharedArrayPoolBuffer(int lengthInElements)
         {
             this.lengthInBytes = lengthInElements * Unsafe.SizeOf<T>();
-            this.array = ArrayPool<byte>.Shared.Rent(this.lengthInBytes);
-            this.lifetimeGuard = new LifetimeGuard(this.array);
+            this.Array = ArrayPool<byte>.Shared.Rent(this.lengthInBytes);
+            this.lifetimeGuard = new LifetimeGuard(this.Array);
         }
 
+        public byte[] Array { get; private set; }
+
         protected override void Dispose(bool disposing)
         {
-            if (this.array == null)
+            if (this.Array == null)
             {
                 return;
             }
 
             this.lifetimeGuard.Dispose();
-            this.array = null;
+            this.Array = null;
         }
 
         public override Span<T> GetSpan()
         {
             this.CheckDisposed();
-            return MemoryMarshal.Cast<byte, T>(this.array.AsSpan(0, this.lengthInBytes));
+            return MemoryMarshal.Cast<byte, T>(this.Array.AsSpan(0, this.lengthInBytes));
         }
 
-        protected override object GetPinnableObject() => this.array;
+        protected override object GetPinnableObject() => this.Array;
 
         public void AddRef()
         {
@@ -53,7 +54,7 @@ public void AddRef()
         [Conditional("DEBUG")]
         private void CheckDisposed()
         {
-            if (this.array == null)
+            if (this.Array == null)
             {
                 throw new ObjectDisposedException("SharedArrayPoolBuffer");
             }

src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs

@@ -8,12 +8,10 @@
 
 namespace SixLabors.ImageSharp.Memory.Internals
 {
-    internal partial class UniformUnmanagedMemoryPool
-#if !NETSTANDARD1_3
-        // In case UniformUnmanagedMemoryPool is finalized, we prefer to run its finalizer after the guard finalizers,
-        // but we should not rely on this.
-        : System.Runtime.ConstrainedExecution.CriticalFinalizerObject
-#endif
+    // CriticalFinalizerObject:
+    // In case UniformUnmanagedMemoryPool is finalized, we prefer to run its finalizer after the guard finalizers,
+    // but we should not rely on this.
+    internal partial class UniformUnmanagedMemoryPool : System.Runtime.ConstrainedExecution.CriticalFinalizerObject
     {
         private static int minTrimPeriodMilliseconds = int.MaxValue;
         private static readonly List<WeakReference<UniformUnmanagedMemoryPool>> AllPools = new();

src/ImageSharp/Memory/Allocators/Internals/UnmanagedBuffer{T}.cs

@@ -31,7 +31,7 @@ public UnmanagedBuffer(int lengthInElements, UnmanagedBufferLifetimeGuard lifeti
             this.lifetimeGuard = lifetimeGuard;
         }
 
-        private void* Pointer => this.lifetimeGuard.Handle.Pointer;
+        public void* Pointer => this.lifetimeGuard.Handle.Pointer;
 
         public override Span<T> GetSpan()
         {

src/ImageSharp/Memory/Buffer2D{T}.cs

@@ -56,7 +56,7 @@ internal Buffer2D(MemoryGroup<T> memoryGroup, int width, int height)
         /// It's public counterpart is <see cref="MemoryGroup"/>,
         /// which only exposes the view of the MemoryGroup.
         /// </remarks>
-        internal MemoryGroup<T> FastMemoryGroup { get; }
+        internal MemoryGroup<T> FastMemoryGroup { get; private set; }
 
         /// <summary>
         /// Gets a reference to the element at the specified position.
@@ -97,35 +97,37 @@ internal Buffer2D(MemoryGroup<T> memoryGroup, int width, int height)
         [MethodImpl(InliningOptions.ShortMethod)]
         public Span<T> DangerousGetRowSpan(int y)
         {
-            DebugGuard.MustBeGreaterThanOrEqualTo(y, 0, nameof(y));
-            DebugGuard.MustBeLessThan(y, this.Height, nameof(y));
+            if ((uint)y >= (uint)this.Height)
+            {
+                this.ThrowYOutOfRangeException(y);
+            }
 
-            return this.GetRowMemoryCore(y).Span;
+            return this.FastMemoryGroup.GetRowSpanCoreUnsafe(y, this.Width);
         }
 
-        internal bool TryGetPaddedRowSpan(int y, int padding, out Span<T> paddedSpan)
+        internal bool DangerousTryGetPaddedRowSpan(int y, int padding, out Span<T> paddedSpan)
         {
             DebugGuard.MustBeGreaterThanOrEqualTo(y, 0, nameof(y));
             DebugGuard.MustBeLessThan(y, this.Height, nameof(y));
 
             int stride = this.Width + padding;
 
-            Memory<T> memory = this.FastMemoryGroup.GetRemainingSliceOfBuffer(y * (long)this.Width);
+            Span<T> slice = this.FastMemoryGroup.GetRemainingSliceOfBuffer(y * (long)this.Width);
 
-            if (memory.Length < stride)
+            if (slice.Length < stride)
             {
                 paddedSpan = default;
                 return false;
             }
 
-            paddedSpan = memory.Span.Slice(0, stride);
+            paddedSpan = slice.Slice(0, stride);
             return true;
         }
 
         [MethodImpl(InliningOptions.ShortMethod)]
         internal ref T GetElementUnsafe(int x, int y)
         {
-            Span<T> span = this.GetRowMemoryCore(y).Span;
+            Span<T> span = this.FastMemoryGroup.GetRowSpanCoreUnsafe(y, this.Width);
             return ref span[x];
         }
 
@@ -139,7 +141,7 @@ internal Memory<T> GetSafeRowMemory(int y)
         {
             DebugGuard.MustBeGreaterThanOrEqualTo(y, 0, nameof(y));
             DebugGuard.MustBeLessThan(y, this.Height, nameof(y));
-            return this.FastMemoryGroup.View.GetBoundedSlice(y * (long)this.Width, this.Width);
+            return this.FastMemoryGroup.View.GetBoundedMemorySlice(y * (long)this.Width, this.Width);
         }
 
         /// <summary>
@@ -168,25 +170,36 @@ internal Memory<T> GetSafeRowMemory(int y)
         /// Swaps the contents of 'destination' with 'source' if the buffers are owned (1),
         /// copies the contents of 'source' to 'destination' otherwise (2). Buffers should be of same size in case 2!
         /// </summary>
-        internal static void SwapOrCopyContent(Buffer2D<T> destination, Buffer2D<T> source)
-        {
-            MemoryGroup<T>.SwapOrCopyContent(destination.FastMemoryGroup, source.FastMemoryGroup);
-            SwapOwnData(destination, source);
-        }
-
-        [MethodImpl(InliningOptions.ShortMethod)]
-        private Memory<T> GetRowMemoryCore(int y) => this.FastMemoryGroup.GetBoundedSlice(y * (long)this.Width, this.Width);
-
-        private static void SwapOwnData(Buffer2D<T> a, Buffer2D<T> b)
+        internal static bool SwapOrCopyContent(Buffer2D<T> destination, Buffer2D<T> source)
         {
-            Size aSize = a.Size();
-            Size bSize = b.Size();
+            bool swapped = false;
+            if (MemoryGroup<T>.CanSwapContent(destination.FastMemoryGroup, source.FastMemoryGroup))
+            {
+                (destination.FastMemoryGroup, source.FastMemoryGroup) =
+                    (source.FastMemoryGroup, destination.FastMemoryGroup);
+                destination.FastMemoryGroup.RecreateViewAfterSwap();
+                source.FastMemoryGroup.RecreateViewAfterSwap();
+                swapped = true;
+            }
+            else
+            {
+                if (destination.FastMemoryGroup.TotalLength != source.FastMemoryGroup.TotalLength)
+                {
+                    throw new InvalidMemoryOperationException(
+                        "Trying to copy/swap incompatible buffers. This is most likely caused by applying an unsupported processor to wrapped-memory images.");
+                }
 
-            b.Width = aSize.Width;
-            b.Height = aSize.Height;
+                source.FastMemoryGroup.CopyTo(destination.MemoryGroup);
+            }
 
-            a.Width = bSize.Width;
-            a.Height = bSize.Height;
+            (destination.Width, source.Width) = (source.Width, destination.Width);
+            (destination.Height, source.Height) = (source.Height, destination.Height);
+            return swapped;
         }
+
+        [MethodImpl(InliningOptions.ColdPath)]
+        private void ThrowYOutOfRangeException(int y) =>
+            throw new ArgumentOutOfRangeException(
+                $"DangerousGetRowSpan({y}). Y was out of range. Height={this.Height}");
     }
 }

src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs

@@ -29,23 +29,23 @@ internal static void Clear<T>(this IMemoryGroup<T> group)
         /// Returns a slice that is expected to be within the bounds of a single buffer.
         /// Otherwise <see cref="ArgumentOutOfRangeException"/> is thrown.
         /// </summary>
-        internal static Memory<T> GetBoundedSlice<T>(this IMemoryGroup<T> group, long start, int length)
+        internal static Memory<T> GetBoundedMemorySlice<T>(this IMemoryGroup<T> group, long start, int length)
             where T : struct
         {
             Guard.NotNull(group, nameof(group));
             Guard.IsTrue(group.IsValid, nameof(group), "Group must be valid!");
             Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length));
             Guard.MustBeLessThan(start, group.TotalLength, nameof(start));
 
-            int bufferIdx = (int)(start / group.BufferLength);
+            int bufferIdx = (int)Math.DivRem(start, group.BufferLength, out long bufferStartLong);
+            int bufferStart = (int)bufferStartLong;
 
             // if (bufferIdx < 0 || bufferIdx >= group.Count)
             if ((uint)bufferIdx >= group.Count)
             {
                 throw new ArgumentOutOfRangeException(nameof(start));
             }
 
-            int bufferStart = (int)(start % group.BufferLength);
             int bufferEnd = bufferStart + length;
             Memory<T> memory = group[bufferIdx];
 
@@ -57,31 +57,6 @@ internal static Memory<T> GetBoundedSlice<T>(this IMemoryGroup<T> group, long st
             return memory.Slice(bufferStart, length);
         }
 
-        /// <summary>
-        /// Returns the slice of the buffer starting at global index <paramref name="start"/> that goes until the end of the buffer.
-        /// </summary>
-        internal static Memory<T> GetRemainingSliceOfBuffer<T>(this IMemoryGroup<T> group, long start)
-            where T : struct
-        {
-            Guard.NotNull(group, nameof(group));
-            Guard.IsTrue(group.IsValid, nameof(group), "Group must be valid!");
-            Guard.MustBeLessThan(start, group.TotalLength, nameof(start));
-
-            int bufferIdx = (int)(start / group.BufferLength);
-
-            // if (bufferIdx < 0 || bufferIdx >= group.Count)
-            if ((uint)bufferIdx >= group.Count)
-            {
-                throw new ArgumentOutOfRangeException(nameof(start));
-            }
-
-            int bufferStart = (int)(start % group.BufferLength);
-
-            Memory<T> memory = group[bufferIdx];
-
-            return memory.Slice(bufferStart);
-        }
-
         internal static void CopyTo<T>(this IMemoryGroup<T> source, Span<T> target)
             where T : struct
         {

src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupSpanCache.cs

@@ -0,0 +1,52 @@
+// Copyright (c) Six Labors.
+// Licensed under the Apache License, Version 2.0.
+
+using System;
+using System.Buffers;
+using SixLabors.ImageSharp.Memory.Internals;
+
+namespace SixLabors.ImageSharp.Memory
+{
+    /// <summary>
+    /// Cached pointer or array data enabling fast <see cref="Span{T}"/> access from
+    /// known <see cref="IMemoryOwner{T}"/> implementations.
+    /// </summary>
+    internal unsafe struct MemoryGroupSpanCache
+    {
+        public SpanCacheMode Mode;
+        public byte[] SingleArray;
+        public void* SinglePointer;
+        public void*[] MultiPointer;
+
+        public static MemoryGroupSpanCache Create<T>(IMemoryOwner<T>[] memoryOwners)
+            where T : struct
+        {
+            IMemoryOwner<T> owner0 = memoryOwners[0];
+            MemoryGroupSpanCache memoryGroupSpanCache = default;
+            if (memoryOwners.Length == 1)
+            {
+                if (owner0 is SharedArrayPoolBuffer<T> sharedPoolBuffer)
+                {
+                    memoryGroupSpanCache.Mode = SpanCacheMode.SingleArray;
+                    memoryGroupSpanCache.SingleArray = sharedPoolBuffer.Array;
+                }
+                else if (owner0 is UnmanagedBuffer<T> unmanagedBuffer)
+                {
+                    memoryGroupSpanCache.Mode = SpanCacheMode.SinglePointer;
+                    memoryGroupSpanCache.SinglePointer = unmanagedBuffer.Pointer;
+                }
+            }
+            else if (owner0 is UnmanagedBuffer<T>)
+            {
+                memoryGroupSpanCache.Mode = SpanCacheMode.MultiPointer;
+                memoryGroupSpanCache.MultiPointer = new void*[memoryOwners.Length];
+                for (int i = 0; i < memoryOwners.Length; i++)
+                {
+                    memoryGroupSpanCache.MultiPointer[i] = ((UnmanagedBuffer<T>)memoryOwners[i]).Pointer;
+                }
+            }
+
+            return memoryGroupSpanCache;
+        }
+    }
+}