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] Link Resource Nested Types from assemblies #5317

Merged
merged 1 commit into from
Mar 2, 2021

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Nov 19, 2020

The idea is to remove the Resource class NestedTypes from the final
linked assemblies in release mode. We do this by examining all the
locations where the fields are used and replacing those IL calls with a
call which specifies the actual resource id directly. This will remove
the need to call global::Android.Runtime.ResourceIdManager.UpdateIdValues();
completely. It will also allow us to completely remove the Resource
class sub classes. Due to an odd linker failure we cannot completely
remove the Resource class itself. However it will be completely empty.

As mentioned the call to UpdateIdValues will no longer be required.
This should reduce startup time slightly. One thing to note is this will
only be true for Release apps when enabled. Debug apps will still
call UpdateIdValues. This is due to the time is takes to run
the Linker Step. Applying this to a Debug build would effect the build
times during development.

A new MSBuild property AndroidLinkResources has been added to control
this feature. It is turned off by default for now.

This seems to save between 100-280kb for an apk. This is dependent on
how many assemblies are Android assemblies and contain or use Resources.

Xamarin.Forms base app apkdiff results

Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
  -          10 assemblies/Mono.Android.dll
  -       4,001 assemblies/Xamarin.Essentials.dll
  -      45,426 assemblies/Xamarin.Forms.Platform.Android.dll
  -     102,839 assemblies/formstest.Android.dll
Summary:
  +           0 Other entries 0.00% (of 887,808)
  +           0 Dalvik executables 0.00% (of 3,341,152)
  -     152,276 Assemblies -3.23% (of 4,709,077)
  +           0 Shared libraries 0.00% (of 41,693,796)
  -     151,552 Package size difference -0.73% (of 20,820,695)

Maps sample apkdiff results

Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
  -           4 lib/armeabi-v7a/libxamarin-app.so
  -          14 assemblies/Mono.Android.dll
  -       4,757 assemblies/Xamarin.Essentials.dll
  -      46,051 assemblies/Xamarin.Forms.Platform.dll
  -      47,586 assemblies/Xamarin.Forms.Platform.Android.dll
  -      48,470 assemblies/Xamarin.Forms.Maps.Android.dll
  -     135,589 assemblies/FormsMapsSample.Android.dll
Summary:
  +           0 Other entries 0.00% (of 1,137,209)
  +           0 Dalvik executables 0.00% (of 2,519,972)
  -     282,467 Assemblies -5.51% (of 5,123,833)
  -           4 Shared libraries -0.00% (of 41,843,596)
  -     282,624 Package size difference -1.34% (of 21,075,664)

@dellis1972
Copy link
Contributor Author

@radekdoulik this is a work in progress, but if you have any advice on how to do this more efficiently please feel free to comment.

@dellis1972 dellis1972 force-pushed the linkresources branch 2 times, most recently from 6f0e10d to c8cdb58 Compare December 4, 2020 12:56
@dellis1972 dellis1972 force-pushed the linkresources branch 3 times, most recently from d283fe7 to 8af10e9 Compare December 11, 2020 16:22
@dellis1972 dellis1972 force-pushed the linkresources branch 2 times, most recently from 6f0e3f9 to 6c89991 Compare January 7, 2021 13:48
@dellis1972 dellis1972 force-pushed the linkresources branch 2 times, most recently from 4599cb6 to 87cc566 Compare February 3, 2021 10:49
@dellis1972 dellis1972 marked this pull request as ready for review February 3, 2021 15:49
@dellis1972 dellis1972 changed the title [WIP] Investigate Linking of Resource.designer file from assemblies. [Xamarin.Android.Build.Tasks] Link Resource Nested Types from assemblies Feb 3, 2021
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to wait on: xamarin/Xamarin.Forms#13176

Is there a way to make a test for this?

  • Xamarin.Forms app using <Image Source="foo" /> where foo is AndroidResource
  • Launch the app, verify the image is displayed?

@dellis1972 dellis1972 force-pushed the linkresources branch 2 times, most recently from 847a9cb to 428996c Compare February 12, 2021 15:09
@jonpryor
Copy link
Member

@jonathanpeppers wrote:

Might want to wait on: xamarin/Xamarin.Forms#13176

xamarin/Xamarin.Forms#13176 has been merged. How do we know when we can rely on it?

@jonathanpeppers
Copy link
Member

xamarin/Xamarin.Forms#13176 has been merged. How do we know when we can rely on it?

@jonpryor the [Obsolete] messages say:

[Obsolete("GetDrawableByName(string) is obsolete as of version 4.8. "
			+ "Please use GetDrawableId(string, context) instead.")]

I see the change in if I download & ilspy Xamarin.Forms 5.0.0.2012 but not 4.8.0.1821.

So maybe this is only in Xamarin.Forms 5.0 and higher.

@dellis1972
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines failed to run 1 pipeline(s).

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, LGTM 👍

The idea is to remove the `Resource` class NestedTypes from the final
linked assemblies in release mode. We do this by examining all the
locations where the fields are used and replacing those IL calls with a
call which specifies the actual resource id directly. This will remove
the need to call `global::Android.Runtime.ResourceIdManager.UpdateIdValues();`
completely. It will also allow us to completely remove the `Resource`
class sub classes. Due to an odd linker failure we cannot completely
remove the `Resource` class itself. However it will be completely empty.

As mentioned the he call to `UpdateIdValues` will no  longer be required.
This should reduce startup time slightly. One thing to note is this will
only be true for Release apps. Debug apps will still call `UpdateIdValues`.
This is mostly due to the time is takes to run this new Linker Step.
Applying this to a Debug build would effect the  build times during
development.

A new MSBuild property `AndroidLinkResources` has been added to control
this feature. It is turned off by default for now.

This seems to save between 100-280kb for an apk. This is dependent on
how many assemblies are Android assemblies and contain or use Resources.

Xamarin.Forms base app apkdiff results

```
Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
  -          10 assemblies/Mono.Android.dll
  -       4,001 assemblies/Xamarin.Essentials.dll
  -      45,426 assemblies/Xamarin.Forms.Platform.Android.dll
  -     102,839 assemblies/formstest.Android.dll
Summary:
  +           0 Other entries 0.00% (of 887,808)
  +           0 Dalvik executables 0.00% (of 3,341,152)
  -     152,276 Assemblies -3.23% (of 4,709,077)
  +           0 Shared libraries 0.00% (of 41,693,796)
  -     151,552 Package size difference -0.73% (of 20,820,695)
```

Maps sample apkdiff results

```
Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
  -           4 lib/armeabi-v7a/libxamarin-app.so
  -          14 assemblies/Mono.Android.dll
  -       4,757 assemblies/Xamarin.Essentials.dll
  -      46,051 assemblies/Xamarin.Forms.Platform.dll
  -      47,586 assemblies/Xamarin.Forms.Platform.Android.dll
  -      48,470 assemblies/Xamarin.Forms.Maps.Android.dll
  -     135,589 assemblies/FormsMapsSample.Android.dll
Summary:
  +           0 Other entries 0.00% (of 1,137,209)
  +           0 Dalvik executables 0.00% (of 2,519,972)
  -     282,467 Assemblies -5.51% (of 5,123,833)
  -           4 Shared libraries -0.00% (of 41,843,596)
  -     282,624 Package size difference -1.34% (of 21,075,664)
```
@jonpryor
Copy link
Member

jonpryor commented Mar 2, 2021

Squash-and-merge:

Summary:

[Xamarin.Android.Build.Tasks] Add $(AndroidLinkResources) (#5317)

commit message:

The generated `Resource` class in `Resource.designer.cs` can be quite
large.

When doing "Release" builds, update the linker to *remove* the
`Resource` class and its nested types.  We do this by examining all
the locations where the fields are used and replacing those IL calls
with a call which specifies the actual resource id value directly.

This will remove the need to call
`global::Android.Runtime.ResourceIdManager.UpdateIdValues();` from
the static constructors in the `Resource` class and its nested types,
which will improve app launch times (as `UpdateIdValues()` involves
Reflection, which is not "fast").

Due to an odd linker failure we cannot completely remove the
`Resource` class itself.  It *will* be completely empty.

"Debug" builds will *not* remove the `Resource` type, and will retain
the call to `UpdateIdValues()`, in order to ensure that app rebuild
and redeploy times are as fast as possible.

The new `$(AndroidLinkResources)` MSBuild property has been added to
control this feature.  It is disabled by default for now.

`$(AndroidLinkResources)`=True appears to save between 100-280kb for
an `.apk`.  This is dependent on how many assemblies are Android
assemblies and contain or use Resources.

Xamarin.Forms base app `apkdiff` results

	Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
	  -          10 assemblies/Mono.Android.dll
	  -       4,001 assemblies/Xamarin.Essentials.dll
	  -      45,426 assemblies/Xamarin.Forms.Platform.Android.dll
	  -     102,839 assemblies/formstest.Android.dll
	Summary:
	  +           0 Other entries 0.00% (of 887,808)
	  +           0 Dalvik executables 0.00% (of 3,341,152)
	  -     152,276 Assemblies -3.23% (of 4,709,077)
	  +           0 Shared libraries 0.00% (of 41,693,796)
	  -     151,552 Package size difference -0.73% (of 20,820,695)

Maps sample `apkdiff` results

	Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
	  -           4 lib/armeabi-v7a/libxamarin-app.so
	  -          14 assemblies/Mono.Android.dll
	  -       4,757 assemblies/Xamarin.Essentials.dll
	  -      46,051 assemblies/Xamarin.Forms.Platform.dll
	  -      47,586 assemblies/Xamarin.Forms.Platform.Android.dll
	  -      48,470 assemblies/Xamarin.Forms.Maps.Android.dll
	  -     135,589 assemblies/FormsMapsSample.Android.dll
	Summary:
	  +           0 Other entries 0.00% (of 1,137,209)
	  +           0 Dalvik executables 0.00% (of 2,519,972)
	  -     282,467 Assemblies -5.51% (of 5,123,833)
	  -           4 Shared libraries -0.00% (of 41,843,596)
	  -     282,624 Package size difference -1.34% (of 21,075,664)

@jonpryor jonpryor merged commit 9e6ce03 into dotnet:master Mar 2, 2021
@dellis1972 dellis1972 deleted the linkresources branch March 2, 2021 16:16
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 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.

4 participants