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

Mam remapping opt #7059

Merged
merged 2 commits into from
Jun 7, 2022
Merged

Mam remapping opt #7059

merged 2 commits into from
Jun 7, 2022

Conversation

grendello
Copy link
Contributor

Fixes: #7020

Optimize MAM remapping by generating native data at build time,
to be included in libxamarin-app.so and eliminating the need
to parse XML and use managed dictionaries at run time when
remapping types/methods.

@grendello grendello force-pushed the mam-remapping-opt branch 2 times, most recently from b541ca5 to 6d67118 Compare June 2, 2022 15:04
src/Mono.Android/Android.Runtime/AndroidRuntime.cs Outdated Show resolved Hide resolved
Comment on lines 119 to 86
"PackageSize": 3492724
"PackageSize": 3012575
Copy link
Member

Choose a reason for hiding this comment

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

Yay!

Co-authored-by: Jonathan Peppers <jonathan.peppers@gmail.com>
/>
</Target>

<Target Name="_GenerateAndroidRemapNativeCode"
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we expecting these targets to run on every build? i.e no support for skipping if nothing changes between builds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We require the sources they generate for all builds, even if the remapping data isn't there (libxamarin-app.so ABI requirements). However, if they already generated the LLVM IR code and there are no changes, then they don't need to regenerate the sources.

@jonpryor
Copy link
Member

jonpryor commented Jun 7, 2022

[Xamarin.Android.Build.Tasks, monodroid] Optimize member remapping (#7059)

Fixes: https://github.com/xamarin/xamarin-android/issues/7020

Context: f6f11a5a797cd8602854942dccb0f2b1db57c473

Commit f6f11a5a introduced type & member remapping.  The problem with
its approach was that it used XML as the representation format, which
needed to be parsed during process startup.  This would contribute to
app startup slowdowns if there were any remappings, but even if there
weren't any remappings, the minimum App size increased by ~490KB, due
to the added dependencies on `System.Private.Xml.dll` & more.

This app size increase is not ideal.

Remove most of the `.apk` size increases by moving the remapping info
into `libxamarin-app.so`.  If no remappings are used, then the size
increase to `libxamarin-app.so` is negligible, and the `.apk` size is
only 61KB larger than pre-f6f11a5a.


~~ API Changes ~~

`Android.Runtime.AndroidTypeManager` gains the following members:

	partial class AndroidTypeManager {
	    struct JniRemappingReplacementMethod {
	        public string target_type, target_name;
	        public bool   is_static;
	    }
	    static extern byte* _monodroid_lookup_replacement_type (
	            string jniSimpleReference
	    );
	    static extern JniRemappingReplacementMethod* _monodroid_lookup_replacement_method_info (
	            string jniSourceType,
	            string jniMethodName,
	            string jniMethodSignature
	    );
	}

`AndroidTypeManager._monodroid_lookup_replacement_type()` replaces
the `JNIEnv.ReplacementTypes` dictionary from f6f11a5a.

`AndroidTypeManager._monodroid_lookup_replacement_method_info()`
replaces the `JNIEnv.ReplacementMethods` dictionary from f6f11a5a.

Both `_monodroid_lookup_replacement_type()` and
`_monodroid_lookup_replacement_method_info()` are P/Invokes into
`libxamarin-app.so`.


~~ `libxamarin-app.so` Changes ~~

The contents of the `@(_AndroidRemapMembers)` item group are now
stored within `libxamarin-app.so`, with the following structure:

	const uint32_t                          jni_remapping_replacement_method_index_entry_count;
	const JniRemappingIndexTypeEntry        jni_remapping_method_replacement_index[];

	const uint32_t                          jni_remapping_replacement_type_count;
	const JniRemappingTypeReplacementEntry  jni_remapping_type_replacements[];

	struct JniRemappingString {
	    const uint32_t                  length;
	    const char                     *str;
	};
	struct JniRemappingReplacementMethod {
	    const char                     *target_type, *target_name;
	    const bool                      is_static;
	};
	struct JniRemappingIndexMethodEntry {
	    JniRemappingString              name, signature;
	    JniRemappingReplacementMethod   replacement
	};
	struct struct JniRemappingIndexTypeEntry {
	    JniRemappingString              name;
	    uint32_t                        method_count;
	    JniRemappingIndexMethodEntry   *methods;
	};
	struct JniRemappingTypeReplacementEntry {
	    JniRemappingString              name;
	    const char                     *replacement;
	};
	const char *
	_monodroid_lookup_replacement_type (const char *);

	const JniRemappingReplacementMethod*
	_monodroid_lookup_replacement_method_info (const char *jniSourceType, const char *jniMethodName, const char *jniMethodSignature);

Referring to the `<replacements/>` XML from f6f11a5a in
`@(_AndroidRemapMembers)`:

  * `//replace-type/@from` fills `JniRemappingTypeReplacementEntry::name`,
    `//replace-type/@to` fills `JniRemappingTypeReplacementEntry::replacement`,
    and `_monodroid_lookup_replacement_type()` performs a linear
    search over `jni_remapping_type_replacements`.

  * `//replace-method/@source-type` fills `JniRemappingIndexTypeEntry::name`,
    `//replace-method/@source-method-name` fills `JniRemappingIndexMethodEntry::name`,
    `//replace-method/@source-method-signature` fills `JniRemappingIndexMethodEntry::signature`,
    `//replace-method/@target-type` fills `JniRemappingReplacementMethod::target_type`,
    `//replace-method/@target-method-name` fills `JniRemappingReplacementMethod::target_name`,
    `//replace-method/@target-method-signature` and
    `//replace-method/@target-method-parameter-count` are *ignored*,
    `//replace-method/@target-method-instance-to-static` fills `JniRemappingReplacementMethod::is_static`,
    and `_monodroid_lookup_replacement_method_info()` performs a
    search over `jni_remapping_method_replacement_index` looking for
    entries with "matching" type names, method names, and method
    signatures, and once a match is found it returns a pointer to the
    `JniRemappingReplacementMethod` instance.

Co-authored-by: Jonathan Peppers <jonathan.peppers@gmail.com>

@jonpryor jonpryor merged commit f99fc81 into dotnet:main Jun 7, 2022
@grendello grendello deleted the mam-remapping-opt branch June 7, 2022 17:13
@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.

Optimize type & member remapping storage, lookup
4 participants