-
Notifications
You must be signed in to change notification settings - Fork 534
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
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
Logs: r.java.zip I think I want to use some of the TPL helpers in #2881 before we merge this. |
jonathanpeppers
commented
Mar 29, 2019
src/Xamarin.Android.Build.Tasks/Tasks/GenerateLibraryResources.cs
Outdated
Show resolved
Hide resolved
jonathanpeppers
force-pushed
the
libraries-r.java
branch
2 times, most recently
from
April 1, 2019 13:54
55deedd
to
55f194d
Compare
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
force-pushed
the
libraries-r.java
branch
from
April 1, 2019 14:18
55f194d
to
22d43cb
Compare
dellis1972
approved these changes
Apr 1, 2019
jonpryor
reviewed
Apr 1, 2019
src/Xamarin.Android.Build.Tasks/Tasks/GenerateLibraryResources.cs
Outdated
Show resolved
Hide resolved
It's a bit cleaner to use a string as the key.
A single MSBuild test failure occurred on Windows, it seemed unrelated? This should fix that issue: #2911 |
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.
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: 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:
R.java
file for eachlibrary.
R.java
file that contains everyresource id for every library. These libraries are not using most
of these ids.
This has a few problems:
_GenerateJavaDesignerForComponent
.exponentially more fields for each library with resources.
An example from @PureWeen:
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:
R.txt
file.R.txt
file.R.txt
back to the main appR.java
file for each library: containing only thelines from the
R.txt
and updated integer values from the main appR.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 onlything new is a
R.java
writer: a pretty simple port from java.The results are great!
_GenerateJavaDesignerForComponent
is now completely gone. This is atotal savings of ~3 seconds on first build and incremental builds
with library changes.
To compare APKs, I used:
Which omits a line for each field such as:
So then, before these changes:
After:
12K less fields in a "Hello World" Xamarin.Forms app!
Comparing file sizes seems good, too:
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 isan 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:
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 libraryR.txt
file is found. This will still befaster than invoking
aapt
, even though we have more fields thanneeded.
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 libraryR.java
we generate.