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

Rework _GenerateJavaDesignerForComponent #2836

Closed
dellis1972 opened this issue Mar 15, 2019 · 15 comments
Closed

Rework _GenerateJavaDesignerForComponent #2836

dellis1972 opened this issue Mar 15, 2019 · 15 comments
Assignees
Labels
enhancement Proposed change to current functionality.
Milestone

Comments

@dellis1972
Copy link
Contributor

The _GenerateJavaDesignerForComponent is tasked with generating the R.java files for all the referenced libraries which have an AndroidManifest.xml. This is so any java code which use resource items such as android.support.transition.R.abc_fade_in will compile.

Currently we call aapt or aapt2 once per library to generate the file. We do this on the first build. But we also do it on subsequent builds if the library changes. For Nuget packages this means once its built it never gets built again unless the user does a Clean.

We do run these tasks in Parallel, which does improve things quite a bit. But it does put the CPU under allot of stress, since we have to launch a aapt* process per library. So if you have 8 cores on your machine you get 7 aapt* processes running.

Is this the most efficient way of doing this? Looking at the output of the R.java file the ONLY code difference between ALL the files is the package declaration at the top.
E.g package android.support.compat; vs package android.support.transition;
So the next question is why don't we generate one R.java file and then do a search & replace for each library reading the required packageName from the AndroidManifest.xml.

This would mean

A) We are not duplicating a whole TON of work
B) the initial build should be much quicker since we are not spawning up a whole ton of processes.

@dellis1972 dellis1972 added the enhancement Proposed change to current functionality. label Mar 15, 2019
@dellis1972 dellis1972 added this to the d16-2 milestone Mar 15, 2019
@jonathanpeppers
Copy link
Member

So if I look at the <Aapt2Link/> call for one of these:

    Executing link --manifest C:\Users\jopepper\Desktop\Git\xamarin-android\tests\Xamarin.Forms-Performance-Integration\Droid\obj\Debug\90\lp\10\jl\res\..\manifest\AndroidManifest.xml --java obj\Debug\90\android\src -R obj\Debug\90\flata\a4b85b611459d0952d6a8dfe9ad015c40a882909.flata -R obj\Debug\90\flata\d8ab1a034f0be43229b4da43251e84f2f7e9b249.flata -R obj\Debug\90\flata\3fff511ef9264bad797ba8942541ecffd454b351.flata -R obj\Debug\90\flata\5f8a75baa4723a993fc610218c21ecf3b47a1c37.flata -R obj\Debug\90\flata\e8c79609a2a8bfa4c540009f80389af291132eea.flata -R obj\Debug\90\flata\796f9008b9fc36582ee0579e7506dea4d7cc0686.flata -R obj\Debug\90\flata\f6ec8bb92c8420670fff18f26c063c37d80eca24.flata -R obj\Debug\90\flata\f2bead6468118783c404222da88d7e9e9ede701f.flata -R obj\Debug\90\flata\\compiled.flata --auto-add-overlay -I C:\Users\jopepper\android-toolchain\sdk\platforms\android-28\android.jar -o C:\Users\jopepper\AppData\Local\Temp\tmp944D.tmp
    Executing link --manifest C:\Users\jopepper\Desktop\Git\xamarin-android\tests\Xamarin.Forms-Performance-Integration\Droid\obj\Debug\90\lp\13\jl\res\..\manifest\AndroidManifest.xml --java obj\Debug\90\android\src -R obj\Debug\90\flata\a4b85b611459d0952d6a8dfe9ad015c40a882909.flata -R obj\Debug\90\flata\d8ab1a034f0be43229b4da43251e84f2f7e9b249.flata -R obj\Debug\90\flata\3fff511ef9264bad797ba8942541ecffd454b351.flata -R obj\Debug\90\flata\5f8a75baa4723a993fc610218c21ecf3b47a1c37.flata -R obj\Debug\90\flata\e8c79609a2a8bfa4c540009f80389af291132eea.flata -R obj\Debug\90\flata\796f9008b9fc36582ee0579e7506dea4d7cc0686.flata -R obj\Debug\90\flata\f6ec8bb92c8420670fff18f26c063c37d80eca24.flata -R obj\Debug\90\flata\f2bead6468118783c404222da88d7e9e9ede701f.flata -R obj\Debug\90\flata\\compiled.flata --auto-add-overlay -I C:\Users\jopepper\android-toolchain\sdk\platforms\android-28\android.jar -o C:\Users\jopepper\AppData\Local\Temp\tmp945F.tmp
    Executing link --manifest C:\Users\jopepper\Desktop\Git\xamarin-android\tests\Xamarin.Forms-Performance-Integration\Droid\obj\Debug\90\lp\22\jl\res\..\manifest\AndroidManifest.xml --java obj\Debug\90\android\src -R obj\Debug\90\flata\a4b85b611459d0952d6a8dfe9ad015c40a882909.flata -R obj\Debug\90\flata\d8ab1a034f0be43229b4da43251e84f2f7e9b249.flata -R obj\Debug\90\flata\3fff511ef9264bad797ba8942541ecffd454b351.flata -R obj\Debug\90\flata\5f8a75baa4723a993fc610218c21ecf3b47a1c37.flata -R obj\Debug\90\flata\e8c79609a2a8bfa4c540009f80389af291132eea.flata -R obj\Debug\90\flata\796f9008b9fc36582ee0579e7506dea4d7cc0686.flata -R obj\Debug\90\flata\f6ec8bb92c8420670fff18f26c063c37d80eca24.flata -R obj\Debug\90\flata\f2bead6468118783c404222da88d7e9e9ede701f.flata -R obj\Debug\90\flata\\compiled.flata --auto-add-overlay -I C:\Users\jopepper\android-toolchain\sdk\platforms\android-28\android.jar -o C:\Users\jopepper\AppData\Local\Temp\tmp9473.tmp
    Executing link --manifest C:\Users\jopepper\Desktop\Git\xamarin-android\tests\Xamarin.Forms-Performance-Integration\Droid\obj\Debug\90\lp\15\jl\res\..\manifest\AndroidManifest.xml --java obj\Debug\90\android\src -R obj\Debug\90\flata\a4b85b611459d0952d6a8dfe9ad015c40a882909.flata -R obj\Debug\90\flata\d8ab1a034f0be43229b4da43251e84f2f7e9b249.flata -R obj\Debug\90\flata\3fff511ef9264bad797ba8942541ecffd454b351.flata -R obj\Debug\90\flata\5f8a75baa4723a993fc610218c21ecf3b47a1c37.flata -R obj\Debug\90\flata\e8c79609a2a8bfa4c540009f80389af291132eea.flata -R obj\Debug\90\flata\796f9008b9fc36582ee0579e7506dea4d7cc0686.flata -R obj\Debug\90\flata\f6ec8bb92c8420670fff18f26c063c37d80eca24.flata -R obj\Debug\90\flata\f2bead6468118783c404222da88d7e9e9ede701f.flata -R obj\Debug\90\flata\\compiled.flata --auto-add-overlay -I C:\Users\jopepper\android-toolchain\sdk\platforms\android-28\android.jar -o C:\Users\jopepper\AppData\Local\Temp\tmp944E.tmp
    Executing link --manifest C:\Users\jopepper\Desktop\Git\xamarin-android\tests\Xamarin.Forms-Performance-Integration\Droid\obj\Debug\90\lp\18\jl\res\..\manifest\AndroidManifest.xml --java obj\Debug\90\android\src -R obj\Debug\90\flata\a4b85b611459d0952d6a8dfe9ad015c40a882909.flata -R obj\Debug\90\flata\d8ab1a034f0be43229b4da43251e84f2f7e9b249.flata -R obj\Debug\90\flata\3fff511ef9264bad797ba8942541ecffd454b351.flata -R obj\Debug\90\flata\5f8a75baa4723a993fc610218c21ecf3b47a1c37.flata -R obj\Debug\90\flata\e8c79609a2a8bfa4c540009f80389af291132eea.flata -R obj\Debug\90\flata\796f9008b9fc36582ee0579e7506dea4d7cc0686.flata -R obj\Debug\90\flata\f6ec8bb92c8420670fff18f26c063c37d80eca24.flata -R obj\Debug\90\flata\f2bead6468118783c404222da88d7e9e9ede701f.flata -R obj\Debug\90\flata\\compiled.flata --auto-add-overlay -I C:\Users\jopepper\android-toolchain\sdk\platforms\android-28\android.jar -o C:\Users\jopepper\AppData\Local\Temp\tmp9462.tmp
    Executing link --manifest C:\Users\jopepper\Desktop\Git\xamarin-android\tests\Xamarin.Forms-Performance-Integration\Droid\obj\Debug\90\lp\20\jl\res\..\manifest\AndroidManifest.xml --java obj\Debug\90\android\src -R obj\Debug\90\flata\a4b85b611459d0952d6a8dfe9ad015c40a882909.flata -R obj\Debug\90\flata\d8ab1a034f0be43229b4da43251e84f2f7e9b249.flata -R obj\Debug\90\flata\3fff511ef9264bad797ba8942541ecffd454b351.flata -R obj\Debug\90\flata\5f8a75baa4723a993fc610218c21ecf3b47a1c37.flata -R obj\Debug\90\flata\e8c79609a2a8bfa4c540009f80389af291132eea.flata -R obj\Debug\90\flata\796f9008b9fc36582ee0579e7506dea4d7cc0686.flata -R obj\Debug\90\flata\f6ec8bb92c8420670fff18f26c063c37d80eca24.flata -R obj\Debug\90\flata\f2bead6468118783c404222da88d7e9e9ede701f.flata -R obj\Debug\90\flata\\compiled.flata --auto-add-overlay -I C:\Users\jopepper\android-toolchain\sdk\platforms\android-28\android.jar -o C:\Users\jopepper\AppData\Local\Temp\tmp945E.tmp
    Executing link --manifest C:\Users\jopepper\Desktop\Git\xamarin-android\tests\Xamarin.Forms-Performance-Integration\Droid\obj\Debug\90\lp\19\jl\res\..\manifest\AndroidManifest.xml --java obj\Debug\90\android\src -R obj\Debug\90\flata\a4b85b611459d0952d6a8dfe9ad015c40a882909.flata -R obj\Debug\90\flata\d8ab1a034f0be43229b4da43251e84f2f7e9b249.flata -R obj\Debug\90\flata\3fff511ef9264bad797ba8942541ecffd454b351.flata -R obj\Debug\90\flata\5f8a75baa4723a993fc610218c21ecf3b47a1c37.flata -R obj\Debug\90\flata\e8c79609a2a8bfa4c540009f80389af291132eea.flata -R obj\Debug\90\flata\796f9008b9fc36582ee0579e7506dea4d7cc0686.flata -R obj\Debug\90\flata\f6ec8bb92c8420670fff18f26c063c37d80eca24.flata -R obj\Debug\90\flata\f2bead6468118783c404222da88d7e9e9ede701f.flata -R obj\Debug\90\flata\\compiled.flata --auto-add-overlay -I C:\Users\jopepper\android-toolchain\sdk\platforms\android-28\android.jar -o C:\Users\jopepper\AppData\Local\Temp\tmp9461.tmp
    Executing link --manifest C:\Users\jopepper\Desktop\Git\xamarin-android\tests\Xamarin.Forms-Performance-Integration\Droid\obj\Debug\90\lp\16\jl\res\..\manifest\AndroidManifest.xml --java obj\Debug\90\android\src -R obj\Debug\90\flata\a4b85b611459d0952d6a8dfe9ad015c40a882909.flata -R obj\Debug\90\flata\d8ab1a034f0be43229b4da43251e84f2f7e9b249.flata -R obj\Debug\90\flata\3fff511ef9264bad797ba8942541ecffd454b351.flata -R obj\Debug\90\flata\5f8a75baa4723a993fc610218c21ecf3b47a1c37.flata -R obj\Debug\90\flata\e8c79609a2a8bfa4c540009f80389af291132eea.flata -R obj\Debug\90\flata\796f9008b9fc36582ee0579e7506dea4d7cc0686.flata -R obj\Debug\90\flata\f6ec8bb92c8420670fff18f26c063c37d80eca24.flata -R obj\Debug\90\flata\f2bead6468118783c404222da88d7e9e9ede701f.flata -R obj\Debug\90\flata\\compiled.flata --auto-add-overlay -I C:\Users\jopepper\android-toolchain\sdk\platforms\android-28\android.jar -o C:\Users\jopepper\AppData\Local\Temp\tmp9460.tmp

It looks like this switch is putting R.java files per Java package to be built and placed in the app:

--java obj\Debug\90\android\src

If we built a single R.java, how would we know how to split up the massive file into multiple files? I would think the resource ids would be all mixed up and you wouldn't know which one came from which package?

I think there can be Java code that depends on R.id.foo working via a specific package name.

@dellis1972
Copy link
Contributor Author

We use the list of AndroidManifest.xml files to drive where the R.java files need to be copied too.
Note that ALL the R.java files are the same they contain ALL the resources the only difference is the package value and a the comments. The values of the id's are irrelevant since the final app R.java replaces them all.

@jonathanpeppers
Copy link
Member

Ok, I see:

  • So we should be creating an R.java that only contains resources for a given jar file (and any other jars it references)
  • We are actually creating R.java files that contain all resources for all jar files...

Since we are already doing things this way, we can reduce this to a single aapt call.

