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.

Sign up for GitHub

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

Jump to bottom

Enable deduplication of generics in Xamarin.iOS build #17766

Merged

rolfbjarne merged 12 commits into xamarin:net8.0 from kotlarmilos:improvement/ios-dedup

Mar 30, 2023

Contributor

kotlarmilos commented Mar 10, 2023 •

edited by rolfbjarne

This PR contributes to dotnet/runtime#83193. It creates a new target _CreateAOTDedupAssembly, and makes the _AOTCompile depend on it. Also, it changes the AOTCompile task to pass dedup-skip and dedup-include to the Mono AOT compiler.

The change was tested on MySingleView app and Monotouch tests. Both apps are working, but some monotouch tests are failing due to Arg_NotlmplementedException. Assumption is that calls between Obj-C and C# could be problematic, as with the dedup improvement enabled, extra methods got moved from origin assemblies to the dedup assembly, so native to managed mapping could be corrupted.

Here are preliminary results comparing size on disk and build time between the baseline (net8.0 branch) and the target (this branch). Binlog details are attached.

App	Baseline size on disk .ipa (MB)	Target size on disk .ipa (MB)	Baseline size on disk .app (MB)	Target size on disk .app (MB)	Baseline build time (s)	Target build time (s)	.app diff (%)
MySingleView Release iOS	5,40	5,40	29,2	15,20	29,18	16,77	47,94
MySingleView Release iOSSimulator-arm64	N/A	N/A	469,5	341,80	468,00	330,00	27,20
Monotouch Release llvm iOS	49,00	38,80	209,60	157,40	115,00	130,00	24,90

Draft PR should get a full test run on the changes. The following tasks should be resolved before making this PR ready for review.

Tasks

Fix failing Monotouch tests and confirm that other tests are passing
Arguments of aot-instances.dll are calculated in inlined msbuild task, which is not the optimal solution. They might be calculated in ComputeAOTArguments, but the task is used during repository build.

/cc: @rolfbjarne @akoeplinger @ivanpovazan


          Enable deduplication of generics in Xamarin.iOS build

bd340ca

Member

dalexsoto commented Mar 10, 2023

/azp run

azure-pipelines bot commented Mar 10, 2023

Azure Pipelines successfully started running 1 pipeline(s).


          Auto-format source code

39f2d8a

Contributor

github-actions bot commented Mar 10, 2023

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

Member

dalexsoto commented Mar 10, 2023

/azp run

azure-pipelines bot commented Mar 10, 2023

Azure Pipelines successfully started running 1 pipeline(s).

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

ivanpovazan reviewed

View reviewed changes

dotnet/targets/Xamarin.Shared.Sdk.targets Outdated Show resolved Hide resolved

ivanpovazan reviewed

View reviewed changes

dotnet/targets/Xamarin.Shared.Sdk.targets Outdated Show resolved Hide resolved

ivanpovazan reviewed

View reviewed changes

msbuild/Xamarin.MacDev.Tasks/Tasks/AOTCompileTaskBase.cs Outdated Show resolved Hide resolved

Contributor

ivanpovazan commented Mar 11, 2023

The estimated savings/improvements look amazing! Well done @kotlarmilos! 🚀
I left some comments/suggestions and would be happy to help.

rolfbjarne requested changes

View reviewed changes

dotnet/targets/Xamarin.Shared.Sdk.targets Outdated

@@ @@ -1022,6 +1048,39 @@ @@
               		</ItemGroup>
               	</Target>
+              	<Target Name="_CreateAOTDedupAssembly"
+              			Condition="'$(_RunAotCompiler)' == 'true'">

Member

rolfbjarne Mar 13, 2023

Adding Inputs and Outputs makes it so that the assembly isn't recreated on every build, only the first build:

Suggested change

      
            			Condition="'$(_RunAotCompiler)' == 'true'">
          
            			Condition="'$(_RunAotCompiler)' == 'true'"
          
            			Inputs="$(MSBuildThisFileFullPath)"
          
            			Outputs="$(_DedupAssembly)"
          
            			>

This also means you'll have to define _DedupAssembly elsewhere (maybe the _ComputeVariables target?)

dotnet/targets/Xamarin.Shared.Sdk.targets Outdated Show resolved Hide resolved

dotnet/targets/Xamarin.Shared.Sdk.targets Outdated

