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] Exclude unused bundle files in AppleAppBuilder #86316

Merged

Conversation

kotlarmilos
Copy link
Member

This PR aims to reduce the bundle size of the HelloiOS app by excluding unused globalization files and the dedup assembly. These files are currently included in the performance measurements but are not actually used by the app.

Here is an example with the unused bundle files included.

SOD - iOS HelloWorld .app Size

Metric Average
SOD - iOS HelloWorld .app Size 29462794.000 bytes
pub/HelloiOS.app/Program.dll 6144.000 bytes
pub/HelloiOS.app/HelloiOS 20916632.000 bytes
pub/HelloiOS.app/icudt_EFIGS.dat 550832.000 bytes
pub/HelloiOS.app/System.Console.dll 17408.000 bytes
pub/HelloiOS.app/icudt_CJK.dat 956416.000 bytes
pub/HelloiOS.app/System.Private.CoreLib.aotdata 1825744.000 bytes
pub/HelloiOS.app/Program.runtimeconfig.json 1064.000 bytes
pub/HelloiOS.app/System.Console.aotdata 5800.000 bytes
pub/HelloiOS.app/System.Private.CoreLib.dll 1155072.000 bytes
pub/HelloiOS.app/aot-instances.aotdata 1033080.000 bytes
pub/HelloiOS.app/icudt_no_CJK.dat 1107168.000 bytes
pub/HelloiOS.app/Program.aotdata 10944.000 bytes
pub/HelloiOS.app/icudt.dat 1859648.000 bytes
pub/HelloiOS.app/Program.deps.json 12781.000 bytes
pub/HelloiOS.app/aot-instances.dll 3072.000 bytes
pub/HelloiOS.app/Info.plist 981.000 bytes
pub/HelloiOS.app/aot-instances.cs 0.000 bytes
pub/HelloiOS.app/PkgInfo 8.000 bytes

@SamMonoRT
Copy link
Member

@kotlarmilos - how do you determine they are un-used?

@kotlarmilos
Copy link
Member Author

@kotlarmilos - how do you determine they are un-used?

The runtime loads the AOT module of a container assembly using the assembly name (#83711), so the actual assembly is not required. As for the globalization files, the HelloiOS sample app should use icudt.dat, making the others unnecessary.

I have checked the HelloiOS sample app locally, but can't confirm that other globalization files are never used. @akoeplinger is it safe to remove cudt_*.dat from the bundle? Such a change could mainly affect tests/internal bundles.

@filipnavara
Copy link
Member

is it safe to remove cudt_*.dat from the bundle?

Yes, it is.

@kotlarmilos
Copy link
Member Author

/cc: @akoeplinger @ivanpovazan

@@ -109,6 +109,7 @@
EnableAppSandbox="$(EnableAppSandbox)"
DiagnosticPorts="$(DiagnosticPorts)"
StripSymbolTable="$(StripDebugSymbols)"
ExcludeFromAppDir="$(_iOSLikeDedupAssembly)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This is really a nit but it would be cleaner to introduce a new local Item say:

  • _ExcludeFromAppDir Include="$(_iOSLikeDedupAssembly)" around lines 60-65 above

and then have the full list passed to the AppleAppBuilderTask:

  • ExcludeFromAppDir="@(_ExcludeFromAppDir)"

This way it would be simpler to expand the list of excluded items - if we figure out in the future something else should be excluded.

Otherwise, LGTM!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I think we could do the same thing to more properties and simplify the appbuilder.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

I think we could do the same thing to more properties and simplify the appbuilder.

I suggest we remove redundant properties and simplify the appbuilder in #86578.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

I think we could do the same thing to more properties and simplify the appbuilder.

I suggest we remove redundant properties and simplify the appbuilder in #86578.

Agreed!

@kotlarmilos kotlarmilos merged commit b09e864 into dotnet:main May 26, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants