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

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

Open
kotlarmilos opened this issue Nov 25, 2022 · 15 comments
Open

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

kotlarmilos opened this issue Nov 25, 2022 · 15 comments
Assignees
Labels
area-Codegen-AOT-mono size-reduction Issues impacting final app size primary for size sensitive workloads tenet-performance Performance related issue
Milestone

Comments

@kotlarmilos
Copy link
Member

kotlarmilos commented Nov 25, 2022

Description

Microbenchmark improvement #78182 introduced concrete instances in addition to the gshared methods in AOT mode to optimize #78163.

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

Filename SOD without concrete instances (80208e6) SOD with concrete instances (486682a)
HelloiOS 17382408 bytes 18872480 bytes

Number of symbols has increased by 5k.

Filename Number of symbols without concrete instances (80208e6) Number of symbols with concrete instances (486682a)
HelloiOS 80007 85179

At the same time 80208e6 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

@kotlarmilos kotlarmilos added the tenet-performance Performance related issue label Nov 25, 2022
@dotnet-issue-labeler
Copy link

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.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 25, 2022
@ivanpovazan
Copy link
Member

@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
Copy link
Member Author

Good point, #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
Copy link
Member

/cc @jonathanpeppers

@marek-safar marek-safar added area-Codegen-AOT-mono trimming-for-aot `EnableAggressiveTrimming=true` used for running tests with AOT labels Nov 25, 2022
@marek-safar marek-safar added this to the 8.0.0 milestone Nov 25, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 25, 2022
@marek-safar marek-safar removed the trimming-for-aot `EnableAggressiveTrimming=true` used for running tests with AOT label Nov 25, 2022
@vargaz
Copy link
Contributor

vargaz commented Nov 26, 2022

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
Copy link
Contributor

vargaz commented Nov 27, 2022

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
Copy link
Member Author

Related issues:

#71430
#71431

@ivanpovazan
Copy link
Member

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:

  • if SIMD is enabled -> do not generate extra instances (leave codegen as is - falling back to GSHAREDVTs)
  • if SIMD is disabled -> generate extra instances to improve performance

@vargaz
Copy link
Contributor

vargaz commented Dec 1, 2022

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
Copy link
Member

If I am understanding this correctly, something similar was attempted here: #71430 (comment)
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
Copy link
Contributor

vargaz commented Dec 1, 2022

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

@ivanpovazan
Copy link
Member

I guess this comment: dotnet/perf-autofiling-issues#9599 (comment) 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
Copy link
Member

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
Copy link
Contributor

vargaz commented Dec 12, 2022

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).

@kotlarmilos kotlarmilos changed the title Mono iOS disk size regression [mono][perf] iOS disk size regression on 11 Nov 2022 Dec 13, 2022
@kotlarmilos kotlarmilos added the size-reduction Issues impacting final app size primary for size sensitive workloads label Dec 13, 2022
@ghost
Copy link

ghost commented Dec 13, 2022

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 #78182 introduced concrete instances in addition to the gshared methods in AOT mode to optimize #78163.

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

Filename SOD without concrete instances (80208e6) SOD with concrete instances (486682a)
HelloiOS 17382408 bytes 18872480 bytes

Number of symbols has increased by 5k.

Filename Number of symbols without concrete instances (80208e6) Number of symbols with concrete instances (486682a)
HelloiOS 80007 85179

At the same time 80208e6 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

@kotlarmilos kotlarmilos added this to the 9.0.0 milestone Jul 13, 2023
@vargaz vargaz removed their assignment Mar 22, 2024
@kotlarmilos kotlarmilos modified the milestones: 9.0.0, Future Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Codegen-AOT-mono size-reduction Issues impacting final app size primary for size sensitive workloads tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

5 participants