The only other diffs I see are in the javadoc comments (don't matter), but I looked at aapt2 output. I will check aapt to be sure it's the same.

@jonathanpeppers
Copy link
Member

jonathanpeppers commented Mar 25, 2019

Ok, I think our current behavior actually might be a problem:

It has been observed that Xamarin.Android apps "seem" to hit the dex limit sooner than Java apps.

I have not had time to actually investigate and see if this is the case--and see by how much.

For a "Hello World" Xamarin.Forms app, if I look at R.java, it contains 2,267 fields. There are 9 R.java's: 8 for libraries, and 1 for the main app.

Does this mean we have potentially some amount of extra fields? Could be some percentage of 18,136 fields (8 x 2,267)? I see the same number of fields with aapt vs aapt2.

@jonathanpeppers
Copy link
Member

Ok so if I just look at a random class: Landroid/support/compat/R$drawable;

Here is dexdump output of each: dexdump.zip

@dellis1972
Copy link
Contributor Author

The problem we have is there are java classes in the support libs which reference R.xx values for other libraries. For example AppCompat Theme stuff is referenced and used by other libraries and aar files. During compilation if javac cannot find these we get a build error. These are the build errors we see when Nuget's have not been restored properly.

So I believe that the whole point of _GenerateJavaDesignerForComponent was to generate those R.java files for libraries. Now I would love to cut down on what we generate but we don't really know what is being referenced until we compile the code. It does feel like this target (_GenerateJavaDesignerForComponent) is a bit of a hammer for fixing a particular problem, and probably not in the most efficient way.

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Mar 26, 2019
Context: dotnet#2836 (comment)

We have noticed apps using Xamarin.Forms.Maps might be close to
hitting the dex limit for fields.

An example from @PureWeen:

    1>  trouble writing output: Too many field references to fit in one dex file: 70468; max is 65536.

We think the way Xamarin.Android invokes `aapt` (or `aapt2`) might be
creating `R.java` files with more fields than are needed.

Step 1 is to add a test project that uses Xamarin.Forms.Maps: we did
not have one.
jonpryor pushed a commit that referenced this issue Mar 27, 2019
Context: #2836 (comment)

We have noticed apps using Xamarin.Forms.Maps might be close to
hitting the dex limit for fields.

An example from @PureWeen:

	1>  trouble writing output: Too many field references to fit in one dex file: 70468; max is 65536.

We think the way Xamarin.Android invokes `aapt` (or `aapt2`) might be
creating `R.java` files with more fields than are needed.

Step 1 is to add a test project that uses Xamarin.Forms.Maps: we did
not have one.
@jonathanpeppers
Copy link
Member

Ok, so I think I understand now what Java/Android Studio does.

At the top level, the Android gradle plugin has a processResources task.

It runs aapt once on a single directory to produce an R.txt. All their resources must be dumped into a single directory.

Then the R.txt file is parsed via SymbolLoader.java and only fields that are needed are included for each library: https://android.googlesource.com/platform/tools/build/+/refs/heads/master/builder/src/main/java/com/android/builder/AndroidBuilder.java#726

This Java code writes out the individual R.java files via SymbolWriter.java.

So we would have to port this code to C#? Or consider integrating with gradle?

@dellis1972
Copy link
Contributor Author

lets not integrate with gradle if we can help it. We already produce the R.txt with our calls to aapt and aapt2. So we could just add this as a step before CompileJava and replace the _GenerateJavaDesignerForComponent

@dellis1972
Copy link
Contributor Author

We have a basic R.txt parser in the ManagedResourceParser https://github.com/xamarin/xamarin-android/blob/master/src/Xamarin.Android.Build.Tasks/Utilities/ManagedResourceParser.cs#L159.

my App R.txt for a Standard Xamarin.Forms app has 2900 entires in it.

Looking at the code you posted it seems the AndroidBuilder.java uses both the main app R.txt AND the R.txt which is included in the library.
https://android.googlesource.com/platform/tools/build/+/refs/heads/master/builder/src/main/java/com/android/builder/AndroidBuilder.java#745

Luckily we do include that in our extraction process so we have obj/Debug/lp/32/R.txt for example. These files are much smaller (or don't exist).

So I guess they do some kind of merge process. between that main one and the one the library has.

@erikpowa
Copy link
Contributor

This is so any java code which use resource items such as android.support.transition.R.abc_fade_in will compile.

For the record, the other side acting the same way with Resource.designer.cs. If you have a XA Library referencing foreg the appcompat library and uses 1 resource from it, there will be Resource.designer.cs with junk static fields and field setters for the UpdateValues. Problem/bad part is the Linker can't just catch them all (the static array initialization are the annoying ones).

Since I have OCD against junk IL code, I try to move resource access out from XA Libraries, to the root App via XML + IDs (this way the Resource.designer.cs will not have the the appcompat resource ids, only the ones I specify) in XA libraries and to ensure things, I do pre-link before LinkAssemblies, looking for unused
resource access aka for (resource)fields that doesn't have ldsfld anywhere, then clean up any stsfld (references/patterns like ldc_i4 stsfld or ldc_i4_1 newarr dup ldtoken call stsfld) and then just removing the fields themselves.)

@jonathanpeppers
Copy link
Member

@erikpowa if you enable the linker (link all assemblies) are these removed? You can also add [assembly:LinkerSafe] if you want the SDK only option to strip them.

You can also prevent Resources.designer.cs from being created at all in a library project if you just omit <AndroidResgenFile>Resources\Resource.Designer.cs</AndroidResgenFile>. Xamarin.Forms does this (or excludes the file from <Compile/>) for several of their library projects that reference the support libraries.

@erikpowa
Copy link
Contributor

erikpowa commented Mar 28, 2019

@jonathanpeppers

if you enable the linker (link all assemblies) are these removed?

Not really... especially from the RootAssembly (as far as I know the root assembly being skipped by default and that holds basically every resource id). About the libraries, linker strips only the classes like Dimension if the class itself is unused. I'm pretty sure stripping the resources will require a custom Step (if possible) for the Linker, because static constructor initializes static fields and if that class being called / used in anyway, that .cctor() will stay 100% with it's field setter opcodes, therefore the fields are staying aswell because the linker think they are used (tested it long ago). That's why I wrote a custom link task with few others.

[assembly:LinkerSafe]

I never heard of it actually. Forgot to mention that my context is with AndroidLinkMode=Full.

if you just omit

Well if I omit it, how would that compile if I have resource that needs to be accessed? or how the resource id would point to the correct resource ? since the RootAssembly should contain that static UpdateIdValues() within that the updates for those static fields to the correct resource ids.

Btw the UpdateIdValues() in the RootAssembly will contain ldsfld stsfld when static field updates static field... remember I said that if the resource class is being used, then the whole class stay with all it's field... + the RootAssembly not stripped = lot's of unused IL code everywhere. (Edit: You can't just strip the UpdateIdValues(), that is required, even if you strip the RootAssembly)

@jonathanpeppers
Copy link
Member

jonathanpeppers commented Mar 28, 2019

@erikpowa so I think we should revisit the managed side after fixing the java side.

Either a new linker step that strips unused fields--or maybe we don't need to generate as much C# code in the first place? Although, for Intellisense to work, it would need fields that aren't in use yet...

If you have a linker step fixing this up, is it compatible with our stuff? would you be willing to open a PR?

@erikpowa
Copy link
Contributor

erikpowa commented Mar 29, 2019

@jonathanpeppers I haven't dived myself into the designtime resource-generator, but the only problem is the "strong references" to the fields. As I see as for solution:
Currently, library resource fields are required to be static, because UpdateIdValues() will change them when app starts (or atleast when the Resource class first accessed, which is also bad because of it uses reflection when "we know the metadata at compile time"). Foreg:
global::CommonUI.Resource.Dimension.size200 = global::MainAssembly.Resource.Dimension.size200;
this is one line from UpdateIdValues() in MainAssembly
where global::CommonUI.Resource.Dimension.size200 is static field from a library
where global::MainAssembly.Resource.Dimension.size200 const field from the RootAssembly (having the final resourceId)

If the res designer already knows what resource will be set for which static field, then why isn't this knowledge/process moved out?
Let's say global::MainAssembly.Resource.Dimension.size200 has value 2131099880.
"LinkAssemblies" uses mono cecil anyway, it would be easier just modify the field's value in the libraries when the app compiles (we know the field, we know what value it's gonna get) Foreg: In CommonUI library, the global::CommonUI.Resource.Dimension.size200 will be set to 2131099880 in the UpdateIdValues(), but if we could make the global::CommonUI.Resource.Dimension.size200 a const field and modify the IL code to have a value with 2131099880 then all the problem will disappear... no longer strong reference to fields, no longer required to call UpdateIdValues(), no static constructor with trigger of UpdateIdValues, no reflection used and the linker will remove any unused const field(?).

is it compatible with our stuff

For resource cleaning, I have just temporary solution, unsafe and not optimized at all.
Here. (Edit: ohh btw it's exponentially slows down if more xa library references support library)
I would do PR once I have time to do things like this, but currently I don't know where to hook it, when to hook such flow.
Is it possibe to plug and play Linker Steps without actuall implementing it into Xamarin.Android.Build.Tasks?

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Mar 29, 2019
Context: https://android.googlesource.com/platform/tools/base/+/refs/heads/master/build-system/builder/
Fixes: dotnet#2680
Fixes: dotnet#2836

The current behavior in the `_GenerateJavaDesignerForComponent`
MSBuild target does the following:

* For each library that has Android resources... (in parallel)
* Run an instance of aapt/aapt2 to generate the `R.java` file for each
  library.
* This actually creates an `R.java` file that contains *every*
  resource id for *every* library. These libraries are not using most
  of these ids.

This has a few problems:

* dotnet#2680 notes a problem where a file is locked on Windows during
  `_GenerateJavaDesignerForComponent`.

    Xamarin.Android.Common.targets(1541,2): The process cannot access the file 'C:\repos\msalnet\tests\devapps\XForms\XForms.Android\obj\Debug\90\lp\26\jl\manifest\AndroidManifest.xml' because it is being used by another process.
        at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
        at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
        at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
        at System.IO.StreamWriter.CreateFile(String path, Boolean append, Boolean checkHost)
        at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding, Int32 bufferSize, Boolean checkHost)
        at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding)
        at Xamarin.Android.Tasks.ManifestDocument.Save(String filename)
        at Xamarin.Android.Tasks.Aapt.GenerateCommandLineCommands(String ManifestFile, String currentAbi, String currentResourceOutputFile)
        at Xamarin.Android.Tasks.Aapt.ProcessManifest(ITaskItem manifestFile)
        at System.Threading.Tasks.Parallel.<>c__DisplayClass30_0`2.<ForEachWorker>b__0(Int32 i)
        at System.Threading.Tasks.Parallel.<>c__DisplayClass17_0`1.<ForWorker>b__1()
        at System.Threading.Tasks.Task.InnerInvoke()
        at System.Threading.Tasks.Task.InnerInvokeWithArg(Task childTask)
        at System.Threading.Tasks.Task.<>c__DisplayClass176_0.<ExecuteSelfReplicating>b__0(Object ) [C:\repos\msalnet\tests\devapps\XForms\XForms.Android\XForms.Android.csproj]

* We are hugely contributing to the dex limit for fields. Apps contain
  exponentially more fields for each library with resources.

An example from @PureWeen:

    1>  trouble writing output: Too many field references to fit in one dex file: 70468; max is 65536.

* Quite a few instances of aapt/aapt2 startup on developer's machines:
  this pegs the CPU. We have had a few general complaints about it.

Reviewing the source code for the Android gradle plugin, here is what
they do:

* Build the main app's "full" `R.txt` file.
* For each library, load its `R.txt` file.
* Map each resource in the library's `R.txt` back to the main app
* Write a small `R.java` file for each library: containing *only* the
  lines from the `R.txt` and updated integer values from the main app
  `R.txt` file.

Looking into this, we can do the exact same thing? We have the `R.txt`
file one directory above where we extract resources for each library.
We already had code parsing `R.txt` files I could repurpose, the only
thing *new* is a `R.java` writer: a pretty simple port from java.

The results are great!

    Before:
    3173 ms  _GenerateJavaDesignerForComponentAapt2     1 calls
    After:
      20 ms  GenerateLibraryResources                   1 calls

`_GenerateJavaDesignerForComponent` is now completely gone. This is a
total savings of ~3 seconds on first build and incremental builds
with library changes.

To compare APKs, I used:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F

Which omits a line for each field such as:

    F d 0	0	16	xamarin.forms_performance_integration.R$color int abc_background_cache_hint_selector_material_dark

So then, before these changes:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F | wc -l
    29681

After:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-After.apk | grep ^F | wc -l
    17210

12K less fields in a "Hello World" Xamarin.Forms app!

Comparing file sizes seems good, too:

    $ zipinfo Xamarin.Forms_Performance_Integration-Before.apk | grep classes.dex
    -rw-rw-r--  6.3 unx  3657872 b- defX 19-Mar-28 16:37 classes.dex
    $ zipinfo Xamarin.Forms_Performance_Integration-After.apk | grep classes.dex
    -rw-rw-r--  6.3 unx  3533120 b- defX 19-Mar-28 16:20 classes.dex

Dex file in the APK is ~120KB smaller.
@jonathanpeppers
Copy link
Member

@erikpowa to add a new linker step, you would probably have to add a new linker step to Xamarin.Android.Build.Tasks.dll and build your own version. See: https://github.com/xamarin/xamarin-android/blob/e6326dcba43dfb02c7401de8ae38c03166505df8/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs#L58

I'll file a new issue about this--it is kind of unrelated to the R.java issue we are trying to solve here.

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Mar 29, 2019
Context: https://android.googlesource.com/platform/tools/base/+/refs/heads/master/build-system/builder/
Fixes: dotnet#2680
Fixes: dotnet#2836

The current behavior in the `_GenerateJavaDesignerForComponent`
MSBuild target does the following:

* For each library that has Android resources... (in parallel)
* Run an instance of aapt/aapt2 to generate the `R.java` file for each
  library.
* This actually creates an `R.java` file that contains *every*
  resource id for *every* library. These libraries are not using most
  of these ids.

This has a few problems:

* dotnet#2680 notes a problem where a file is locked on Windows during
  `_GenerateJavaDesignerForComponent`.

    Xamarin.Android.Common.targets(1541,2): The process cannot access the file 'C:\repos\msalnet\tests\devapps\XForms\XForms.Android\obj\Debug\90\lp\26\jl\manifest\AndroidManifest.xml' because it is being used by another process.
        at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
        at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
        at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
        at System.IO.StreamWriter.CreateFile(String path, Boolean append, Boolean checkHost)
        at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding, Int32 bufferSize, Boolean checkHost)
        at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding)
        at Xamarin.Android.Tasks.ManifestDocument.Save(String filename)
        at Xamarin.Android.Tasks.Aapt.GenerateCommandLineCommands(String ManifestFile, String currentAbi, String currentResourceOutputFile)
        at Xamarin.Android.Tasks.Aapt.ProcessManifest(ITaskItem manifestFile)
        at System.Threading.Tasks.Parallel.<>c__DisplayClass30_0`2.<ForEachWorker>b__0(Int32 i)
        at System.Threading.Tasks.Parallel.<>c__DisplayClass17_0`1.<ForWorker>b__1()
        at System.Threading.Tasks.Task.InnerInvoke()
        at System.Threading.Tasks.Task.InnerInvokeWithArg(Task childTask)
        at System.Threading.Tasks.Task.<>c__DisplayClass176_0.<ExecuteSelfReplicating>b__0(Object ) [C:\repos\msalnet\tests\devapps\XForms\XForms.Android\XForms.Android.csproj]

* We are hugely contributing to the dex limit for fields. Apps contain
  exponentially more fields for each library with resources.

An example from @PureWeen:

    1>  trouble writing output: Too many field references to fit in one dex file: 70468; max is 65536.

* Quite a few instances of aapt/aapt2 startup on developer's machines:
  this pegs the CPU. We have had a few general complaints about it.

Reviewing the source code for the Android gradle plugin, here is what
they do:

* Build the main app's "full" `R.txt` file.
* For each library, load its `R.txt` file.
* Map each resource in the library's `R.txt` back to the main app
* Write a small `R.java` file for each library: containing *only* the
  lines from the `R.txt` and updated integer values from the main app
  `R.txt` file.

Looking into this, we can do the exact same thing? We have the `R.txt`
file one directory above where we extract resources for each library.
We already had code parsing `R.txt` files I could repurpose, the only
thing *new* is a `R.java` writer: a pretty simple port from java.

The results are great!

    Before:
    3173 ms  _GenerateJavaDesignerForComponentAapt2     1 calls
    After:
      20 ms  GenerateLibraryResources                   1 calls

`_GenerateJavaDesignerForComponent` is now completely gone. This is a
total savings of ~3 seconds on first build and incremental builds
with library changes.

To compare APKs, I used:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F

Which omits a line for each field such as:

    F d 0	0	16	xamarin.forms_performance_integration.R$color int abc_background_cache_hint_selector_material_dark

