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

[Xamarin.Android.Build.Tasks] generate R.java like Android Studio #2896

Merged
merged 2 commits into from
Apr 1, 2019

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Mar 29, 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:

    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 jonathanpeppers added the do-not-merge PR should not be merged. label Mar 29, 2019
@jonathanpeppers
Copy link
Member Author

jonathanpeppers commented Mar 29, 2019

Logs: r.java.zip

I think I want to use some of the TPL helpers in #2881 before we merge this.

@jonathanpeppers jonathanpeppers force-pushed the libraries-r.java branch 2 times, most recently from 55deedd to 55f194d Compare April 1, 2019 13:54
@jonathanpeppers jonathanpeppers removed the do-not-merge PR should not be merged. label 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.
It's a bit cleaner to use a string as the key.
@jonathanpeppers
Copy link
Member Author

jonathanpeppers commented Apr 1, 2019

A single MSBuild test failure occurred on Windows, it seemed unrelated?

This should fix that issue: #2911

@jonpryor jonpryor merged commit 0355558 into dotnet:master Apr 1, 2019
jonathanpeppers added a commit that referenced this pull request 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 jonathanpeppers deleted the libraries-r.java branch April 29, 2019 12:21
@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 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.

None yet

4 participants