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

D8 and R8 integration #2019

Merged
merged 2 commits into from
Oct 30, 2018
Merged

D8 and R8 integration #2019

merged 2 commits into from
Oct 30, 2018

Commits on Oct 29, 2018

  1. [Xamarin.Android.Build.Tasks] d8 and r8 support

    Fixes: dotnet#1423
    Fixes: dotnet#2040
    
    ~~ Overview ~~
    
    This enables d8 and r8 integration for Xamarin.Android. d8 is a
    "next-generation" dex compiler (over dx), and r8 is a replacement for
    proguard. For full details on the feature, see
    `Documentation/guides/D8andR8.md`.
    
    ~~ MSBuild targets changes ~~
    
    New MSBuild properties include:
    
    - `$(AndroidDexTool)` - an enum-style property with options: `dx` and
      `d8`. Defaults to `dx`.
    - `$(AndroidLinkTool)` - an enum-style property, that can be left
      blank to disable code shrinking. Valid values are `proguard` and
      `r8`.
    
    Existing MSBuild properties still retain the old behavior:
    
    - `$(AndroidEnableProguard)` will set `$(AndroidLinkTool)` to
      `proguard`, although it is no longer used internally by
      Xamarin.Android targets.
    - `$(AndroidEnableDesugar)` will default to `true` if `d8` or `r8` are
      used.
    
    New MSBuild tasks include:
    
    - `<D8 />` - runs `d8.jar` with the required options for
      Xamarin.Android.
    - `<R8 />` - subclasses `<D8 />`, and adds functionality for multi-dex
      and code-shrinking.
    
    Additionally, any new MSBuild targets are placed in a new
    `Xamarin.Android.D8.targets` file. This is good first step to make
    `Xamarin.Android.Common.targets` smaller.
    
    Additionally:
    
    * `build.props` is now invalidated via the `$(AndroidDexTool)`
      and `$(AndroidLinkTool)` properties, instead of
      `$(AndroidEnableProguard)`
    * `build.props` is invalidated via `$(AndroidEnableDesugar)`
    
    ~~ Test changes ~~
    
    Tests that need to validate various combinations of properties are now
    using parameters such as:
    
        [Values ("dx", "d8")] string dexTool, [Values ("", "proguard", "r8")] string linkTool
    
    Set on the `XamarinAndroidApplicationProject` such as:
    
        var proj = new XamarinAndroidApplicationProject {
            DexTool = dexTool,
            LinkTool = linkTool,
        };
    
    In other cases, a simple `useD8` flag was added to set `DexTool="d8"`.
    
    Since adding test parameters, exponentially causes our test cases to
    expand, I removed some non-essential parameters:
    
    - `BuildProguardEnabledProject` dropped `useLatestSdk`, as it does not
      seem applicable here (and is deprecated). Otherwise would have 24
      test cases...
    - `BuildApplicationWithSpacesInPath` dropped `isRelease` and defaulted
      it to `true`. We aren't going to likely encounter issues with spaces
      in a path that happen *only* in a `Debug` build. Otherwise we would
      have 24 test cases here...
    - `Desugar` dropped `enableDesugar` because it is certain this
      application project will not build *without* desugar. We don't need
      to test this, and would have 24 test cases otherwise...
    
    Also dropped some `[TestCaseSource]` attributes where the `[Values]`
    parameter was much cleaner.
    
    ~~ Changes to test/sample projects ~~
    
    `HelloWorld` - `$(AndroidDexTool)` set to `d8` if unspecified. So we
    can track the performance benefit.
    
    `Xamarin.Forms Integration` - uses `d8` and `$(AndroidLinkTool)` set
    to `r8`, using a `proguard.cfg` file for Xamarin.Forms. Will help us
    track startup performance benefits of Java code shrinking and build
    performance.
    
    `Mono.Android-Tests` - uses `d8` and `$(AndroidLinkTool)` set to `r8`,
    to verify these on-device tests pass.
    
    `Runtime-MultiDex` - `$(AndroidDexTool)` set to `d8` if unspecified,
    to validate multi-dex is working properly with `d8`.
    
    ~~ Xamarin.Android build changes ~~
    
    This adds https://r8.googlesource.com/r8 as a submodule in
    `external/r8`, and builds `d8.jar` and `r8.jar` via `src/r8/r8.csproj`.
    
    Currently using version 1.2.50 of r8.
    
    ~~ Deployment changes ~~
    
    Three new files will need to be included in Xamarin.Android
    installers:
    
    - `d8.jar`
    - `r8.jar`
    - `Xamarin.Android.D8.targets`
    
    ~~ General changes ~~
    
    * Fixed doc for `xa4304` error code
    * Added `xa4305` error code for `CompileToDalvik`, `R8`, and
      `CreateMultiDexMainDexClassList`
    * Removed log messages in `CreateMultiDexMainDexClassList`
    
    ~~ Performance Comparison ~~
    
    | MSBuild Target         | Options Enabled        | Time    | APK size (bytes) | dex size (bytes) |
    | ---                    | ---                    | ---:    | ---:             | ---:             |
    | _CompileToDalvikWithDx | n/a                    | 11074ms | 13378157         | 3894720          |
    | _CompileToDalvikWithD8 | d8, (desugar enabled)  | 8543ms  | 13124205         | 3314064          |
    | _CompileToDalvikWithD8 | d8, (desugar disabled) | 9550ms  | 13124205         | 3314064          |
    | _CompileToDalvikWithDx | multi-dex              | 15632ms | 13390498         | 3916496          |
    | _CompileToDalvikWithD8 | d8, multi-dex          | 25979ms | 13054626         | 3264096          |
    | _CompileToDalvikWithDx | proguard               | 11903ms | 12804717         | 2446964          |
    | _CompileToDalvikWithD8 | d8, r8                 | 13799ms | 12513901         | 1835588          |
    | _CompileToDalvikWithDx | multi-dex, proguard    | 17279ms | 12804770         | 2449512          |
    | _CompileToDalvikWithD8 | d8, multi-dex, r8      | 13792ms | 12513954         | 1837588          |
    
    _NOTE: desugar is enabled by default with d8/r8_
    
    I timed this builds with [this script][powershell_script], with a
    "Hello World" Xamarin.Forms app. Build logs here:
    [d8andr8.zip][d8andr8_zip]
    
    One can draw their own conclusions on which options are faster,
    better, smaller. See further detail in `D8andR8.md`.
    
    [powershell_script]: https://github.com/jonathanpeppers/HelloWorld/blob/39e2854f6ca39c0941fb8bd6f2a16d8b7663003e/build.ps1
    [d8andr8_zip]: https://github.com/xamarin/xamarin-android/files/2470385/d8andr8.zip
    
    Co-authored-by: Atsushi Eno <atsushieno@gmail.com>
    jonathanpeppers and atsushieno committed Oct 29, 2018
    Configuration menu
    Copy the full SHA
    c8719f0 View commit details
    Browse the repository at this point in the history
  2. Requested changes from review + build refactoring

    ~~ xa-prep-tasks ~~
    
    The `<DownloadUri/>` MSBuild task now has an optional `HashHeader`
    property:
    - If set, a http HEAD request is made to the URL, and the destination
      file path gets a suffix added for the value.
    - To make this work properly, `DestinationFiles` is also an `[Output]`
      property, since these paths could change as a result.
    
    ~~ android-toolchain ~~
    
    Chromium's `depot_tools` are downloaded the same as all of our other
    dependencies. I had to update the `<DownloadUri/>` MSBuild task to
    have an optional `HashHeader` property and made `DestinationFiles` an
    `<Output/>`. We can use this to populate `@(_DownloadedItem)`.
    
    `_UnzipFiles` also "bootstraps" `depot_tools`, and has to set/unset
    `$PATH` appropriately. I did this in the same place as the license
    acceptance for the Android SDK.
    
    ~~ r8.targets ~~
    
    - Removed all of the `depot_tools` downloading code, and setup
      `android-toolchain` as a `@(ProjectReference)`
    - All targets that set `$PATH`, unset it when finished
    - Use `$(_GradleArgs)` to supply `--no-daemon`
    - `_CleanR8` should delete `*.jar` files in our build tree
    
    ~~ Xamarin.Android.Build.Tasks ~~
    
    - Fixed any formatting that we noticed
    - Renamed `$(AndroidD8JarPath)` and `$(AndroidR8JarPath)` to be
      prefixed with `Android`
    - Refactored `$(IntermediateOutputPath)_dex_stamp` stamp file in
      `_CompileToDalvikWithDx` and `_CompileToDalvikWithD8` to match the
      new convention of `$(_AndroidStampDirectory)_CompileToDalvik.stamp`
    - `*.dex` files weren't in `FileWrites`???
    - `CompileToDalvik` had a `DexOutputs` output property that was
      completely unused, so I removed it. Also removed extra log messages.
    jonathanpeppers committed Oct 29, 2018
    Configuration menu
    Copy the full SHA
    2e8acea View commit details
    Browse the repository at this point in the history