So then, before these changes:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F | wc -l
    29681

After:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-After.apk | grep ^F | wc -l
    17210

12K less fields in a "Hello World" Xamarin.Forms app!

Comparing file sizes seems good, too:

    $ zipinfo Xamarin.Forms_Performance_Integration-Before.apk | grep classes.dex
    -rw-rw-r--  6.3 unx  3657872 b- defX 19-Mar-28 16:37 classes.dex
    $ zipinfo Xamarin.Forms_Performance_Integration-After.apk | grep classes.dex
    -rw-rw-r--  6.3 unx  3533120 b- defX 19-Mar-28 16:20 classes.dex

Dex file in the APK is ~120KB smaller.

~~ What if R.txt is missing? ~~

I found this was the case when the
`<GetAdditionalResourcesFromAssemblies/>` MSBuild task runs. This is
an old codepath that allowed old support libraries to work.

In this case, a directory is created such as:

* `obj\Debug\resourcecache\CF390EBB0064FDA00BB090E733D37E89`
  * `adil`
  * `assets`
  * `libs`
  * `res`
  * `AndroidManifest.xml`
  * `classes.jar`

No `R.txt` file?

Checking the zip files we download:

    $ for z in ~/.local/share/Xamarin/zips/*.zip; do zipinfo $z; done | grep R.txt
    # no results

This actually makes sense, since the zip file contains the *actual
resources*.

To make this case work properly, we should just process the main app's
`R.txt` file when no library `R.txt` file is found. This will still be
faster than invoking `aapt`, even though we have more fields than
needed.

~~ Tests ~~

I added a set of unit tests for the `<GenerateLibraryResources/>`
MSBuild task. I also had to remove a few assertions that looked for
the `_GenerateJavaDesignerForComponent` MSBuild target.

Lastly, I added some assertions to a test that uses an old support
library to verify it's main `R.java` reasonably matches the library
`R.java` we generate.
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Apr 1, 2019
Context: https://android.googlesource.com/platform/tools/base/+/refs/heads/master/build-system/builder/
Fixes: dotnet#2680
Fixes: dotnet#2836

The current behavior in the `_GenerateJavaDesignerForComponent`
MSBuild target does the following:

* For each library that has Android resources... (in parallel)
* Run an instance of aapt/aapt2 to generate the `R.java` file for each
  library.
* This actually creates an `R.java` file that contains *every*
  resource id for *every* library. These libraries are not using most
  of these ids.

This has a few problems:

* dotnet#2680 notes a problem where a file is locked on Windows during
  `_GenerateJavaDesignerForComponent`.

    Xamarin.Android.Common.targets(1541,2): The process cannot access the file 'C:\repos\msalnet\tests\devapps\XForms\XForms.Android\obj\Debug\90\lp\26\jl\manifest\AndroidManifest.xml' because it is being used by another process.
        at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
        at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
        at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
        at System.IO.StreamWriter.CreateFile(String path, Boolean append, Boolean checkHost)
        at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding, Int32 bufferSize, Boolean checkHost)
        at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding)
        at Xamarin.Android.Tasks.ManifestDocument.Save(String filename)
        at Xamarin.Android.Tasks.Aapt.GenerateCommandLineCommands(String ManifestFile, String currentAbi, String currentResourceOutputFile)
        at Xamarin.Android.Tasks.Aapt.ProcessManifest(ITaskItem manifestFile)
        at System.Threading.Tasks.Parallel.<>c__DisplayClass30_0`2.<ForEachWorker>b__0(Int32 i)
        at System.Threading.Tasks.Parallel.<>c__DisplayClass17_0`1.<ForWorker>b__1()
        at System.Threading.Tasks.Task.InnerInvoke()
        at System.Threading.Tasks.Task.InnerInvokeWithArg(Task childTask)
        at System.Threading.Tasks.Task.<>c__DisplayClass176_0.<ExecuteSelfReplicating>b__0(Object ) [C:\repos\msalnet\tests\devapps\XForms\XForms.Android\XForms.Android.csproj]

* We are hugely contributing to the dex limit for fields. Apps contain
  exponentially more fields for each library with resources.

An example from @PureWeen:

    1>  trouble writing output: Too many field references to fit in one dex file: 70468; max is 65536.

* Quite a few instances of aapt/aapt2 startup on developer's machines:
  this pegs the CPU. We have had a few general complaints about it.

Reviewing the source code for the Android gradle plugin, here is what
they do:

* Build the main app's "full" `R.txt` file.
* For each library, load its `R.txt` file.
* Map each resource in the library's `R.txt` back to the main app
* Write a small `R.java` file for each library: containing *only* the
  lines from the `R.txt` and updated integer values from the main app
  `R.txt` file.

Looking into this, we can do the exact same thing? We have the `R.txt`
file one directory above where we extract resources for each library.
We already had code parsing `R.txt` files I could repurpose, the only
thing *new* is a `R.java` writer: a pretty simple port from java.

The results are great!

    Before:
    3173 ms  _GenerateJavaDesignerForComponentAapt2     1 calls
    After:
      20 ms  GenerateLibraryResources                   1 calls

`_GenerateJavaDesignerForComponent` is now completely gone. This is a
total savings of ~3 seconds on first build and incremental builds
with library changes.

To compare APKs, I used:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F

Which omits a line for each field such as:

    F d 0	0	16	xamarin.forms_performance_integration.R$color int abc_background_cache_hint_selector_material_dark

So then, before these changes:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F | wc -l
    29681

After:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-After.apk | grep ^F | wc -l
    17210

12K less fields in a "Hello World" Xamarin.Forms app!

Comparing file sizes seems good, too:

    $ zipinfo Xamarin.Forms_Performance_Integration-Before.apk | grep classes.dex
    -rw-rw-r--  6.3 unx  3657872 b- defX 19-Mar-28 16:37 classes.dex
    $ zipinfo Xamarin.Forms_Performance_Integration-After.apk | grep classes.dex
    -rw-rw-r--  6.3 unx  3533120 b- defX 19-Mar-28 16:20 classes.dex

Dex file in the APK is ~120KB smaller.

~~ What if R.txt is missing? ~~

I found this was the case when the
`<GetAdditionalResourcesFromAssemblies/>` MSBuild task runs. This is
an old codepath that allowed old support libraries to work.

In this case, a directory is created such as:

* `obj\Debug\resourcecache\CF390EBB0064FDA00BB090E733D37E89`
  * `adil`
  * `assets`
  * `libs`
  * `res`
  * `AndroidManifest.xml`
  * `classes.jar`

No `R.txt` file?

Checking the zip files we download:

    $ for z in ~/.local/share/Xamarin/zips/*.zip; do zipinfo $z; done | grep R.txt
    # no results

This actually makes sense, since the zip file contains the *actual
resources*.

To make this case work properly, we should just process the main app's
`R.txt` file when no library `R.txt` file is found. This will still be
faster than invoking `aapt`, even though we have more fields than
needed.

~~ Tests ~~

I added a set of unit tests for the `<GenerateLibraryResources/>`
MSBuild task. I also had to remove a few assertions that looked for
the `_GenerateJavaDesignerForComponent` MSBuild target.

Lastly, I added some assertions to a test that uses an old support
library to verify it's main `R.java` reasonably matches the library
`R.java` we generate.
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Apr 1, 2019
Context: https://android.googlesource.com/platform/tools/base/+/refs/heads/master/build-system/builder/
Fixes: dotnet#2680
Fixes: dotnet#2836

The current behavior in the `_GenerateJavaDesignerForComponent`
MSBuild target does the following:

* For each library that has Android resources... (in parallel)
* Run an instance of aapt/aapt2 to generate the `R.java` file for each
  library.
* This actually creates an `R.java` file that contains *every*
  resource id for *every* library. These libraries are not using most
  of these ids.

This has a few problems:

* dotnet#2680 notes a problem where a file is locked on Windows during
  `_GenerateJavaDesignerForComponent`.

    Xamarin.Android.Common.targets(1541,2): The process cannot access the file 'C:\repos\msalnet\tests\devapps\XForms\XForms.Android\obj\Debug\90\lp\26\jl\manifest\AndroidManifest.xml' because it is being used by another process.
        at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
        at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
        at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
        at System.IO.StreamWriter.CreateFile(String path, Boolean append, Boolean checkHost)
        at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding, Int32 bufferSize, Boolean checkHost)
        at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding)
        at Xamarin.Android.Tasks.ManifestDocument.Save(String filename)
        at Xamarin.Android.Tasks.Aapt.GenerateCommandLineCommands(String ManifestFile, String currentAbi, String currentResourceOutputFile)
        at Xamarin.Android.Tasks.Aapt.ProcessManifest(ITaskItem manifestFile)
        at System.Threading.Tasks.Parallel.<>c__DisplayClass30_0`2.<ForEachWorker>b__0(Int32 i)
        at System.Threading.Tasks.Parallel.<>c__DisplayClass17_0`1.<ForWorker>b__1()
        at System.Threading.Tasks.Task.InnerInvoke()
        at System.Threading.Tasks.Task.InnerInvokeWithArg(Task childTask)
        at System.Threading.Tasks.Task.<>c__DisplayClass176_0.<ExecuteSelfReplicating>b__0(Object ) [C:\repos\msalnet\tests\devapps\XForms\XForms.Android\XForms.Android.csproj]

* We are hugely contributing to the dex limit for fields. Apps contain
  exponentially more fields for each library with resources.

An example from @PureWeen:

    1>  trouble writing output: Too many field references to fit in one dex file: 70468; max is 65536.

* Quite a few instances of aapt/aapt2 startup on developer's machines:
  this pegs the CPU. We have had a few general complaints about it.

Reviewing the source code for the Android gradle plugin, here is what
they do:

* Build the main app's "full" `R.txt` file.
* For each library, load its `R.txt` file.
* Map each resource in the library's `R.txt` back to the main app
* Write a small `R.java` file for each library: containing *only* the
  lines from the `R.txt` and updated integer values from the main app
  `R.txt` file.

Looking into this, we can do the exact same thing? We have the `R.txt`
file one directory above where we extract resources for each library.
We already had code parsing `R.txt` files I could repurpose, the only
thing *new* is a `R.java` writer: a pretty simple port from java.

The results are great!

    Before:
    3173 ms  _GenerateJavaDesignerForComponentAapt2     1 calls
    After:
      20 ms  GenerateLibraryResources                   1 calls

`_GenerateJavaDesignerForComponent` is now completely gone. This is a
total savings of ~3 seconds on first build and incremental builds
with library changes.

To compare APKs, I used:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F

Which omits a line for each field such as:

    F d 0	0	16	xamarin.forms_performance_integration.R$color int abc_background_cache_hint_selector_material_dark

So then, before these changes:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F | wc -l
    29681

After:

    $ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-After.apk | grep ^F | wc -l
    17210

12K less fields in a "Hello World" Xamarin.Forms app!

Comparing file sizes seems good, too:

    $ zipinfo Xamarin.Forms_Performance_Integration-Before.apk | grep classes.dex
    -rw-rw-r--  6.3 unx  3657872 b- defX 19-Mar-28 16:37 classes.dex
    $ zipinfo Xamarin.Forms_Performance_Integration-After.apk | grep classes.dex
    -rw-rw-r--  6.3 unx  3533120 b- defX 19-Mar-28 16:20 classes.dex

Dex file in the APK is ~120KB smaller.

~~ What if R.txt is missing? ~~

I found this was the case when the
`<GetAdditionalResourcesFromAssemblies/>` MSBuild task runs. This is
an old codepath that allowed old support libraries to work.

In this case, a directory is created such as:

* `obj\Debug\resourcecache\CF390EBB0064FDA00BB090E733D37E89`
  * `adil`
  * `assets`
  * `libs`
  * `res`
  * `AndroidManifest.xml`
  * `classes.jar`

No `R.txt` file?

Checking the zip files we download:

    $ for z in ~/.local/share/Xamarin/zips/*.zip; do zipinfo $z; done | grep R.txt
    # no results

This actually makes sense, since the zip file contains the *actual
resources*.

To make this case work properly, we should just process the main app's
`R.txt` file when no library `R.txt` file is found. This will still be
faster than invoking `aapt`, even though we have more fields than
needed.

~~ Tests ~~

I added a set of unit tests for the `<GenerateLibraryResources/>`
MSBuild task. I also had to remove a few assertions that looked for
the `_GenerateJavaDesignerForComponent` MSBuild target.

Lastly, I added some assertions to a test that uses an old support
library to verify it's main `R.java` reasonably matches the library
`R.java` we generate.
jonpryor pushed a commit that referenced this issue Apr 1, 2019
)

Context: https://android.googlesource.com/platform/tools/base/+/refs/heads/master/build-system/builder/
Fixes: #2680
Fixes: #2836

The current behavior in the `_GenerateJavaDesignerForComponent`
MSBuild target does the following:

  * For each library that has Android resources... (in parallel)
  * Run an instance of `aapt`/`aapt2` to generate the `R.java` file
    for each library.
  * This actually creates an `R.java` file that contains *every*
    resource id for *every* library.  These libraries are not using
    most of these ids.

This has a few problems:

  * Issue #2680 notes a problem where a file is locked on Windows
    during `_GenerateJavaDesignerForComponent`:

        Xamarin.Android.Common.targets(1541,2): The process cannot access the file 'C:\repos\msalnet\tests\devapps\XForms\XForms.Android\obj\Debug\90\lp\26\jl\manifest\AndroidManifest.xml' because it is being used by another process.
            at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
            at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
            at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
            at System.IO.StreamWriter.CreateFile(String path, Boolean append, Boolean checkHost)
            at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding, Int32 bufferSize, Boolean checkHost)
            at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding)
            at Xamarin.Android.Tasks.ManifestDocument.Save(String filename)
            at Xamarin.Android.Tasks.Aapt.GenerateCommandLineCommands(String ManifestFile, String currentAbi, String currentResourceOutputFile)
            at Xamarin.Android.Tasks.Aapt.ProcessManifest(ITaskItem manifestFile)
            at System.Threading.Tasks.Parallel.<>c__DisplayClass30_0`2.<ForEachWorker>b__0(Int32 i)
            at System.Threading.Tasks.Parallel.<>c__DisplayClass17_0`1.<ForWorker>b__1()
            at System.Threading.Tasks.Task.InnerInvoke()
            at System.Threading.Tasks.Task.InnerInvokeWithArg(Task childTask)
            at System.Threading.Tasks.Task.<>c__DisplayClass176_0.<ExecuteSelfReplicating>b__0(Object ) [C:\repos\msalnet\tests\devapps\XForms\XForms.Android\XForms.Android.csproj]

  * We are hugely contributing to the dex limit for fields.  Apps
    contain exponentially more fields for each library with resources.

    An example from @PureWeen:

        1>  trouble writing output: Too many field references to fit in one dex file: 70468; max is 65536.

  * Quite a few instances of `aapt`/`aapt2` startup on developer's
    machines: this pegs the CPU.  We have had a few general
    complaints about it.

Reviewing the source code for the Android gradle plugin, here is what
they do:

 1. Build the main app's "full" `R.txt` file.
 2. For each library, load its `R.txt` file.
 3. Map each resource in the library's `R.txt` back to the main app
 4. Write a small `R.java` file for each library containing *only*
    the lines from the `R.txt` and updated integer values from the
    main app `R.txt` file.

Looking into this, can we do the exact same thing?  We have the
`R.txt` file one directory above where we extract resources for each
library.  We already had code parsing `R.txt` files we could
repurpose.  The only thing *new* is a `R.java` writer: a pretty
simple port from java.

The results are great!

	Before:
	3173 ms  _GenerateJavaDesignerForComponentAapt2     1 calls
	After:
	  20 ms  GenerateLibraryResources                   1 calls

`_GenerateJavaDesignerForComponent` is now completely gone.  This is
a total savings of ~3 seconds on first build and incremental builds
with library changes.

To compare APKs, I used:

	$ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F

Which omits a line for each field such as:

	F d 0	0	16	xamarin.forms_performance_integration.R$color int abc_background_cache_hint_selector_material_dark

So then, before these changes there were ~30000 fields:

	$ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F | wc -l
	29681

After, there are less than 18000 (58%!):

	$ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-After.apk | grep ^F | wc -l
	17210

12K less fields in a "Hello World" Xamarin.Forms app!

Comparing file sizes seems good, too:

	$ zipinfo Xamarin.Forms_Performance_Integration-Before.apk | grep classes.dex
	-rw-rw-r--  6.3 unx  3657872 b- defX 19-Mar-28 16:37 classes.dex
	$ zipinfo Xamarin.Forms_Performance_Integration-After.apk | grep classes.dex
	-rw-rw-r--  6.3 unx  3533120 b- defX 19-Mar-28 16:20 classes.dex

The `.dex` file in the `.apk` is ~120KB smaller.


~~ What if R.txt is missing? ~~

I found this was the case when the
`<GetAdditionalResourcesFromAssemblies/>` MSBuild task runs.  This is
an old codepath that allowed old support libraries to work.

In this case, a directory is created such as:

  * `obj\Debug\resourcecache\CF390EBB0064FDA00BB090E733D37E89`
      * `aidl`
      * `AndroidManifest.xml`
      * `assets`
      * `classes.jar`
      * `libs`
      * `res`

No `R.txt` file?

Checking the zip files we download:

	$ for z in ~/.local/share/Xamarin/zips/*.zip; do zipinfo $z; done | grep R.txt
	# no results

This actually makes sense, since the zip file contains the
*actual resources*.

To make this case work properly, we should just process the main
app's `R.txt` file when no library `R.txt` file is found.  This will
still be faster than invoking `aapt`, even though we have more
fields than needed.


~~ Tests ~~

I added a set of unit tests for the `<GenerateLibraryResources/>`
MSBuild task.  I also had to remove a few assertions that looked for
the `_GenerateJavaDesignerForComponent` MSBuild target.

Lastly, I added some assertions to a test that uses an old support
library to verify it's main `R.java` reasonably matches the library
`R.java` we generate.
jonathanpeppers added a commit that referenced this issue Apr 1, 2019
)

Context: https://android.googlesource.com/platform/tools/base/+/refs/heads/master/build-system/builder/
Fixes: #2680
Fixes: #2836

The current behavior in the `_GenerateJavaDesignerForComponent`
MSBuild target does the following:

  * For each library that has Android resources... (in parallel)
  * Run an instance of `aapt`/`aapt2` to generate the `R.java` file
    for each library.
  * This actually creates an `R.java` file that contains *every*
    resource id for *every* library.  These libraries are not using
    most of these ids.

This has a few problems:

  * Issue #2680 notes a problem where a file is locked on Windows
    during `_GenerateJavaDesignerForComponent`:

        Xamarin.Android.Common.targets(1541,2): The process cannot access the file 'C:\repos\msalnet\tests\devapps\XForms\XForms.Android\obj\Debug\90\lp\26\jl\manifest\AndroidManifest.xml' because it is being used by another process.
            at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
            at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
            at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
            at System.IO.StreamWriter.CreateFile(String path, Boolean append, Boolean checkHost)
            at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding, Int32 bufferSize, Boolean checkHost)
            at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding)
            at Xamarin.Android.Tasks.ManifestDocument.Save(String filename)
            at Xamarin.Android.Tasks.Aapt.GenerateCommandLineCommands(String ManifestFile, String currentAbi, String currentResourceOutputFile)
            at Xamarin.Android.Tasks.Aapt.ProcessManifest(ITaskItem manifestFile)
            at System.Threading.Tasks.Parallel.<>c__DisplayClass30_0`2.<ForEachWorker>b__0(Int32 i)
            at System.Threading.Tasks.Parallel.<>c__DisplayClass17_0`1.<ForWorker>b__1()
            at System.Threading.Tasks.Task.InnerInvoke()
            at System.Threading.Tasks.Task.InnerInvokeWithArg(Task childTask)
            at System.Threading.Tasks.Task.<>c__DisplayClass176_0.<ExecuteSelfReplicating>b__0(Object ) [C:\repos\msalnet\tests\devapps\XForms\XForms.Android\XForms.Android.csproj]

  * We are hugely contributing to the dex limit for fields.  Apps
    contain exponentially more fields for each library with resources.

    An example from @PureWeen:

        1>  trouble writing output: Too many field references to fit in one dex file: 70468; max is 65536.

  * Quite a few instances of `aapt`/`aapt2` startup on developer's
    machines: this pegs the CPU.  We have had a few general
    complaints about it.

Reviewing the source code for the Android gradle plugin, here is what
they do:

 1. Build the main app's "full" `R.txt` file.
 2. For each library, load its `R.txt` file.
 3. Map each resource in the library's `R.txt` back to the main app
 4. Write a small `R.java` file for each library containing *only*
    the lines from the `R.txt` and updated integer values from the
    main app `R.txt` file.

Looking into this, can we do the exact same thing?  We have the
`R.txt` file one directory above where we extract resources for each
library.  We already had code parsing `R.txt` files we could
repurpose.  The only thing *new* is a `R.java` writer: a pretty
simple port from java.

The results are great!

	Before:
	3173 ms  _GenerateJavaDesignerForComponentAapt2     1 calls
	After:
	  20 ms  GenerateLibraryResources                   1 calls

`_GenerateJavaDesignerForComponent` is now completely gone.  This is
a total savings of ~3 seconds on first build and incremental builds
with library changes.

To compare APKs, I used:

	$ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F

Which omits a line for each field such as:

	F d 0	0	16	xamarin.forms_performance_integration.R$color int abc_background_cache_hint_selector_material_dark

So then, before these changes there were ~30000 fields:

	$ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-Before.apk | grep ^F | wc -l
	29681

After, there are less than 18000 (58%!):

	$ ~/android-toolchain/sdk/tools/bin/apkanalyzer dex packages Xamarin.Forms_Performance_Integration-After.apk | grep ^F | wc -l
	17210

12K less fields in a "Hello World" Xamarin.Forms app!

Comparing file sizes seems good, too:

	$ zipinfo Xamarin.Forms_Performance_Integration-Before.apk | grep classes.dex
	-rw-rw-r--  6.3 unx  3657872 b- defX 19-Mar-28 16:37 classes.dex
	$ zipinfo Xamarin.Forms_Performance_Integration-After.apk | grep classes.dex
	-rw-rw-r--  6.3 unx  3533120 b- defX 19-Mar-28 16:20 classes.dex

The `.dex` file in the `.apk` is ~120KB smaller.

~~ What if R.txt is missing? ~~

I found this was the case when the
`<GetAdditionalResourcesFromAssemblies/>` MSBuild task runs.  This is
an old codepath that allowed old support libraries to work.

In this case, a directory is created such as:

  * `obj\Debug\resourcecache\CF390EBB0064FDA00BB090E733D37E89`
      * `aidl`
      * `AndroidManifest.xml`
      * `assets`
      * `classes.jar`
      * `libs`
      * `res`

No `R.txt` file?

Checking the zip files we download:

	$ for z in ~/.local/share/Xamarin/zips/*.zip; do zipinfo $z; done | grep R.txt
	# no results

This actually makes sense, since the zip file contains the
*actual resources*.

To make this case work properly, we should just process the main
app's `R.txt` file when no library `R.txt` file is found.  This will
still be faster than invoking `aapt`, even though we have more
fields than needed.

~~ Tests ~~

I added a set of unit tests for the `<GenerateLibraryResources/>`
MSBuild task.  I also had to remove a few assertions that looked for
the `_GenerateJavaDesignerForComponent` MSBuild target.

Lastly, I added some assertions to a test that uses an old support
library to verify it's main `R.java` reasonably matches the library
`R.java` we generate.
@ghost ghost locked as resolved and limited conversation to collaborators Jun 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Proposed change to current functionality.
Projects
None yet
Development

No branches or pull requests

4 participants