[Xamarin.Android.Build.Tasks] remove the android\assets\shrunk directory #3925
+11
−36
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonathanpeppers
force-pushed
the
shrunkframeworkassemblies
branch
2 times, most recently
from
c3aa501
to
20f192f
Compare
Context: dotnet#3753 Context: xamarin/monodroid#1053 There appears to be an issue with mono:2019-10 when the Xamarin.Forms integration project is built with AOT in `Release` mode. The issue somehow stems from two `Mono.Android.dll` existing: * `obj\Debug\android\assets\Mono.Android.dll` * `obj\Debug\android\assets\shrunk\Mono.Android.dll` Looking at how AOT is invoked, it seems like either `Mono.Android.dll` could be picked up by the parameters being passed in. We might be AOT'ing a user's `Foo.dll`, and both directories are in `$MONO_PATH` or passed via other arguments. How would we know which is picked up? The `shrunk` directory is used by the `<RemoveRegisterAttribute/>` MSBuild task that: * Runs after the linker & `<GenerateJavaStubs/>` * Removes all the `[RegisterAttribute]` from types, to further shrink `Mono.Android.dll`. It looks like we copy all `@(_ResolvedFrameworkAssemblies)` to the `shrunk` directory and fix up item groups. Why don't we just get rid of this `shrunk` directory and not copy anything? We can edit `Mono.Android.dll` in place, and incremental builds should be fine due to the use of stamp files. I also moved the flag file to: $(_AndroidStampDirectory)_RemoveRegisterAttribute.stamp To match our new MSBuild conventions. There is also usage of `@(_ShrunkFrameworkAssemblies)` that needs to be updated in monodroid.
jonathanpeppers
force-pushed
the
shrunkframeworkassemblies
branch
from
20f192f
to
3e18d5a
Compare
|
There is a problem with this change:
Looking at the sequence of events, |
jonathanpeppers
added a commit
to jonathanpeppers/xamarin-android
that referenced
this pull request
Nov 22, 2019
…runk In the case where `$(AndroidLinkMode)` is not `None`, we run a `<RemoveRegisterAttribute/>` to remove all `[Register]` attributes from `Mono.Android.dll`. This is a step to save on APK size. However, we found that the AOT compiler is seeing *two* `Mono.Android.dll` files: * obj\Debug\android\assets\Mono.Android.dll * obj\Debug\android\assets\shrunk\Mono.Android.dll @vargaz pointed out that this is bad, some of the AOT images will be linked against the wrong one and fail to load at runtime. We could possibly be silently falling back to the JIT... I first attempted to just remove the `shrunk` directory and write `Mono.Android.dll` *in place*. This idea didn't pan out: dotnet#3925 (comment) That idea broke incremental builds... So the next idea is to just copy *every assembly* into the `android\assets\shrunk` directory. Then the `AOT` compiler only operates against the `shrunk` directory. This should only happen during `Release` builds, so the build performance hit should be OK. We can also investigate removing `[Register]` from *all* assemblies in a future PR, as that will likely save further APK size from the support libraries, Google Play Services, etc.
|
Closing in favor of #3950 |
jonpryor
pushed a commit
that referenced
this pull request
Nov 25, 2019
…3950) In the case where `$(AndroidLinkMode)` is not `None`, we run the `<RemoveRegisterAttribute/>` task to remove all use of `[Register]` custom attributes from `Mono.Android.dll`. This is a step to save on APK size. However, we found that the AOT compiler is seeing *two* `Mono.Android.dll` files: * `obj\Debug\android\assets\Mono.Android.dll` * `obj\Debug\android\assets\shrunk\Mono.Android.dll` @vargaz pointed out that this is bad, as some of the AOT images will be linked against the wrong one and fail to load at runtime. We could possibly be silently falling back to the JIT... I first attempted to just remove the `shrunk` directory and write `Mono.Android.dll` *in place*. [This idea][0] didn't pan out, as it broke incremental builds: Skipping target "_LinkAssembliesShrink" because all output files are up-to-date with respect to the input files. ... Building target "_GenerateJavaStubs" completely. Input file "obj\Release\android\assets\UnnamedProject.dll" is newer than output file "obj\Release\stamp\_GenerateJavaStubs.stamp". ... Failed to read 'C:\src\xamarin-android\bin\TestDebug\temp\BuildAotApplication_arm64-v8a_Hybrid_True_True\obj\Release\android\assets\Mono.Android.dll' with debugging symbols. Retrying to load it without it. Error details are logged below. Mono.Cecil.Cil.SymbolsNotMatchingException: Symbols were found but are not matching the assembly at Mono.Cecil.ModuleDefinition.ReadSymbols(ISymbolReader reader, Boolean throwIfSymbolsAreNotMaching) in /Users/builder/jenkins/workspace/archive-mono/2019-08/android/debug/external/cecil/Mono.Cecil/ModuleDefinition.cs:line 1069 at Mono.Cecil.ModuleReader.ReadSymbols(ModuleDefinition module, ReaderParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2019-08/android/debug/external/cecil/Mono.Cecil/AssemblyReader.cs:line 111 at Mono.Cecil.ModuleReader.CreateModule(Image image, ReaderParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2019-08/android/debug/external/cecil/Mono.Cecil/AssemblyReader.cs:line 82 at Mono.Cecil.ModuleDefinition.ReadModule(String fileName, ReaderParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2019-08/android/debug/external/cecil/Mono.Cecil/ModuleDefinition.cs:line 1102 at Mono.Cecil.AssemblyDefinition.ReadAssembly(String fileName, ReaderParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2019-08/android/debug/external/cecil/Mono.Cecil/AssemblyDefinition.cs:line 131 at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.ReadAssembly(String file) in C:\src\xamarin-android\external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 163 Instead, just copy *every assembly* into the `android\assets\shrunk` directory, and have the `AOT` compiler only operates against the `shrunk` directory. This should only happen during `Release` builds, so the build performance hit should be OK. We can also investigate removing `[Register]` from *all* assemblies in a future PR, as that will likely save further APK size from the support libraries, Google Play Services, etc. [0]: #3925 (comment)
jonpryor
pushed a commit
that referenced
this pull request
Nov 27, 2019
…3950) In the case where `$(AndroidLinkMode)` is not `None`, we run the `<RemoveRegisterAttribute/>` task to remove all use of `[Register]` custom attributes from `Mono.Android.dll`. This is a step to save on APK size. However, we found that the AOT compiler is seeing *two* `Mono.Android.dll` files: * `obj\Debug\android\assets\Mono.Android.dll` * `obj\Debug\android\assets\shrunk\Mono.Android.dll` @vargaz pointed out that this is bad, as some of the AOT images will be linked against the wrong one and fail to load at runtime. We could possibly be silently falling back to the JIT... I first attempted to just remove the `shrunk` directory and write `Mono.Android.dll` *in place*. [This idea][0] didn't pan out, as it broke incremental builds: Skipping target "_LinkAssembliesShrink" because all output files are up-to-date with respect to the input files. ... Building target "_GenerateJavaStubs" completely. Input file "obj\Release\android\assets\UnnamedProject.dll" is newer than output file "obj\Release\stamp\_GenerateJavaStubs.stamp". ... Failed to read 'C:\src\xamarin-android\bin\TestDebug\temp\BuildAotApplication_arm64-v8a_Hybrid_True_True\obj\Release\android\assets\Mono.Android.dll' with debugging symbols. Retrying to load it without it. Error details are logged below. Mono.Cecil.Cil.SymbolsNotMatchingException: Symbols were found but are not matching the assembly at Mono.Cecil.ModuleDefinition.ReadSymbols(ISymbolReader reader, Boolean throwIfSymbolsAreNotMaching) in /Users/builder/jenkins/workspace/archive-mono/2019-08/android/debug/external/cecil/Mono.Cecil/ModuleDefinition.cs:line 1069 at Mono.Cecil.ModuleReader.ReadSymbols(ModuleDefinition module, ReaderParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2019-08/android/debug/external/cecil/Mono.Cecil/AssemblyReader.cs:line 111 at Mono.Cecil.ModuleReader.CreateModule(Image image, ReaderParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2019-08/android/debug/external/cecil/Mono.Cecil/AssemblyReader.cs:line 82 at Mono.Cecil.ModuleDefinition.ReadModule(String fileName, ReaderParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2019-08/android/debug/external/cecil/Mono.Cecil/ModuleDefinition.cs:line 1102 at Mono.Cecil.AssemblyDefinition.ReadAssembly(String fileName, ReaderParameters parameters) in /Users/builder/jenkins/workspace/archive-mono/2019-08/android/debug/external/cecil/Mono.Cecil/AssemblyDefinition.cs:line 131 at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.ReadAssembly(String file) in C:\src\xamarin-android\external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 163 Instead, just copy *every assembly* into the `android\assets\shrunk` directory, and have the `AOT` compiler only operates against the `shrunk` directory. This should only happen during `Release` builds, so the build performance hit should be OK. We can also investigate removing `[Register]` from *all* assemblies in a future PR, as that will likely save further APK size from the support libraries, Google Play Services, etc. [0]: #3925 (comment)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: #3753
Context: https://github.com/xamarin/monodroid/pull/1053
There appears to be an issue with mono:2019-10 when the Xamarin.Forms
integration project is built with AOT in
Releasemode.The issue somehow stems from two
Mono.Android.dllexisting:obj\Debug\android\assets\Mono.Android.dllobj\Debug\android\assets\shrunk\Mono.Android.dllLooking at how AOT is invoked, it seems like either
Mono.Android.dllcould be picked up by the parameters being passed in. We might be
AOT'ing a user's
Foo.dll, and both directories are in$MONO_PATHor passed via other arguments. How would we know which is picked up?
The
shrunkdirectory is used by the<RemoveRegisterAttribute/>MSBuild task that:
<GenerateJavaStubs/>[RegisterAttribute]from types, to further shrinkMono.Android.dll.It looks like we copy all
@(_ResolvedFrameworkAssemblies)to theshrunkdirectory and fix up item groups. Why don't we just get ridof this
shrunkdirectory and not copy anything? We can editMono.Android.dllin place, and incremental builds should be fine dueto the use of stamp files.
I also moved the flag file to:
To match our new MSBuild conventions.
There is also usage of
@(_ShrunkFrameworkAssemblies)that needs tobe updated in monodroid.