+              		<PropertyGroup>
+              			<_DedupAssembly>$(IntermediateLinkDir)aot-instances.dll</_DedupAssembly>
+              		</PropertyGroup>
+              		<WriteLinesToFile File="$(_IntermediateNativeLibraryDir)/aot-instances.cs" Overwrite="true" Lines="" WriteOnlyWhenDifferent="true" />

Member

rolfbjarne Mar 13, 2023

Suggested change

      
            		<WriteLinesToFile File="$(_IntermediateNativeLibraryDir)/aot-instances.cs" Overwrite="true" Lines="" WriteOnlyWhenDifferent="true" />
          
            		<WriteLinesToFile File="$(_IntermediateNativeLibraryDir)aot-instances.cs" Overwrite="true" Lines="" WriteOnlyWhenDifferent="true" />

dotnet/targets/Xamarin.Shared.Sdk.targets Outdated

+              			<_DedupAssembly>$(IntermediateLinkDir)aot-instances.dll</_DedupAssembly>
+              		</PropertyGroup>
+              		<WriteLinesToFile File="$(_IntermediateNativeLibraryDir)/aot-instances.cs" Overwrite="true" Lines="" WriteOnlyWhenDifferent="true" />
+              		<Csc Sources="$(_IntermediateNativeLibraryDir)\aot-instances.cs"

Member

rolfbjarne Mar 13, 2023

Suggested change

      
            		<Csc Sources="$(_IntermediateNativeLibraryDir)\aot-instances.cs"
          
            		<Csc Sources="$(_IntermediateNativeLibraryDir)aot-instances.cs"

dotnet/targets/Xamarin.Shared.Sdk.targets Outdated

@@ @@ -1022,6 +1048,39 @@ @@
               		</ItemGroup>
               	</Target>
+              	<Target Name="_CreateAOTDedupAssembly"
+              			Condition="'$(_RunAotCompiler)' == 'true'">

Member

rolfbjarne Mar 13, 2023

This is necessary so that we don't try to do this when building on Windows without being connected to a Mac:

Suggested change

      
            			Condition="'$(_RunAotCompiler)' == 'true'">
          
            			Condition="'$(_RunAotCompiler)' == 'true' And '$(IsMacEnabled)' == 'true'">

Member

rolfbjarne commented Mar 13, 2023

Both apps are working, but some monotouch tests are failing due to Arg_NotlmplementedException.

This didn't happen on the bots, so it might have been something local to you.


          Calculate dedup arguments in ComputeAOTArguments step

b80f9ea

Contributor Author

kotlarmilos commented Mar 14, 2023

/azp run

azure-pipelines bot commented Mar 14, 2023

Commenter does not have sufficient privileges for PR 17766 in repo xamarin/xamarin-macios

Member

dalexsoto commented Mar 14, 2023

/azp run

azure-pipelines bot commented Mar 14, 2023

Azure Pipelines successfully started running 1 pipeline(s).

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

Member

rolfbjarne commented Mar 14, 2023

Looks like Mac Catalyst apps crash on startup on ARM64:

=================================================================
	Native Crash Reporting
=================================================================
Got a SIGABRT while executing native code. This usually indicates
a fatal error in the mono runtime or one of the native libraries 
used by your application.
=================================================================

=================================================================
	Native stacktrace:
=================================================================
	0x1129b3628 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mono_dump_native_crash_info
	0x112971a14 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mono_handle_native_crash
	0x112cba27c - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : sigabrt_signal_handler.cold.1
	0x1129b2edc - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mono_runtime_setup_stat_profiler
	0x18a0602a4 - /usr/lib/system/libsystem_platform.dylib : _sigtramp
	0x18a031cec - /usr/lib/system/libsystem_pthread.dylib : pthread_kill
	0x189f6a2c8 - /usr/lib/system/libsystem_c.dylib : abort
	0x112a4cd84 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mono_log_write_os_log
	0x112a3fe94 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : monoeg_g_logv_nofree
	0x112a3ff4c - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : monoeg_assertion_message
	0x112a3ff90 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mono_assertion_message_unreachable
	0x11295d178 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mono_aot_get_method
	0x1128da49c - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : jit_compile_method_with_opt
	0x1128de800 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mono_jit_runtime_invoke
	0x112acacb8 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mono_runtime_invoke_checked
	0x112a835dc - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : create_exception_two_strings
	0x112a833ac - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mono_exception_from_name_two_strings_checked
	0x112a60110 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mono_runtime_init_checked
	0x1128de130 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mini_init
	0x1129366d8 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libmonosgen-2.0.dylib : mono_jit_init_version
	0x11233ad0c - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libxamarin-dotnet-debug.dylib : xamarin_bridge_initialize
	0x1***03a8 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MonoBundle/libxamarin-dotnet-debug.dylib : xamarin_main
	0x1095be984 - /Users/builder/azdo/_work/3/s/artifacts/mac-test-package/tests/monotouch-test/dotnet/MacCatalyst/bin/Debug/net8.0-maccatalyst/maccatalyst-arm64/monotouchtest.app/Contents/MacOS/monotouchtest : main
	0x189d07e50 - /usr/lib/dyld : start

