Repository navigation
Performance optimization opportunities in common pixel formats. #2232
Description
Activity
ImageSharp is .NET 6+ now, correct?
If you want to assign this to me, I can work on getting a fix up and take a peek at some of the other SIMD code for other .NET 6+ specific improvements.
Reacted by James Jackson-SouthImageSharp is .NET 6+ now, correct?
If you want to assign this to me, I can work on getting a fix up and take a peek at some of the other SIMD code for other .NET 6+ specific improvements.
@tannergooding yes it's .NET6+ now. I have assigned you to this task, thanks!
Reacted by James Jackson-SouthJimBobSquarePants commented
on Sep 16, 2022 MemberAuthorMore actions@tannergooding Any and all suggestions will be deeply appreciated 😄
Few questions...
- How would you like me to structure the PR(s)?
- What's the consideration for
#ifdefvs providing helper methods? - What's the recommended way to benchmark and present numbers as part of a PR?
- How much "freedom" is there in changing internal implementation details of structs/methods?
For 1, I'm mostly asking as I can try and do several "smallish" PRs each targeting a specific type or I can do large PRs that try and cover changes more holistically across the repo.
For 2, mostly asking what the preferred way of handling cases such as where
Arm64support might be added. For .NET 6, we have to directly useAdvSimd. While for .NET 7, you can just use the methods/operators directly onVector128<float>and the JIT will do the "optimal" thing for x64 or AdvSimd or WASM or other future platforms.For 3, naturally I'd like to show numbers actually indicating changes are wins and in a format expected by the repo. I didn't see any callouts in the contributing docs, but maybe I missed it.
For 4, as an example there are a number of structs that contain 2/3/4 float fields (often exposed via auto-props). Due to the JIT specializing Vector2/3/4 but not user-defined structs today (hopefully I can get this fixed eventually, so all qualifying types get this same specialization), you will likely get better codegen by wrapping a single
Vector2/3/4instead.Additionally there is the potential to wrap
Vector128<float>for "even better" perf. However, the last one comes with a consideration that the type can no longer be used for interop, if that's a consideration at all. -- There is an easy workaround of getting the backingVector128<float>field and using.AsVector4()if interop is a consideration, but it is something to think about.There's also a slightly more complex consideration in that while
Vector2/3/4are currently accelerated and generally performant, types likeMatrix3x2,Matrix4x4, and other neighboring types are not (I'm going to try and improve that in .NET 8, but no guarantees). I could write-up more performant helpers and utilizeUnsafe.Asto achieve significant perf wins for .NET 6/7 without changing the public surface area from taking theSystem.Numericstypes.Also some cases where
Vector128<float>is a better choice thanVector4in .NET 6, but where they are effectively the same in .NET 7. The.AsVector4()and.AsVector128()APIs allow zero-cost conversion in .NET 6+, so usingVector128<float>in internal methods can provide wins.JimBobSquarePants commented
on Sep 18, 2022 MemberAuthorMore actionsApologies for the slow reply, was offine this weekend.
I'll try to summarise a response to each question for you.
- Since the focus is mostly on pixel formats here, I think we can err towards the larger side. I can see a lot of repitition on the horizon so it should be ok to review.
- I'd like to avoid
#ifdefwhere possible since we are only targetting the single framework. If we addArm64(_something we really need to get a performant build process in place for - QEMU is far too slow!) we can useAdvSimdand when .NET 8 comes around, we'll switch target to that and incrementally replace the methods to use the new APIs as and when we can. - Benchmarking is a little adhoc. There is a folder in the benchmark project under
General/PixelConversionwhere you can place benchmarks in whatever format you see fit in. - You have absolute freedom there, as long as the changes do not make maintaining the codebase more difficult. I'd personally love to put a lot of focus into streamlining and simplifying some of the internal code but there's still some other chunky tasks - Optimize pixel blending with integer arithmetics #1433 - that need to be done.
Thanks again for your help here!
Reacted by Tanner GoodingThanks a lot for the responses, they all make sense to me!
Reacted by James Jackson-SouthJust wanted to give an update on this. I've been familiarizing myself with the codebase and the various SIMD code, making notes of possible improvements as I go.
I'll be closing on my first house and moving over the next couple weeks so will endup taking a short break, but hope to have a PR up and a bullet list of potential additional improvements closer to the end of the month.
Congratulations on the house!
No worries, enjoy your break. Looking forward to seeing what you come up with!
Reacted by Tanner Gooding-- Am still working on this. Codegen wasn't doing what I expected so I spent some time fixing that up in the JIT (believe you've seen the PR already 😄).
With the changes I've made, the codegen on my local changes is significantly better. So should provide some nice wins off the bat and even better gains once 8.0 comes around.
Reacted by Günther Foidl, Brian Popow and James Jackson-SouthJimBobSquarePants commented
on Dec 17, 2022 MemberAuthorMore actionsYeah, I saw that PR 😃 awesome stuff!
NET 8 seems like it's going to allow massive improvements to the ImageSharp codebase. Simplified SIMD with out of the box ARM support will be a gamechanger.
Reacted by Tanner GoodingAlso got a rewrite of Matrix4x4 and Matrix3x2 in: dotnet/runtime#80091
This resulted in perf improvements of 2x up to 48x and should have a huge positive impact on ImageSharp. There is also opportunity for me to rewrite/improve Plane and Quaternion, but I didn't see any broad impact in my initial profile captures.
I plan on rerunning ImageSharp perf benchmarks once I have a
dotnet/installerbuild available, but that represents the largest point of unexpected perf I was seeing. After that, I can finish up on the changes I've been working on here and it should be even better.Reacted by Dmitry KushnirReacted by Scott Williams, James Jackson-South and Dmitry KushnirJimBobSquarePants commented
on Jan 11, 2023 MemberAuthorMore actionsHaha.... You're a whole new level of awesome. I was expecting a few small changes; you're rewriting the runtime for the benefit of all. Truly amazing!
JimBobSquarePants commented
on Jan 15, 2023 MemberAuthorMore actionsI think our ColorMatrix type can benefit from copying those changes.
Reacted by Tanner Gooding
Prerequisites
DEBUGandRELEASEmodeImageSharp version
v3 alpha +
Other ImageSharp packages and versions
NA
Environment (Operating system, version and so on)
NA
.NET Framework version
NA
Description
As described here there are several performance opportunities can be implemented in many of our pixel format types. This should be fairly low hanging fruit with good return.
Notably on .NET 6/7, you could make this even more efficient by doing something like:
This converts all 4 elements at once and then extracts the truncated bytes directly:
vpextrbmore in the future so it can be justvpextrb [rcx+2], xmm0, 0instead ofvpextrb eax, xmm0, 0followed bymov [rcx+2], al.You can also optimize in .NET 6+ by directly using
Vector128.Create(). This creates a method local constant and avoids the static initializer entirely:Steps to Reproduce
NA
Images
No response