dotnet / runtime

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

[mono][perf] iOS disk size regression on 11 Nov 2022 #78851

Open kotlarmilos opened 1 year ago

kotlarmilos commented 1 year ago

Description

Microbenchmark improvement https://github.com/dotnet/runtime/pull/78182 introduced concrete instances in addition to the gshared methods in AOT mode to optimize https://github.com/dotnet/runtime/issues/78163.

For Mono iOS sample app, binary disk size (SOD) has increased by 8%.

Filename SOD without concrete instances (https://github.com/dotnet/runtime/commit/80208e61520ac6dce0acf543d5e25d4599faa7b7) SOD with concrete instances (https://github.com/dotnet/runtime/commit/486682a719e064ed30dd4c6db94f5c34a05ad5f2)
HelloiOS 17382408 bytes 18872480 bytes

Number of symbols has increased by 5k.

Filename Number of symbols without concrete instances (https://github.com/dotnet/runtime/commit/80208e61520ac6dce0acf543d5e25d4599faa7b7) Number of symbols with concrete instances (https://github.com/dotnet/runtime/commit/486682a719e064ed30dd4c6db94f5c34a05ad5f2)
HelloiOS 80007 85179

At the same time https://github.com/dotnet/runtime/commit/80208e61520ac6dce0acf543d5e25d4599faa7b7 excluded dynamic libraries from the bundle as static linking is used.

For Mono iOS sample app, SOD has decreased by 5%.

File Value (bytes)
HelloiOS 17042528
Info.plist 980
PkgInfo 8
Program.aotdata 4792
Program.deps.json 11754
Program.dll 6656
Program.runtimeconfig.json 1064
System.Console.aotdata 41952
System.Console.dll 17408
System.Private.CoreLib.aotdata 826296
System.Private.CoreLib.dll 998912
icudt.dat 1824544
icudt_CJK.dat 956416
icudt_EFIGS.dat 550320
icudt_no_CJK.dat 1071600
(EXCLUDED) libSystem.IO.Compression.Native.dylib 877152
(EXCLUDED) libSystem.Native.dylib 145728
(EXCLUDED) libSystem.Net.Security.Native.dylib 72064
(EXCLUDED) libSystem.Security.Cryptography.Native.Apple.dylib 104384
Screenshot 2022-12-13 at 11 50 40

cc @ivanpovazan

Configuration

OS: iOS Architecture: arm64 .NET version: 7.0.100-rc.1.22431.12

dotnet-issue-labeler[bot] commented 1 year ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

ivanpovazan commented 1 year ago

@kotlarmilos if this affected Mono iOS sample size it probably regressed MAUI iOS sizes as well. Do you know if this is being tracked already?

kotlarmilos commented 1 year ago

Good point, https://github.com/dotnet/runtime/pull/78182 is not backported to .net7 so the regression is not present in MAUI iOS size measurements. I haven't found an issue related to the regression in MAUI iOS.

ivanpovazan commented 1 year ago

/cc @jonathanpeppers

vargaz commented 1 year ago

So the extra methods look to be Vector/Scalar method instances, these were previously shared, but now we generate specialized instances for them, leading to the size increase.

vargaz commented 1 year ago

Most of these methods appear to be the fallback methods in case SIMD is not found. If SIMD is enabled on ios, then this code will not be called, if it's not enabled, then the app should not call it since the performance is not good. So there should be a way to avoid generating code for them.

kotlarmilos commented 1 year ago

Related issues:

https://github.com/dotnet/runtime/issues/71430 https://github.com/dotnet/runtime/issues/71431

ivanpovazan commented 1 year ago

Most of these methods appear to be the fallback methods in case SIMD is not found. If SIMD is enabled on ios, then this code will not be called, if it's not enabled, then the app should not call it since the performance is not good. So there should be a way to avoid generating code for them.

@vargaz just to clarify, are you suggesting that:

vargaz commented 1 year ago

If SIMD is enabled, then these methods are never going to be called. So putting a if (!SIMDIsEnabled) else , and having either the linker or the aot compiler eliminate the if (!SIMDIsEnabled) conditional would make sure that even if these instances are aot-ed, then the method bodies will be very small, just a throw.

ivanpovazan commented 1 year ago

If I am understanding this correctly, something similar was attempted here: https://github.com/dotnet/runtime/issues/71430#issuecomment-1185618983 where in my experimentation I tried to exclude Scalar fallbacks, which turned out to be impossible -> ie software fallbacks are needed even on arm64 architectures. Am I missing the point?

vargaz commented 1 year ago

Was just an idea, not sure whenever it will work.

ivanpovazan commented 1 year ago

I guess this comment: https://github.com/dotnet/perf-autofiling-issues/issues/9599#issuecomment-1310570220 goes along with what Zoltan said. If I understand correctly, before going further, we would need to have SIMD work completed on the Mono side.

fanyang-mono commented 1 year ago

All the SIMD support that we added so far should be fully enabled for full AOT mode, which is what iOS is using. @vargaz Are the size increase here caused by code patterns involving generics, which used to be generated into a gsharedvt function, now generated into concrete type instances to facilitate the previous performance regression?

vargaz commented 1 year ago

Previously, we were emitting shared methods, now we emit the same shared methods (to handle all cases) and concrete instances as well (for better performance).

ghost commented 1 year ago

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT, @marek-safar See info in area-owners.md if you want to be subscribed.

Issue Details
### Description Microbenchmark improvement https://github.com/dotnet/runtime/pull/78182 introduced concrete instances in addition to the gshared methods in AOT mode to optimize https://github.com/dotnet/runtime/issues/78163. For Mono iOS sample app, binary disk size (SOD) has increased by 8%. Filename | SOD without concrete instances (https://github.com/dotnet/runtime/commit/80208e61520ac6dce0acf543d5e25d4599faa7b7) | SOD with concrete instances (https://github.com/dotnet/runtime/commit/486682a719e064ed30dd4c6db94f5c34a05ad5f2) ---|-----|---- HelloiOS | 17382408 bytes | 18872480 bytes Number of symbols has increased by 5k. Filename | Number of symbols without concrete instances (https://github.com/dotnet/runtime/commit/80208e61520ac6dce0acf543d5e25d4599faa7b7) | Number of symbols with concrete instances (https://github.com/dotnet/runtime/commit/486682a719e064ed30dd4c6db94f5c34a05ad5f2) ---|-----|---- HelloiOS | 80007 | 85179
At the same time https://github.com/dotnet/runtime/commit/80208e61520ac6dce0acf543d5e25d4599faa7b7 excluded dynamic libraries from the bundle as static linking is used. For Mono iOS sample app, SOD has decreased by 5%. File |Value (bytes) --------------------------------------------------------------------|------------------- HelloiOS | 17042528 Info.plist | 980 PkgInfo | 8 Program.aotdata | 4792 Program.deps.json | 11754 Program.dll | 6656 Program.runtimeconfig.json | 1064 System.Console.aotdata | 41952 System.Console.dll | 17408 System.Private.CoreLib.aotdata | 826296 System.Private.CoreLib.dll | 998912 icudt.dat | 1824544 icudt_CJK.dat | 956416 icudt_EFIGS.dat | 550320 icudt_no_CJK.dat | 1071600 (**EXCLUDED**) libSystem.IO.Compression.Native.dylib | 877152 (**EXCLUDED**) libSystem.Native.dylib| 145728 (**EXCLUDED**) libSystem.Net.Security.Native.dylib | 72064 (**EXCLUDED**) libSystem.Security.Cryptography.Native.Apple.dylib | 104384 Screenshot 2022-12-13 at 11 50 40 cc @ivanpovazan ### Configuration OS: iOS Architecture: arm64 .NET version: 7.0.100-rc.1.22431.12
Author: kotlarmilos
Assignees: vargaz, kotlarmilos
Labels: `tenet-performance`, `area-Codegen-AOT-mono`, `size-reduction`
Milestone: 8.0.0