-
Notifications
You must be signed in to change notification settings - Fork 564
[Xamarin.Android.Build.Tasks] add $(_AndroidAllowDeltaInstall) support #4643
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
Conversation
brendanzagaeski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Draft release notes
As this PR is finalized for merge, when you get a chance, you can add a release note for it to Documentation/release-notes/4643.md as part of the PR.
It looks like this PR can use the new feature or behavior template and probably copy and paste some of the text that this PR is already adding to BuildProcess.md.
If a timing comparison might be available for an example deployment scenario, that would be great to include to give users a rough sense of what kind of improvement they might see.
And if there are any caveats or known issues, those would be good to mention too. Thanks much!
|
@brendanzagaeski - I (finally) added release notes. Can you please approve now. |
Context: xamarin/monodroid#1089 This adds an experimental feature for speeding up incremental deploys. TODO: - [ ] - Update `BuildProcess.md` with docs for the new publish property - [ ] - Add needed files to the installer - [ ] - Add/update one test in `MSBuildDeviceIntegration`, to verify the new feature works - [ ] - After everything is green, we can merge the `monodroid` side and update this PR to target latest xamarin/monodroid/master
This is needed (currently) for the delta install "installer" exes, which come from here.
Jeremie created this new feed, which can house Xamarin public packages. The installer NuGet (normally built manually on Jeremie's machine when updates are needed) live here now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks much for the draft notes!
Reading through these I'm wondering which of the following sounds right:
-
Maybe this PR is primarily intended as groundwork for future development and investigation to understand whether delta installs will benefit real user scenarios?
In this case, the feature can potentially be omitted from the release notes until a future time and the MSBuild property could be prefixed with an underscore (
_ AndroidAllowDeltaInstall) and omitted fromDocumentation/guides/BuildProcess.md. -
Alternatively, maybe there is a plan to allow some users to try the feature and report back how it behaves with their projects? Perhaps something similar to the effort from last year (https://github.com/JonDouglas/AndroidPerformanceAndAppSizeChallenge)?
If that sounds right, then as I'm reading this through from a user perspective, I find myself stumbling over the term "delta install" because it sounds like a way to make deployments faster, so I immediately think "fast deployment." (Note also that a web search for
"delta install" site:developer.android.comdoes not return any results, so the term "delta install" probably isn't too familiar to any Android developers, even Android Studio users.)Given that context, maybe it could be OK to control this feature with a new colon-separated item for
$(AndroidFastDeploymentType)instead of its own property?An admitted complication with this suggestion is that currently
$(AndroidFastDeploymentType)is only intended for Debug deployments, while this new APK delta mode is also relevant for Release configuration deployments. But I think this might be OK because if the feature graduates to non-experimental status, it seems quite possible that users would almost never need separate control over this feature.
Thanks in advance!
|
@brendanzagaeski It's really option 2. Yes, it aims to make deployments faster, but it's conceptually separate from fast deploy and complementary to it. Android Studio does use this technology. It's used to make "Apply Changes" faster, when only resources are updated, and also make all deploys faster by only installing changes. Still, this is experimental so we don't want to say much about it, yet. I'm also fine removing it from the release notes, but it's probably a tiny bit better to have it there. |
Mhm. That's what I was seeing when I was trying web searches. So for me, from a user messaging perspective, if possible, I'd love to avoid making the term "delta install" dev-facing, especially since there are already quite a few vocabulary terms around how to get app changes from the IDE to devices.
It's conceptually separate in terms of implementation, but in terms of customer feature use, it seems like a trickier distinction to draw. A problem is that the existing feature uses the term "fast deployment" instead of something like "deploy assemblies outside the APK." The generality of the term "fast deployment" gobbles up the vocabulary space. But since that term has already been around for a long time in Xamarin.Android, that's water under the bridge now There might be a way around this by adding another term that's even more general than "fast deployment," but that'd take some brainstorming.
The good and maybe bad (because like the name "fast deployment," this mechanism also eats up conceptual space with its generality) news is that a colon-separated item in <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
<AndroidFastDeploymentType>ApkDiff:Assemblies</AndroidFastDeploymentType>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<AndroidFastDeploymentType>ApkDiff</AndroidFastDeploymentType>
</PropertyGroup>Using @dellis1972 can probably tell me if I'm getting too loony in suggesting
|
|
@brendanzagaeski Maybe we should just remove it from the release notes for now. Would you be OK with that? This is an experimental feature, primarily to enable internal performance testing. We'd rather not make customers aware of it until later, when we have more confidence in how we'll productize it. Ideally, it'll be on by default eventually, but we're not there yet. |
|
@BretJohnson if you want to leave it undocumented, prefix the property with an underscore, so it will be |
|
On names, I rather like |
|
On second though, after discussing more with @jonathanpeppers , I'll rename to Later, when/if this is a productized feature, on by default, we can add a different option potentially to disable it. |
|
I renamed to See rename changes in this PR and in https://github.com/xamarin/monodroid/pull/1089 |
Changes: xamarin/monodroid@5849ea1...e795bce * [tools/msbuild] add $(_AndroidAllowDeltaInstall) * Bump to xamarin/androidtools@739afe5
|
@brendanzagaeski per above, I made this setting internal only for now, renaming it to |
brendanzagaeski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved by me for the release notes and documentation side of things. Sounds good to use an underscore-prefixed name for the moment to give some more flexibility for future decisions. Thanks for bearing with me on mulling over how to present the feature to users!
| <_MSBuildFiles Include="$(MSBuildSrcDir)\Xamarin.Android.Build.Debugging.Tasks.pdb" /> | ||
| <_MSBuildFiles Include="$(MSBuildSrcDir)\Xamarin.Android.Common.Debugging.props" /> | ||
| <_MSBuildFiles Include="$(MSBuildSrcDir)\Xamarin.Android.Common.Debugging.targets" /> | ||
| <_MSBuildFiles Include="$(MSBuildSrcDir)\lib\arm64-v8a\installer" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this means we're redistributing installer, this means that we need to update the ThirdPartyNotices.txt that we generate and include in the installers so that the licensing information for installer is available to our users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per @garuma:
they should already have the right entry, the license and copyright is the same as the other tooling (Apache 2.0 copyright Android Open Source Project)
but they may need a specific entry for the entries, their system is different
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonathanpeppers do we need anything else for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
A link to some form of "spec" or "documentation" would also be handy. You never know when you'll need to check a spec, e.g. #4681 (comment) |
#4643) Changes: https://github.com/xamarin/monodroid/compare/52312f241428fc78f4d15baaa80b56f85a180427...8d51d0af0572db1583a8ab5ec095bec3046ae476 * xamarin/monodroid@8d51d0af0: Bump to xamarin/androidtools/d16-7@767cfdfe (#1095) * xamarin/monodroid@a0fcae82e: [tools/msbuild] add $(_AndroidAllowDeltaInstall) (#1089) * xamarin/monodroid@781e564e5: [tests] Fix build from xamarin-android (#1093) Add ***Experimental*** "`.apk` delta installation" support. *What do we want?!* Faster `.apk` installs! (See also PR #4690.) The question, as ever, is *how* to reduce the time of `.apk` installs, and what the tradeoffs are. Fast dev, which removes assemblies from the `.apk`, is one way to do that; PR #4690 is "fastdev v2", which does more there. But for everything else in the `.apk` -- Android Assets, Resources, and more -- how do we make deploying it faster? Here is one way. Android Studio contains an [`installer`][0] utility, built for each supported Android target platform, which is installed onto the target device as `/data/local/tmp/.studio/bin/installer`. `installer` works in terms of "protocol buffers", and can *in place update* an `.apk` file installed on the Android target via a "delta" between the currently installed `.apk` and the "to be installed" `.apk`. `installer` deals with *just* `.apk` contents, and thus doesn't special-case Android Assets, Resources, or IL assemblies. As the `installer` utility is Apache 2 licensed, Xamarin.Android can redistribute and use the `installer` utility as well. To use `installer` to deploy `.apk` updates: 1. Use a Xamarin.Android install which contains `installer` integration, e.g. via `make make prepare-external-git-dependencies` or via a master AzDO-hosted build. 2. Install an app: msbuild /t:Install samples/HelloWorld/HelloWorld.csproj 3. Make a change, e.g. by editing `samples/HelloWorld/Resources/layout/Main.axml`. 4. Install the app, enabling use of this new feature: msbuild /t:Install samples/HelloWorld/HelloWorld.csproj /p:_AndroidAllowDeltaInstall=True This triggers the following in the code: a. Create the `.apk` to deploy, "as normal". b. Check to see if `installer` is present on the Android target. If not, deploy to `/data/local/tmp/.studio/bin/installer`. This happens optimistically, by trying to start up `installer`, looking for certain error codes that indicate the installer isn't present, then installing `installer` if needed. c. Invoke the target's `installer` to obtain the contents of the *currently installed* `.apk` file. d. Compute a diff between (4.c) and the `.apk` in (4.a). e. Deploy the diff to the Android target. f. Invoke `installer` to apply the diff (4.e) to the installed `.apk`, updating its contents to that of (4.a). g. If this process fails for any reason, a "normal" `adb install` is performed. This new workflow is only performed when: * The `$(_AndroidAllowDeltaInstall)` MSBuild Property is True * The App is *already deployed* to the Android target. * The Android target is running API-24 (Android v7.0) or later. Some initial Install timing results for step (4), against a Pixel 3a. Timings are total times, including the build, of which the deploy part takes on the order of 1.5 - 2.5 sec or so: | Test | Normal (s) | Delta (s) | |:----------------------------------------------|--------------:|--------------:| | 18MB APK, fast deploy off, updated 1 assembly | 6.3 | 5.6 | | 9MB APK, fast deploy on, updated 1 resource | 5.6 | 5.3 | [0]: https://android.googlesource.com/platform/tools/base/+/studio-master-dev/deploy/installer
This PR:
Adds an experimental feature for speeding up incremental deploys of APKs, just sending over changes, when _AndroidAllowDeltaInstall is set. See details here https://github.com/xamarin/monodroid/pull/1089
Adds the code to enable delta install (run on the PC) and the Android installers associated with it (run on the device) to androidtools and Xamarin Android. This code used to be in the designer repo and is now shared, part of Xamarin Android. This code is used for both delta install (new experimental feature, used for all APK deploys when _AndroidAllowDeltaInstall is enabled) and for Apply Changes (existing non-experimental feature), updating resources only on the device when the user invokes the Apply Changes command.
Bump to xamarin/monodroid/master@e795bce
Changes: https://github.com/xamarin/monodroid/compare/5849ea1...e795bce