=================================================================
	Basic Fault Address Reporting
=================================================================
Memory around native instruction pointer (0x189ffa868):0x189ffa858  ff 0f 5f d6 c0 03 5f d6 10 29 80 d2 01 10 00 d4  .._..._..)......
0x189ffa868  03 01 00 54 7f 23 03 d5 fd 7b bf a9 fd 03 00 91  ...T.#...{......
0x189ffa878  64 e0 ff 97 bf 03 00 91 fd 7b c1 a8 ff 0f 5f d6  d........{...._.
0x189ffa888  c0 03 5f d6 70 0a 80 d2 01 10 00 d4 03 01 00 54  .._.p..........T

=================================================================
	Managed Stacktrace:
=================================================================
	  at <unknown> <0xffffffff>
=================================================================

Contributor Author

kotlarmilos commented Mar 14, 2023

Thanks! I will try to obtain more verbose stack trace by using the locally built runtime.

The deduplication improvement has been tested on ios, tvos, and arm64 simulators in the runtime.

azure-pipelines bot commented Mar 28, 2023

Azure Pipelines successfully started running 2 pipeline(s).

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

Member

rolfbjarne commented Mar 28, 2023

/azp run

azure-pipelines bot commented Mar 28, 2023

Azure Pipelines successfully started running 1 pipeline(s).

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

Contributor Author

kotlarmilos commented Mar 29, 2023

Failures shouldn't be related. However, there are simulator tests pending. @rolfbjarne should we rerun the CI or pull the latest changes from the base branch?


          Merge remote-tracking branch 'origin/net8.0' into improvement/ios-dedup

Member

rolfbjarne commented Mar 29, 2023

/azp run

azure-pipelines bot commented Mar 29, 2023

Azure Pipelines successfully started running 1 pipeline(s).

Collaborator

vs-mobiletools-engineering-service2 commented Mar 29, 2023

🔥 Failed to compare API and create generator diff 🔥

Failed to update apidiff references

Pipeline on Agent
Hash: 37608221edf5ff0a37a1b99213f5b2647ab52dd8 [PR build]

Collaborator

vs-mobiletools-engineering-service2 commented Mar 29, 2023

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13.0) passed.

Pipeline on Agent
Hash: 37608221edf5ff0a37a1b99213f5b2647ab52dd8 [PR build]

Collaborator

vs-mobiletools-engineering-service2 commented Mar 29, 2023

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: 37608221edf5ff0a37a1b99213f5b2647ab52dd8 [PR build]

Collaborator

vs-mobiletools-engineering-service2 commented Mar 29, 2023

❌ [CI Build] Windows Integration Tests failed ❌

❌ Failed ❌

Pipeline on Agent
Hash: 37608221edf5ff0a37a1b99213f5b2647ab52dd8 [PR build]

Collaborator

vs-mobiletools-engineering-service2 commented Mar 29, 2023

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMBOT-1163.Ventura
Hash: 37608221edf5ff0a37a1b99213f5b2647ab52dd8 [PR build]

This comment has been minimized.

Sign in to view

Collaborator

vs-mobiletools-engineering-service2 commented Mar 29, 2023

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 219 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 6 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 25 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 37608221edf5ff0a37a1b99213f5b2647ab52dd8 [PR build]

emaf approved these changes

View reviewed changes

rolfbjarne approved these changes

View reviewed changes

rolfbjarne merged commit 6d6cee8 into xamarin:net8.0

83 of 86 checks passed

rolfbjarne mentioned this pull request

[ios] Modifying code in library referenced by application results in AOT module loading failure #17708

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment