Faster Transforms #1118

JimBobSquarePants · 2020-02-21T13:21:02Z

Prerequisites

I have written a descriptive pull-request title
I have verified that there are no overlapping pull-requests open
I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
I have provided test coverage for my change (where applicable)

Description

Inverts the call direction of all transforms to allow inlining and improve performance.
Fix invisible pixel bleed in transforms.
Converts IResampler implementations into readonly struct

Bug Fixes

The attached image shows the difference in output following the updates. RGB color information no longer leaks beyond the bounds of the transformed result.

Benchmarks

Massive performance improvements in Affine and Projective transforms.

Rotate

Before

Method	Runtime	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
DoRotate	.NET 4.7.2	64.96 ms	47.74 ms	2.617 ms	-	-	-	11.56 KB
DoRotate	.NET Core 2.1	38.57 ms	14.38 ms	0.788 ms	-	-	-	4.36 KB
DoRotate	.NET Core 3.1	38.67 ms	50.52 ms	2.769 ms	-	-	-	5.68 KB

After

Method	Runtime	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
DoRotate	.NET 4.7.2	28.77 ms	3.304 ms	0.181 ms	-	-	-	6.5 KB
DoRotate	.NET Core 2.1	16.27 ms	1.044 ms	0.057 ms	-	-	-	5.25 KB
DoRotate	.NET Core 3.1	17.12 ms	4.352 ms	0.239 ms	-	-	-	6.57 KB

Skew

Before

Method	Runtime	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
DoSkew	.NET 4.7.2	42.78 ms	11.455 ms	0.628 ms	-	-	-	9.33 KB
DoSkew	.NET Core 2.1	27.71 ms	3.880 ms	0.213 ms	-	-	-	4.34 KB
DoSkew	.NET Core 3.1	25.06 ms	6.461 ms	0.354 ms	-	-	-	5.63 KB

After

Method	Runtime	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
DoSkew	.NET 4.7.2	24.60 ms	33.971 ms	1.862 ms	-	-	-	6.5 KB
DoSkew	.NET Core 2.1	12.13 ms	2.256 ms	0.124 ms	-	-	-	5.21 KB
DoSkew	.NET Core 3.1	12.83 ms	1.442 ms	0.079 ms	-	-	-	6.57 KB

Resize

Resize improvements are less dramatic which is expected. NET Core 3.1 shows a nice improvement though bringing it inline with NETFX. A slight improvement overall.

Before

Method	Runtime	SourceToDest	Mean	Error	StdDev	Ratio	RatioSD	Gen 0	Gen 1	Gen 2	Allocated
SystemDrawing	.NET 4.7.2	3032-400	82.97 ms	5.478 ms	0.300 ms	1.00	0.00	-	-	-	1170 B
'ImageSharp, MaxDegreeOfParallelism = 1'	.NET 4.7.2	3032-400	51.53 ms	3.713 ms	0.203 ms	0.62	0.00	-	-	-	16384 B

SystemDrawing	.NET Core 2.1	3032-400	82.83 ms	2.049 ms	0.112 ms	1.00	0.00	-	-	-	96 B
'ImageSharp, MaxDegreeOfParallelism = 1'	.NET Core 2.1	3032-400	62.95 ms	5.588 ms	0.306 ms	0.76	0.00	-	-	-	15560 B

SystemDrawing	.NET Core 3.1	3032-400	86.34 ms	80.550 ms	4.415 ms	1.00	0.00	-	-	-	96 B
'ImageSharp, MaxDegreeOfParallelism = 1'	.NET Core 3.1	3032-400	57.54 ms	86.545 ms	4.744 ms	0.67	0.08	-	-	-	15536 B

After

Method	Runtime	SourceToDest	Mean	Error	StdDev	Ratio	RatioSD	Gen 0	Gen 1	Gen 2	Allocated
SystemDrawing	.NET 4.7.2	3032-400	80.61 ms	11.068 ms	0.607 ms	1.00	0.00	-	-	-	1365 B
'ImageSharp, MaxDegreeOfParallelism = 1'	.NET 4.7.2	3032-400	49.10 ms	3.663 ms	0.201 ms	0.61	0.00	-	-	-	21372 B

SystemDrawing	.NET Core 2.1	3032-400	80.30 ms	13.570 ms	0.744 ms	1.00	0.00	-	-	-	96 B
'ImageSharp, MaxDegreeOfParallelism = 1'	.NET Core 2.1	3032-400	61.99 ms	20.643 ms	1.132 ms	0.77	0.01	-	-	-	19816 B

SystemDrawing	.NET Core 3.1	3032-400	81.54 ms	49.267 ms	2.701 ms	1.00	0.00	-	-	-	96 B
'ImageSharp, MaxDegreeOfParallelism = 1'	.NET Core 3.1	3032-400	49.75 ms	0.764 ms	0.042 ms	0.61	0.02	-	-	-	19868 B

src/ImageSharp/Processing/Processors/Transforms/Resize/ResamplerExtensions.cs

codecov · 2020-02-21T13:45:58Z

Codecov Report

Merging #1118 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1118      +/-   ##
==========================================
+ Coverage   82.21%   82.22%   +0.01%     
==========================================
  Files         678      678              
  Lines       29118    29117       -1     
  Branches     3300     3300              
==========================================
+ Hits        23938    23942       +4     
+ Misses       4484     4480       -4     
+ Partials      696      695       -1

Flag	Coverage Δ
#unittests	`82.22% <ø> (+0.01%)`	⬆️

Impacted Files	Coverage Δ
src/ImageSharp/Formats/Png/PngDecoderCore.cs	`89.12% <ø> (+1.02%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b25e102...c1b4bbd. Read the comment docs.

antonfirsov

Did not have the chance for an extensive review yet, sharing a few comments and API-related concerns for now.

antonfirsov · 2020-02-23T00:07:29Z

src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernel.cs

-        public int StartIndex { get; }
+        public int StartIndex
+        {
+            [MethodImpl(InliningOptions.ShortMethod)]


I think the JIT is smart enough to do this without our intervention. I see the BCL code pushing back a lot on applications for explicit AggressiveInlining these days, I suppose the JIT does it more reliably with really short methods now.

It's more about expressing intent than anything. The JIT can ignore our suggestion but I believe we should still declare that we want the method inlined.

Hmm that makes sense, but in trivial cases like property getters it gets overly verbose and hurts readability IMO. (Technically all autoproperies should be marked then.)

src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs

src/ImageSharp/Processing/Processors/Transforms/IResampler.cs

src/ImageSharp/Processing/Processors/Transforms/Resize/ResamplerExtensions.cs

…mageSharp into js/faster-transforms

antonfirsov · 2020-02-23T09:59:22Z

I think I have an idea, will prototype it in a few days (and also do a more comprehensive review here).

JimBobSquarePants · 2020-02-23T13:12:30Z

@antonfirsov I just had another crack at this.

I've introduced a new interface IResamplingImageProcessor<TPixel>

    /// <summary>
    /// Implements an algorithm to alter the pixels of an image via a resampling transforms.
    /// </summary>
    /// <typeparam name="TPixel">The pixel format.</typeparam>
    public interface IResamplingImageProcessor<TPixel> : IImageProcessor<TPixel>
        where TPixel : struct, IPixel<TPixel>
    {
        /// <summary>
        /// Applies a resampling transform with the given sampler.
        /// </summary>
        /// <typeparam name="TResampler">The type of sampler.</typeparam>
        /// <param name="sampler">The sampler to use.</param>
        void ApplyTransform<TResampler>(in TResampler sampler)
            where TResampler : struct, IResampler;
    }

IResampler now looks like this.

    /// <summary>
    /// Encapsulates an interpolation algorithm for resampling images.
    /// </summary>
    public interface IResampler
    {
        /// <summary>
        /// Gets the radius in which to sample pixels.
        /// </summary>
        float Radius { get; }

        /// <summary>
        /// Gets the result of the interpolation algorithm.
        /// </summary>
        /// <param name="x">The value to process.</param>
        /// <returns>
        /// The <see cref="float"/>
        /// </returns>
        float GetValue(float x);

        /// <summary>
        /// Applies a transformation upon an image.
        /// </summary>
        /// <typeparam name="TPixel">The pixel format.</typeparam>
        /// <param name="processor">The transforming image processor.</param>
        void ApplyTransform<TPixel>(IResamplingImageProcessor<TPixel> processor)
            where TPixel : struct, IPixel<TPixel>;
    }

With implementations like this.

    /// <summary>
    /// The function implements the nearest neighbor algorithm. This uses an unscaled filter
    /// which will select the closest pixel to the new pixels position.
    /// </summary>
    public readonly struct NearestNeighborResampler : IResampler
    {
        /// <inheritdoc/>
        public float Radius => 1;

        /// <inheritdoc/>
        [MethodImpl(InliningOptions.ShortMethod)]
        public float GetValue(float x) => x;

        /// <inheritdoc/>
        [MethodImpl(InliningOptions.ShortMethod)]
        public void ApplyTransform<TPixel>(IResamplingImageProcessor<TPixel> processor)
            where TPixel : struct, IPixel<TPixel>
            => processor.ApplyTransform(in this);
    }

Everything else is now internal. I moved things about a little to reorganize also.

Performance remains the same as the benchmarks.

hypeartist · 2020-02-23T22:06:04Z

    /// <summary>
    /// Implements an algorithm to alter the pixels of an image via a resampling transforms.
    /// </summary>
    /// <typeparam name="TPixel">The pixel format.</typeparam>
    public interface IResamplingImageProcessor<TPixel> : IImageProcessor<TPixel>
        where TPixel : struct, IPixel<TPixel>
    {
        /// <summary>
        /// Applies a resampling transform with the given sampler.
        /// </summary>
        /// <typeparam name="TResampler">The type of sampler.</typeparam>
        /// <param name="sampler">The sampler to use.</param>
        void ApplyTransform<TResampler>(in TResampler sampler)
            where TResampler : struct, IResampler;
    }

Btw, if I am not mistaken, the method void ApplyTransform<TResampler> won't be inlined even in case the interface IResamplingImageProcessor<TPixel> is implemented by struct due some JIT limitations. I guess @jkotas could clarify this.

JimBobSquarePants · 2020-02-23T22:15:55Z

I’m not so concerned by that method being inlined (it would be nice). Much more important is that the TResampler struct is concrete as that’s what all the actual convolution hot paths use.

antonfirsov · 2020-02-24T00:47:43Z

@JimBobSquarePants the IResamplingImageProcessor<TPixel> trick is great, actually better than the stuff I wanted, we should apply it to the ditherers as well in a follow-up PR.

src/ImageSharp/Processing/Processors/Transforms/Automorphic/AffineTransformProcessor{TPixel}.cs

src/ImageSharp/Processing/Processors/Transforms/Automorphic/AutomorphicTransformUtilities.cs

src/ImageSharp/Processing/Processors/Transforms/IResamplingImageProcessor{TPixel}.cs

JimBobSquarePants · 2020-02-26T00:29:18Z

Imma gonna merge this.

Sergio0694 · 2020-02-26T01:58:41Z

antonfirsov

LGTM. Looks like I spotted everything in during my previous lazy reviews.

JimBobSquarePants added 7 commits February 21, 2020 13:58

Apply Transforms 33% faster

457adb1

Merge branch 'af/disco-buffers' into js/faster-transforms

4dd536d

Update ResamplerExtensions.cs

31c076d

Inline resizing sampler.

14e2a1e

Merge branch 'af/disco-buffers' into js/faster-transforms

ae18de6

Update benchmarks

0b1cfb0

Fix for tests, update reference images

e1005f2

JimBobSquarePants added area:performance area:transforms labels Feb 21, 2020

JimBobSquarePants added this to the 1.0.0-rc1 milestone Feb 21, 2020

JimBobSquarePants requested a review from a team February 21, 2020 13:21

Merge branch 'master' into js/faster-transforms

b304c9e

JimBobSquarePants commented Feb 21, 2020

View reviewed changes

src/ImageSharp/Processing/Processors/Transforms/Resize/ResamplerExtensions.cs Outdated Show resolved Hide resolved

Merge branch 'master' into js/faster-transforms

8e5c600

antonfirsov reviewed Feb 23, 2020

View reviewed changes

JimBobSquarePants added 3 commits February 23, 2020 13:08

Make ResizeKernelMap non-generic.

f9fb732

Merge branch 'js/faster-transforms' of https://github.com/SixLabors/I…

15646ed

…mageSharp into js/faster-transforms

Simplify Resamplers

3b4aa15

Simplify IResampler contract

585b7be

antonfirsov reviewed Feb 24, 2020

View reviewed changes

src/ImageSharp/Processing/Processors/Transforms/Automorphic/AffineTransformProcessor{TPixel}.cs Outdated Show resolved Hide resolved

antonfirsov reviewed Feb 24, 2020

View reviewed changes

src/ImageSharp/Processing/Processors/Transforms/Automorphic/AutomorphicTransformUtilities.cs Outdated Show resolved Hide resolved

antonfirsov reviewed Feb 24, 2020

View reviewed changes

src/ImageSharp/Processing/Processors/Transforms/IResamplingImageProcessor{TPixel}.cs Outdated Show resolved Hide resolved

Update based on feedback.

b25e102

Merge branch 'master' into js/faster-transforms

c1b4bbd

antonfirsov approved these changes Feb 26, 2020

View reviewed changes

JimBobSquarePants merged commit 9f59241 into master Feb 26, 2020

JimBobSquarePants deleted the js/faster-transforms branch February 26, 2020 22:28

Uh oh!

Faster Transforms #1118

Faster Transforms #1118

Uh oh!

Conversation

JimBobSquarePants commented Feb 21, 2020

Prerequisites

Description

Bug Fixes

Benchmarks

Rotate

Skew

Resize

Uh oh!

Uh oh!

codecov bot commented Feb 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

antonfirsov left a comment

Choose a reason for hiding this comment

Uh oh!

antonfirsov Feb 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JimBobSquarePants Feb 23, 2020

Choose a reason for hiding this comment

Uh oh!

antonfirsov Feb 23, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

antonfirsov commented Feb 23, 2020

Uh oh!

JimBobSquarePants commented Feb 23, 2020

Uh oh!

hypeartist commented Feb 23, 2020

Uh oh!

JimBobSquarePants commented Feb 23, 2020

Uh oh!

antonfirsov commented Feb 24, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JimBobSquarePants commented Feb 26, 2020

Uh oh!

Sergio0694 commented Feb 26, 2020

Uh oh!

antonfirsov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Feb 21, 2020 •

edited

Loading

antonfirsov Feb 23, 2020 •

edited

Loading

antonfirsov left a comment •

edited

Loading