Add more seeding to the AoT compiler for iOS #785
Conversation
|
|
| { | ||
| decoder.Decode<TPixel>(Configuration.Default, null); | ||
| } | ||
| catch |
There was a problem hiding this comment.
This isn't nice, but can't think of a better solution.
There was a problem hiding this comment.
Yeah I agree. Would be nice if there were a way to make a dummy call to Decode without generating exceptions.
| /// <summary> | ||
| /// Seeds the <see cref="System.Runtime.CompilerServices.Unsafe"/> calls. | ||
| /// </summary> | ||
| public static void SeedUnsafe() |
There was a problem hiding this comment.
Looks like an implementation detail from the outside.
Can't we hide this from the outside invoking it internally in a lazy way when Seed<TPixel> (and other variants) are invoked?
There was a problem hiding this comment.
I'll lazy call this (once) from Seed<TPixel> and push.
There was a problem hiding this comment.
Calling it from the static constructor instead, since that's guaranteed to only execute once.
antonfirsov
left a comment
antonfirsov
left a comment
There was a problem hiding this comment.
Yeah, better to use the static constructor in this case!
Looks like also seeding the encoder is an easy addition, but I may miss something here.
| /// </summary> | ||
| /// <param name="decoder">The image decoder to seed..</param> | ||
| /// <typeparam name="TPixel">The pixel format.</typeparam> | ||
| private static void AotDecoder<TPixel>(IImageDecoder decoder) |
There was a problem hiding this comment.
Isn't it worth to also seed both decoder and encoder instead?
private static void SeedCodecs<TPixel>(IImageDecoder decoder, IImageEncoder encoder)
{
// invoke both decoder and encoder in separate try-catch blocks
}
|
I'm not sure what the deal is with that CI error, I can't see how any of my changes could have triggered it. |
Codecov Report
@@ Coverage Diff @@
## master #785 +/- ##
==========================================
+ Coverage 88.7% 88.71% +0.01%
==========================================
Files 1012 1012
Lines 43248 43270 +22
Branches 3127 3127
==========================================
+ Hits 38363 38388 +25
- Misses 4181 4183 +2
+ Partials 704 699 -5
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #785 +/- ##
==========================================
+ Coverage 88.66% 88.71% +0.04%
==========================================
Files 1012 1012
Lines 43222 43270 +48
Branches 3120 3127 +7
==========================================
+ Hits 38324 38388 +64
- Misses 4160 4183 +23
+ Partials 738 699 -39
Continue to review full report at Codecov.
|
|
It was a typical intermittent test failure, related to test-only code. Restarting the build helped. |
antonfirsov
left a comment
antonfirsov
left a comment
There was a problem hiding this comment.
@JimBobSquarePants anything against merging this now?
|
@antonfirsov Nope. I'll update from master and get it merged. @swoolcock Thanks for your help!
On that note, it's due to the current lack of SIMD support on those platforms - Our hands are tied by MS. There is improved support coming in NET Core 3.0 and future Mono though so fingers crossed we get what we need. |
4 participants
Prerequisites
Description
Adds some calls to
Unsafe.SizeOffor various primitives and structs to prevent JIT errors on iOS.Also seeds the decoders for a given pixel format, which should solve #776.
Note that although JPEG will now also load without crashing, it is incredibly slow on Xamarin.iOS. A 1366x768 JPEG takes about 5 seconds to load on an iPad Pro.
As per the previous PR #767, this is a difficult one to write tests for.