From 00dc2a8c98bd54ec20a640a4266bbe94b8fe3e67 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 6 Feb 2020 23:11:36 +1100 Subject: [PATCH] Internalize, partially optimize and rename Action methods. --- src/ImageSharp/Advanced/IRowIntervalAction.cs | 29 +++++ .../Advanced/IRowIntervalAction{TBuffer}.cs | 40 ++++++- .../Advanced/ParallelRowIterator.cs | 112 ++++++------------ .../Convolution/BokehBlurProcessor{TPixel}.cs | 2 +- .../Convolution2DProcessor{TPixel}.cs | 2 +- .../Convolution2PassProcessor{TPixel}.cs | 2 +- .../ConvolutionProcessor{TPixel}.cs | 2 +- .../PixelRowDelegateProcessorBase{TPixel}.cs | 2 +- .../Filters/FilterProcessor{TPixel}.cs | 2 +- .../Overlays/GlowProcessor{TPixel}.cs | 2 +- .../Overlays/VignetteProcessor{TPixel}.cs | 2 +- .../Helpers/ParallelRowIteratorTests.cs | 8 +- .../TestUtilities/TestImageExtensions.cs | 2 +- 13 files changed, 119 insertions(+), 88 deletions(-) diff --git a/src/ImageSharp/Advanced/IRowIntervalAction.cs b/src/ImageSharp/Advanced/IRowIntervalAction.cs index df422a65fb..9a67eea715 100644 --- a/src/ImageSharp/Advanced/IRowIntervalAction.cs +++ b/src/ImageSharp/Advanced/IRowIntervalAction.cs @@ -40,6 +40,35 @@ namespace SixLabors.ImageSharp.Advanced } } + internal readonly struct WrappingRowIntervalAction + { + private readonly WrappingRowIntervalInfo info; + private readonly Action action; + + [MethodImpl(InliningOptions.ShortMethod)] + public WrappingRowIntervalAction(in WrappingRowIntervalInfo info, Action action) + { + this.info = info; + this.action = action; + } + + [MethodImpl(InliningOptions.ShortMethod)] + public void Invoke(int i) + { + int yMin = this.info.MinY + (i * this.info.StepY); + + if (yMin >= this.info.MaxY) + { + return; + } + + int yMax = Math.Min(yMin + this.info.StepY, this.info.MaxY); + var rows = new RowInterval(yMin, yMax); + + this.action(rows); + } + } + internal readonly struct WrappingRowIntervalAction where T : struct, IRowIntervalAction { diff --git a/src/ImageSharp/Advanced/IRowIntervalAction{TBuffer}.cs b/src/ImageSharp/Advanced/IRowIntervalAction{TBuffer}.cs index 7b841b9cde..1fd088e09f 100644 --- a/src/ImageSharp/Advanced/IRowIntervalAction{TBuffer}.cs +++ b/src/ImageSharp/Advanced/IRowIntervalAction{TBuffer}.cs @@ -23,7 +23,43 @@ namespace SixLabors.ImageSharp.Advanced void Invoke(in RowInterval rows, Memory memory); } - internal readonly struct WrappingRowIntervalAction + internal readonly struct WrappingRowIntervalBufferAction + where TBuffer : unmanaged + { + private readonly WrappingRowIntervalInfo info; + private readonly MemoryAllocator allocator; + private readonly Action> action; + + [MethodImpl(InliningOptions.ShortMethod)] + public WrappingRowIntervalBufferAction( + in WrappingRowIntervalInfo info, + MemoryAllocator allocator, + Action> action) + { + this.info = info; + this.allocator = allocator; + this.action = action; + } + + [MethodImpl(InliningOptions.ShortMethod)] + public void Invoke(int i) + { + int yMin = this.info.MinY + (i * this.info.StepY); + + if (yMin >= this.info.MaxY) + { + return; + } + + int yMax = Math.Min(yMin + this.info.StepY, this.info.MaxY); + var rows = new RowInterval(yMin, yMax); + + using IMemoryOwner buffer = this.allocator.Allocate(this.info.MaxX); + this.action(rows, buffer.Memory); + } + } + + internal readonly struct WrappingRowIntervalBufferAction where T : struct, IRowIntervalAction where TBuffer : unmanaged { @@ -32,7 +68,7 @@ namespace SixLabors.ImageSharp.Advanced private readonly T action; [MethodImpl(InliningOptions.ShortMethod)] - public WrappingRowIntervalAction( + public WrappingRowIntervalBufferAction( in WrappingRowIntervalInfo info, MemoryAllocator allocator, in T action) diff --git a/src/ImageSharp/Advanced/ParallelRowIterator.cs b/src/ImageSharp/Advanced/ParallelRowIterator.cs index 3bc9e1f90b..5c1b4110e2 100644 --- a/src/ImageSharp/Advanced/ParallelRowIterator.cs +++ b/src/ImageSharp/Advanced/ParallelRowIterator.cs @@ -130,7 +130,7 @@ namespace SixLabors.ImageSharp.Advanced int verticalStep = DivideCeil(height, numOfSteps); var parallelOptions = new ParallelOptions { MaxDegreeOfParallelism = numOfSteps }; var rowInfo = new WrappingRowIntervalInfo(top, bottom, verticalStep, width); - var rowAction = new WrappingRowIntervalAction(in rowInfo, allocator, in body); + var rowAction = new WrappingRowIntervalBufferAction(in rowInfo, allocator, in body); Parallel.For( 0, @@ -145,11 +145,9 @@ namespace SixLabors.ImageSharp.Advanced /// The . /// The to get the parallel settings from. /// The method body defining the iteration logic on a single . - // [Obsolete("Use non-allocating generic versions instead.")] - public static void IterateRows(Rectangle rectangle, Configuration configuration, Action body) + internal static void IterateRows(Rectangle rectangle, Configuration configuration, Action body) { var parallelSettings = ParallelExecutionSettings.FromConfiguration(configuration); - IterateRows(rectangle, in parallelSettings, body); } @@ -159,80 +157,81 @@ namespace SixLabors.ImageSharp.Advanced /// The . /// The . /// The method body defining the iteration logic on a single . - // [Obsolete("Use non-allocating generic versions instead.")] - public static void IterateRows( + internal static void IterateRows( Rectangle rectangle, in ParallelExecutionSettings parallelSettings, Action body) { ValidateRectangle(rectangle); - int maxSteps = DivideCeil( - rectangle.Width * rectangle.Height, - parallelSettings.MinimumPixelsProcessedPerTask); + int top = rectangle.Top; + int bottom = rectangle.Bottom; + int width = rectangle.Width; + int height = rectangle.Height; + int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask); int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps); // Avoid TPL overhead in this trivial case: if (numOfSteps == 1) { - var rows = new RowInterval(rectangle.Top, rectangle.Bottom); + var rows = new RowInterval(top, bottom); body(rows); return; } int verticalStep = DivideCeil(rectangle.Height, numOfSteps); - var parallelOptions = new ParallelOptions { MaxDegreeOfParallelism = numOfSteps }; - - int top = rectangle.Top; - int bottom = rectangle.Bottom; + var rowInfo = new WrappingRowIntervalInfo(top, bottom, verticalStep); + var rowAction = new WrappingRowIntervalAction(in rowInfo, body); Parallel.For( 0, numOfSteps, parallelOptions, - i => - { - int yMin = top + (i * verticalStep); - - if (yMin >= bottom) - { - return; - } - - int yMax = Math.Min(yMin + verticalStep, bottom); - - var rows = new RowInterval(yMin, yMax); + rowAction.Invoke); + } - body(rows); - }); + /// + /// Iterate through the rows of a rectangle in optimized batches defined by -s + /// instantiating a temporary buffer for each invocation. + /// + internal static void IterateRows( + Rectangle rectangle, + Configuration configuration, + Action> body) + where TBuffer : unmanaged + { + var parallelSettings = ParallelExecutionSettings.FromConfiguration(configuration); + IterateRows(rectangle, in parallelSettings, body); } /// /// Iterate through the rows of a rectangle in optimized batches defined by -s /// instantiating a temporary buffer for each invocation. /// - // [Obsolete("Use non-allocating generic versions instead.")] - internal static void IterateRowsWithTempBuffer( + internal static void IterateRows( Rectangle rectangle, in ParallelExecutionSettings parallelSettings, - Action> body) - where T : unmanaged + Action> body) + where TBuffer : unmanaged { ValidateRectangle(rectangle); - int maxSteps = DivideCeil(rectangle.Width * rectangle.Height, parallelSettings.MinimumPixelsProcessedPerTask); + int top = rectangle.Top; + int bottom = rectangle.Bottom; + int width = rectangle.Width; + int height = rectangle.Height; + int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask); int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps); - - MemoryAllocator memoryAllocator = parallelSettings.MemoryAllocator; + MemoryAllocator allocator = parallelSettings.MemoryAllocator; // Avoid TPL overhead in this trivial case: if (numOfSteps == 1) { - var rows = new RowInterval(rectangle.Top, rectangle.Bottom); - using (IMemoryOwner buffer = memoryAllocator.Allocate(rectangle.Width)) + var rows = new RowInterval(top, bottom); + using (IMemoryOwner buffer = allocator.Allocate(width)) { body(rows, buffer.Memory); } @@ -241,48 +240,15 @@ namespace SixLabors.ImageSharp.Advanced } int verticalStep = DivideCeil(rectangle.Height, numOfSteps); - var parallelOptions = new ParallelOptions { MaxDegreeOfParallelism = numOfSteps }; - - int top = rectangle.Top; - int bottom = rectangle.Bottom; + var rowInfo = new WrappingRowIntervalInfo(top, bottom, verticalStep, width); + var rowAction = new WrappingRowIntervalBufferAction(in rowInfo, allocator, body); Parallel.For( 0, numOfSteps, parallelOptions, - i => - { - int yMin = top + (i * verticalStep); - - if (yMin >= bottom) - { - return; - } - - int yMax = Math.Min(yMin + verticalStep, rectangle.Bottom); - - var rows = new RowInterval(yMin, yMax); - - using (IMemoryOwner buffer = memoryAllocator.Allocate(rectangle.Width)) - { - body(rows, buffer.Memory); - } - }); - } - - /// - /// Iterate through the rows of a rectangle in optimized batches defined by -s - /// instantiating a temporary buffer for each invocation. - /// - // [Obsolete("Use non-allocating generic versions instead.")] - internal static void IterateRowsWithTempBuffer( - Rectangle rectangle, - Configuration configuration, - Action> body) - where T : unmanaged - { - IterateRowsWithTempBuffer(rectangle, ParallelExecutionSettings.FromConfiguration(configuration), body); + rowAction.Invoke); } [MethodImpl(InliningOptions.ShortMethod)] diff --git a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor{TPixel}.cs index bb89f03188..afce39dcdb 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor{TPixel}.cs @@ -427,7 +427,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution int width = workingRectangle.Width; float exp = this.gamma; - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( workingRectangle, configuration, (rows, vectorBuffer) => diff --git a/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor{TPixel}.cs index 431e1c6043..e2088084a1 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor{TPixel}.cs @@ -78,7 +78,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution var workingRectangle = Rectangle.FromLTRB(startX, startY, endX, endY); int width = workingRectangle.Width; - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( workingRectangle, this.Configuration, (rows, vectorBuffer) => diff --git a/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs index ce13b4074f..3f12b2a283 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs @@ -97,7 +97,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution var workingRectangle = Rectangle.FromLTRB(startX, startY, endX, endY); int width = workingRectangle.Width; - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( workingRectangle, configuration, (rows, vectorBuffer) => diff --git a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs index faffbd5758..33b80688ca 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs @@ -68,7 +68,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution var workingRectangle = Rectangle.FromLTRB(startX, startY, endX, endY); int width = workingRectangle.Width; - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( workingRectangle, this.Configuration, (rows, vectorBuffer) => diff --git a/src/ImageSharp/Processing/Processors/Effects/PixelRowDelegateProcessorBase{TPixel}.cs b/src/ImageSharp/Processing/Processors/Effects/PixelRowDelegateProcessorBase{TPixel}.cs index bca2887832..8926ddc661 100644 --- a/src/ImageSharp/Processing/Processors/Effects/PixelRowDelegateProcessorBase{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Effects/PixelRowDelegateProcessorBase{TPixel}.cs @@ -39,7 +39,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Effects Configuration configuration = this.Configuration; PixelConversionModifiers modifiers = this.modifiers; - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( interest, this.Configuration, (rows, vectorBuffer) => diff --git a/src/ImageSharp/Processing/Processors/Filters/FilterProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Filters/FilterProcessor{TPixel}.cs index 64d705a2f7..a8ce67af3b 100644 --- a/src/ImageSharp/Processing/Processors/Filters/FilterProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Filters/FilterProcessor{TPixel}.cs @@ -38,7 +38,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Filters ColorMatrix matrix = this.definition.Matrix; - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( interest, this.Configuration, (rows, vectorBuffer) => diff --git a/src/ImageSharp/Processing/Processors/Overlays/GlowProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Overlays/GlowProcessor{TPixel}.cs index 0032a79839..0271caa5d4 100644 --- a/src/ImageSharp/Processing/Processors/Overlays/GlowProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Overlays/GlowProcessor{TPixel}.cs @@ -81,7 +81,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Overlays { rowColors.GetSpan().Fill(glowColor); - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( workingRect, configuration, (rows, amounts) => diff --git a/src/ImageSharp/Processing/Processors/Overlays/VignetteProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Overlays/VignetteProcessor{TPixel}.cs index 95fe35b091..55cacccdf9 100644 --- a/src/ImageSharp/Processing/Processors/Overlays/VignetteProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Overlays/VignetteProcessor{TPixel}.cs @@ -85,7 +85,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Overlays { rowColors.GetSpan().Fill(vignetteColor); - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( workingRect, configuration, (rows, amounts) => diff --git a/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs b/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs index 0abc9aa130..243ffe2204 100644 --- a/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs +++ b/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs @@ -136,7 +136,7 @@ namespace SixLabors.ImageSharp.Tests.Helpers var bufferHashes = new ConcurrentBag(); int actualNumberOfSteps = 0; - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( rectangle, parallelSettings, (RowInterval rows, Memory buffer) => @@ -179,7 +179,7 @@ namespace SixLabors.ImageSharp.Tests.Helpers int[] expectedData = Enumerable.Repeat(0, minY).Concat(Enumerable.Range(minY, maxY - minY)).ToArray(); var actualData = new int[maxY]; - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( rectangle, parallelSettings, (RowInterval rows, Memory buffer) => @@ -262,7 +262,7 @@ namespace SixLabors.ImageSharp.Tests.Helpers var rectangle = new Rectangle(0, 0, width, height); int actualNumberOfSteps = 0; - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( rectangle, parallelSettings, (RowInterval rows, Memory buffer) => @@ -371,7 +371,7 @@ namespace SixLabors.ImageSharp.Tests.Helpers var rect = new Rectangle(0, 0, width, height); ArgumentOutOfRangeException ex = Assert.Throws( - () => ParallelRowIterator.IterateRowsWithTempBuffer(rect, parallelSettings, (rows, memory) => { })); + () => ParallelRowIterator.IterateRows(rect, parallelSettings, (rows, memory) => { })); Assert.Contains(width <= 0 ? "Width" : "Height", ex.Message); } diff --git a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs index 4c9318f93b..9aaca3e3fc 100644 --- a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs +++ b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs @@ -702,7 +702,7 @@ namespace SixLabors.ImageSharp.Tests { Rectangle sourceRectangle = this.SourceRectangle; Configuration configuration = this.Configuration; - ParallelRowIterator.IterateRowsWithTempBuffer( + ParallelRowIterator.IterateRows( sourceRectangle, configuration, (rows, temp) =>