dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.26k stars 4.73k forks source link

Very Poor ImageSharp Performance in MAUI Android #71210

Open david-maw opened 2 years ago

david-maw commented 2 years ago

Description

A piece of code which executes in under 100 ms in Xamarin forms on Android or Windows and under 50 ms in Windows MAUI takes over 30 seconds in Android MAUI.

Steps to Reproduce

  1. Clone repository https://github.com/david-maw/ResizeImageMaui.git from github
  2. Build the project
  3. Run it on Windows
  4. Click the button, on Windows the image will be converted to greyscale in under a second (50 ms in my case).
  5. Try it again on Android, this time the conversion will take much longer (in my case it was over 30 seconds but less than a minute.

FYI there's a similar Xamarin Forms project at https://github.com/david-maw/ResizeImageXamarin.git, in my testing this took about 90 ms to load on Window (so MAUI was almost twice as fast), and just under a second on the Android emulator (so MAUI was 30+ times slower).

This uses SixLabors.ImageSharp to do the image processing so it is likely the implementation of something in that library that's wildly slow on .NET 6 on Android.

Version with bug

6.0 Release Candidate 3

Last version that worked well

Unknown/Other

Affected platforms

Android, I was not able test on other platforms

Affected platform versions

Android 11

Did you find any workaround?

No

Relevant log output

No response

lewing commented 1 year ago

This issue won't be fixed in net6 or 7 any changes are too large to to backport. There have been changes in net8 that will likely impact the performance positively.

mjsb212 commented 1 year ago

So in .net 8 this will be fixed? I switched to SkiSharp as others said & performance is 100x better...but I love imagesharp for many things so this is a bummer

EvgenyMuryshkin commented 1 year ago

Same issue. Native MAUI Downsize works, but rotates image. SkiaSharp works, but also rotates images.

ImageSharp is the only one that works and does not rotate, but slow :( Hope for MAUI 8, migration journey has been terrible.

hallatore commented 1 year ago

@EvgenyMuryshkin Are you trying to load an image with rotation metadata?

Try load it with image instead of bitmap. That ensures correct rotation.

using var image = SKImage.FromEncodedData(imageStream);
return SKBitmap.FromImage(image);
EvgenyMuryshkin commented 1 year ago

@EvgenyMuryshkin Are you trying to load an image with rotation metadata?

Try load it with image instead of bitmap. That ensures correct rotation.

using var image = SKImage.FromEncodedData(imageStream);
return SKBitmap.FromImage(image);

Does not work on iPad, Android seems ok

e012345678 commented 10 months ago

Is there any workaround for the slow performance in ImageSharp for MAUI as of now 2024?

JimBobSquarePants commented 10 months ago

You’re asking the wrong people. Ask the Maui team.

EDIT. Note to self. Never use GitHub on your mobile. I thought the previous comment was in the ImageSharp downstream issue.

MichalStrehovsky commented 10 months ago

You’re asking the wrong people. Ask the Maui team.

Mono team, is there a way to rule out this is not (or confirm it is) caused by gsharedvt? Are there any events generated when gsharedvt code is hot? ImageSharp is generic heavy, it also uses generic virtual methods. Notice the hot stacks have methods instantiated over structs like Rgba32. I could easily see the perf being atrocious if every operation on a pixel changes from "load 32 bits into a register" to "memcpy a statically unknown number of bytes from X to Y". AFAIK Mono AOT lacks the analysis to figure out all necessary generic virtual method bodies and we often end up running gsharedvt versions.

vargaz commented 10 months ago

gsharedvt is only used on ios, not on android.

Redth commented 10 months ago

@SamMonoRT @steveisok any updates / progress in this area? It would be great to have ImageSharp running well on MAUI platforms and WASM!

vargaz commented 10 months ago

Does this really affect wasm ? I tried:

        for (int i = 0; i < 10; ++i) {
            var fs = typeof (Test).Assembly.GetManifestResourceStream ("Wasm.Console.V8.Sample.foo.jpg");
            var image = SixLabors.ImageSharp.Image.Load(fs);
        }

And it runs in about 2s on wasm in AOT mode on a 2000x1500 image, so each decode takes about 0.2s.

vargaz commented 10 months ago

The wasm AOT performance can probably be improved by pre seeding more generics, some methods are still interpreted because the AOT compiler doesn't know about them, like: void SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder.JpegImagePostProcessor:ConvertColorsInto<SixLabors.ImageSharp.PixelFormats.Rgba32> (SixLabors.ImageSharp.ImageFrame`1<SixLabors.ImageSharp.PixelFormats.Rgba32>)

JimBobSquarePants commented 10 months ago

How did you determine which method required pre seeding? We’ve added a lot of method seeding to the library already but it’s been without guidance.

What are the plans going forward to remove the requirement for pre seeding? It feels like a horrible hacky workaround. I would expect compiler analysis to do much better.

vargaz commented 9 months ago

The AOT compiler does appear to have problems figuring out which instances to generate. In this specific case, the caller is ImageDecoderUtilities:Decode<Rgba32> which calls IImageDecoderInternals::Decode on an argument. So in theory, the aot compiler could figure out that the call could possible go to JpegDecoderCore::Decode<Rgba32> and generate that instance. Currently, this kind of analysis is not done.

The ImageSharp codebases unfortunately makes heavy use of generics, interfaces, valuetype generic arguments, etc., which is not very friendly to static compilation.

JimBobSquarePants commented 5 months ago

I can see that this has been removed from a performance backlog and nobody is now assigned. What can be done to change this state?

sharpwood commented 5 months ago

This is a serious issue: ImageSharp cannot be used at all on MAUI Android.

MichalStrehovsky commented 5 months ago

I can see that this has been removed from a performance backlog and nobody is now assigned. What can be done to change this state?

Cc @vitek-karas

JimBobSquarePants commented 3 months ago

Since there's been no movement here, I've created a potential workaround for this particular issue in the ImageSharp codebase, but I have no means to test my changes. Would anyone be able to compile and test the code in this PR

https://github.com/SixLabors/ImageSharp/pull/2762

JimBobSquarePants commented 3 months ago

The sample projects are gone!! 😔

JimBobSquarePants commented 3 months ago

I'd like to note. @beeradmoore has been helping me investigate Android performance issues with MAUI and we've measured that release mode with LLVM performs adequately with the following configuration.

<PropertyGroup Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'android' AND '$(Configuration)' == 'Release'">
    <EnableLLVM>true</EnableLLVM>
    <RunAOTCompilation>true</RunAOTCompilation>
    <AndroidEnableProfiledAot>false</AndroidEnableProfiledAot>
</PropertyGroup>

However, we saw the significant performance issues highlighted in this issue running Android in Debug. I'm unsure whether removing the Release configuration condiftion here allows the library to build and run correctly. Guidance here would be appreciated.

Relevant discussion here:

https://github.com/SixLabors/ImageSharp/pull/2762

jonathanpeppers commented 3 months ago

@JimBobSquarePants the configuration above looks correct to me for Release-mode for ImageSharp usage: "AOT everything" + LLVM.

In Debug-mode, we have the interpreter enabled by default, which probably doesn't play nice with hardware intrinsics / vector-based math. Does UseIntepreter=false make things any better? This will, of course, prevent C# hot reload from working.

JimBobSquarePants commented 3 months ago

@JimBobSquarePants the configuration above looks correct to me for Release-mode for ImageSharp usage: "AOT everything" + LLVM.

In Debug-mode, we have the interpreter enabled by default, which probably doesn't play nice with hardware intrinsics / vector-based math. Does UseIntepreter=false make things any better? This will, of course, prevent C# hot reload from working.

Thanks @jonathanpeppers I don't actually have the tooling to check but this sample application can be used to test against https://github.com/beeradmoore/ImageSharpMAUITest/tree/main

beeradmoore commented 3 months ago

I added this which I changed from true to false to confirm it was being applied.

<PropertyGroup Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'android' AND '$(Configuration)' == 'Debug'">
    <UseInterpreter>true</UseInterpreter>
</PropertyGroup>
Test run JpgLoad JpgResize PngLoad PngResize
UseInterpreter=true 14289.9ms 18556.9ms 123.4ms 344.7ms
UseIntepreter=false 14285.8ms 18491.2ms 122.9ms 349.3ms
UseInterpreter=false 1157.9ms 1387.3ms 35.4ms 42.2ms

What's interesting (and I guess explains it) is in my local source I still had this line enabled

 Console.WriteLine("Vector.IsHardwareAccelerated: " + (System.Numerics.Vector.IsHardwareAccelerated ? "True" : "False"));

This was true for my last set of release mode tests. I just checked and both of these debug modes

Vector.IsHardwareAccelerated: False

With UseInterpreter=true Vector.IsHardwareAccelerated is false, but with UseInterpreter=false Vector.IsHardwareAccelerated is true.

EDIT: This comment was updated to fix the typo UseIntepreter to UseInterpreter and then tests were re-run.

JimBobSquarePants commented 3 months ago

Why would intrinsics be turned off for debug mode? Not-only is that a performance concern but it also vastly transforms the profile of the running code, limiting the ability to identify codegen issues.

jonathanpeppers commented 3 months ago

@beeradmoore did I misspell above UseIntepreter? Sorry. Should be UseInterpreter?

Someone on the Mono team can comment, but it looks like hardware intrinsics for Vector is not implemented for the interpreter. It may be implemented for JIT on Android, if UseInterpreter=false (spelled correctly) makes things faster.

beeradmoore commented 3 months ago

Comment updated and tests re-run with correct spelling. Performance is a lot better, as expected Vector.IsHardwareAccelerated is also reporting true.

Aside from hot reload, is there anything else to consider when using UseInterpreter=false?