Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImageSharp needs to pre-seed generics to avoid "Attempting to JIT compile method... while running in aot-only mode." #52559

Open
MichalStrehovsky opened this issue May 10, 2021 · 15 comments
Assignees
Milestone

Comments

@MichalStrehovsky
Copy link
Member

Brought up on Twitter here: https://twitter.com/James_M_South/status/1391758920117538827

See the comments at the top of the file:

https://github.com/SixLabors/ImageSharp/blob/master/src/ImageSharp/Advanced/AotCompilerTools.cs

From the Twitter exchange, this is not caused by unpredictable dynamic reflection (MakeGenericMethod/MakeGenericType and friends). It doesn't look right that we would need this for "regular" C# - I think this is an AOT compiler bug.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label May 10, 2021
@SamMonoRT
Copy link
Member

@vargaz @lambdageek any thoughts ?

@SamMonoRT SamMonoRT added this to the 6.0.0 milestone Jun 15, 2021
@vargaz
Copy link
Contributor

vargaz commented Jun 15, 2021

Would need to see the actual errors this one works around.

@JimBobSquarePants
Copy link

Here's all the issues tagged AOT we've faced.
https://github.com/SixLabors/ImageSharp/issues?q=aot+label%3Aarea%3Aaot

I have zero experience building Xamarin or UWP apps so I can only go with what I have been told.

@SamMonoRT SamMonoRT removed the untriaged New issue has not been triaged by the area owner label Jun 25, 2021
@SamMonoRT
Copy link
Member

cc @vargaz

@vargaz
Copy link
Contributor

vargaz commented Jul 16, 2021

This is not a 6.0 issue.

@antonfirsov
Copy link
Member

antonfirsov commented Nov 28, 2021

@vargaz this is related to cases when there are open generic factory methods creating generic subclasses through generic interfaces, and the generic factory methods are not directly invoked in application code. Here is simplified version of stuff we do in ImageSharp:

// Library code in a netstandard dll:

public interface IPixel { void Foo(); }
public interface IPixel<TPixel> : IPixel { void Bar(); }
public struct Rgb : IPixel<Rgb> {
    public void Foo() { }
    public void Bar() { }
}

public interface IProcessor {
    IProcessor<TPixel> CreatePixelSpecificProcessor() where TPixel : unmanaged, IPixel<TPixel>;
}

public interface IProcessor<TPixel> where TPixel : unmanaged, IPixel<TPixel> { void Execute(Image<TPixel> image); }

public class ShinyProcessor : IProcessor {
    public IProcessor<TPixel> CreatePixelSpecificProcessor() where TPixel : unmanaged, IPixel<TPixel> => new ShinyProcessor<TPixel>();
}

public class ProcessorBase<TPixel> where TPixel : unmanaged, IPixel<TPixel> { public abstract void Execute(); }

public class ShinyProcessor : ProcessorBase<TPixel> where TPixel : unmanaged, IPixel<TPixel> {
    public override void Execute(Image<TPixel> image) { }
}

public class Image {
    abstract void ExecuteProcessor(IProcessor processor);
    public static void Load() => new Image<Rgb>();
}

public class Image<TPixel> : Image where TPixel : unmanaged, IPixel<TPixel> {
    override void ExecuteProcessor(IProcessor processor) => processor.CreatePixelSpecificProcessor<TPixel>().Execute(this);
}

static class ShinyExtensions {
    public static void Shine(this Image image) => image.ExecuteProcessor(new ShinyProcessor());
}

// Xamarin app code:

var image = Image.Load();
image.Shine();

No idea if the generic TPixel constraints or other details have any significance, shared them for completeness. In the actual library there is even more virtual dispatch around the stuff I simplified here as Image.ExecuteProcessor(). If the code above doesn't help you obtaining a repro, I can share a real library use case that fails today with Xamarin.

@vargaz
Copy link
Contributor

vargaz commented Nov 28, 2021

Is this something that happens with the current version of xamarin.ios ? If it is, what is the exact error message, and what is the method whose AOT code is not found without these changes ?

@antonfirsov
Copy link
Member

I'm not a Xamarin user myself, don't know how to try it, working based on the reports of ImageSharp consumers, one of the recent reports mentions Xamarin.Forms 5.0.0.2012. Error message is always something like:

System.ExecutionEngineException: Attempting to JIT compile method 'Foo.Bar<SomePixelType>(...)' while running in aot-only mode. See https://docs.microsoft.com/xamarin/ios/internals/limitations for more information.

These reports have been around for a very long time, for me it seemed that it's the mentioned AotCompilerTools.cs workaround that made them less frequent recently. Was there a recent fix from which you expect this should be gone? Any chance we can work towards a confirmation if it's fixed or not considering my limited understanding on Xamarin? I can for example create a custom build without the mentioned workaround and provide a code snippet that is expected to fail, if that helps.

@vargaz
Copy link
Contributor

vargaz commented Nov 29, 2021

With recent versions of xamarin.ios, these errors should not happen except in very special cases. Having a test case would help.

@antonfirsov
Copy link
Member

Doesn't Xamarin.Forms 5.0.0.2012 imply recent Xamarin.iOS? Or are these versions unrelated?

@vargaz
Copy link
Contributor

vargaz commented Nov 29, 2021

Looks like there were recent fixes in this area, but they are only in dotnet/runtime, not in the mono/mono version used by current versions of xamarin.ios. So its possible that these workarounds are still needed. Also, even if its fixed, without having access to these pre-seeded generic instances, the runtime could fall back to much slower implementations, so keeping these workarounds is still useful even if the original problem is fixed.

@antonfirsov
Copy link
Member

antonfirsov commented Nov 29, 2021

So the next release will be based on .NET 6 mono right? Are there any regression tests to make sure this stuff will not be broken in the future?

Also, even if its fixed, without having access to these pre-seeded generic instances, the runtime could fall back to much slower implementations

Shouldn't be this considered as a bug, according to OP? Those interpreted code paths are reported to be 60x slower than they should be, rendering such code useless in practice. The workarounds are very hard to manage from library developer's perspective, with two generic arguments the combinations can explode. Also - as mentioned by @MichalStrehovsky in SixLabors/ImageSharp#1703 (comment) - seeding will trigger generation of unused code.

@antonfirsov
Copy link
Member

antonfirsov commented Nov 29, 2021

If the fix means that slow code is generated for everything that is not pre-seeded, the situation is far from good IMHO.

@MichalStrehovsky
Copy link
Member Author

SixLabors/ImageSharp#1703 is about Unity IL2CPP specifically, so it's not a good Xamarin example. But the underlying issue might be similar. I think a lot of these (maybe all?) will be around generic virtual method dispatch - the exception in SixLabors/ImageSharp#1703 complains about PngDecoderCore::Decode<SixLabors.ImageSharp.PixelFormats.Rgba32> and that's a generic virtual method (the method implements a generic interface method).

@SamMonoRT
Copy link
Member

Moving to 8.0.0

@SamMonoRT SamMonoRT modified the milestones: 7.0.0, 8.0.0 Aug 11, 2022
@jonathanpeppers jonathanpeppers moved this from Todo: Performance to Future: Performance in .NET 7 MAUI Performance Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants