-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Comments
@vargaz @lambdageek any thoughts ? |
Would need to see the actual errors this one works around. |
Here's all the issues tagged AOT we've faced. I have zero experience building Xamarin or UWP apps so I can only go with what I have been told. |
cc @vargaz |
This is not a 6.0 issue. |
@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 |
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 ? |
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
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. |
With recent versions of xamarin.ios, these errors should not happen except in very special cases. Having a test case would help. |
Doesn't Xamarin.Forms 5.0.0.2012 imply recent Xamarin.iOS? Or are these versions unrelated? |
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. |
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?
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. |
If the fix means that slow code is generated for everything that is not pre-seeded, the situation is far from good IMHO. |
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 |
Moving to 8.0.0 |
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.
The text was updated successfully, but these errors were encountered: