Skip to content

Conversation

@jonathanpeppers
Copy link
Member

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:

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

…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.
@jonpryor jonpryor merged commit 260396f into dotnet:master Nov 25, 2019
@jonathanpeppers jonathanpeppers deleted the shrunkassemblies branch November 25, 2019 19:57
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)
@github-actions github-actions bot locked and limited conversation to collaborators Jan 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants