-
Notifications
You must be signed in to change notification settings - Fork 529
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
Commits on Apr 1, 2019
-
[Xamarin.Android.Build.Tasks] generate R.java like Android Studio
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.
Configuration menu - View commit details
-
Copy full SHA for 22d43cb - Browse repository at this point
Copy the full SHA 22d43cbView commit details -
Remove Tuple<> based on feedback
It's a bit cleaner to use a string as the key.
Configuration menu - View commit details
-
Copy full SHA for 5211333 - Browse repository at this point
Copy the full SHA 5211333View commit details
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.