Skip to content

Commit

Permalink
[monodroid] Speed up P/Invoke override lookups (#6210)
Browse files Browse the repository at this point in the history
Commit 0cd890b added use of [robin_map][0] to cache P/Invokes lookups.
This incurred a startup time penalty, as the map must be instantiated
and populated with `std::string` instances.

Continue using `robin_map` as a fallback cache, and introduce a
"primary" P/Invoke lookup data structure, an array of structures
describing each P/Invoke entry:

	struct PinvokeEntry {
	    hash_t     hash;
	    const char *name;
	    void       *func;
	};
	static PinvokeEntry internal_pinvokes[] = {
	    // …
	};
	static PinvokeEntry dotnet_pinvokes[] = …

`PinvokeEntry::hash` is a hash of `PinvokeEntry::name`, computed via
a modified [xxHash algorithm][1], in `src/monodroid/jni/xxhash.hh`.

The `internal_pinvokes` & `dotnet_pinvokes` arrays are sorted on
`PinvokeEntry::hash`, allowing a binary search to be performed after
hashing the function name to find.

The `internal_pinvokes` array contains P/Invokes for the
`xa-internal-api` and `java-interop` library names, and are fully
configured at build-time, as we can directly reference the target
functions:

	PinvokeEntry internal_pinvokes[] = {
	    {0x1e035ea, "java_interop_jnienv_get_string_chars", reinterpret_cast<void*>(&java_interop_jnienv_get_string_chars)},
	    // …
	};

The `dotnet_pinvokes` array *doesn't* mention the function targets:

	static PinvokeEntry dotnet_pinvokes[] = {
	    {0xaf6b1c, "AndroidCryptoNative_RsaPrivateDecrypt", nullptr},
	    // …
	};

`PinvokeEntry::func` will be computed on first request for a given
P/Invoke, when we load the relevant library and look up the symbol,
storing the pointer in the array afterwards.

Both the internal and dotnet library names are matched without using
string comparison.  Instead, a hash of the library name passed from
Mono is generated and compared against pre-generated hashes to
determine which table is to be used for lookups.

Both the tables and library name hashes are generated by the new
`generate-pinvoke-tables` C++ app, which also serves as a build-time
test of the xxHash implementation (it includes a handful of simple
compile-time tests).  Alas, currently this program can be built only
on the Linux CI bots, since neither Windows nor macOS machines
support enough of C++20.  For this reason, the generated file is
committed to the repository instead of generating it each time
`src/monodroid` is built.  When building on CI Linux machines, we do
compile the utility, generate the tables, and compare the generated
file to the one committed to repository.  If the files differ, the
build will error out and let us examine the differences (both the new
generated file and the diff are part of the build status archive).

More work needs to be done in order to compare the public interfaces
found in the dotnet shared libraries against the table stored in the
generator.

The generated tables allow us to not use the "slow" `robin_map<>` hash
path during application startup (MAUI apps use the .NET 6+ P/Invokes
during that phase).  Additionally, the `robin_map<>` cache now uses
the xxHash hashes as well for faster lookups.

In addition, both the embedded assembly mmap code as well as the
.NET 6+ P/Invoke entry code now use the GCC/clang `__atomic` APIs to
atomically set pointers, which avoids having to use any form of locks.

A new lock guard class, `StartupAwareLock`, is used to take a lock
only after the startup phase of Xamarin.Android is complete and it's
possible that multiple threads may be calling the runtime routines.

All of the above changes improve application startup time on a
Pixel 3 XL device by:

  * Plain XA sample: ~17ms faster
  * Hello MAUI sample: ~100ms faster

[0]: https://github.com/Tessil/robin-map
[1]: https://github.com/ekpyron/xxhashct
  • Loading branch information
grendello committed Sep 1, 2021
1 parent e9a4c9f commit 38aa561
Show file tree
Hide file tree
Showing 17 changed files with 2,879 additions and 455 deletions.
Expand Up @@ -30,6 +30,8 @@
<_BuildStatusFiles Include="$(XamarinAndroidSourcePath)**\.ninja_log" />
<_BuildStatusFiles Include="$(XamarinAndroidSourcePath)**\android-*.config.cache" />
<_BuildStatusFiles Include="$(XamarinAndroidSourcePath)\bin\Build$(Configuration)\clang-tidy*.log" />
<_BuildStatusFiles Include="$(XamarinAndroidSourcePath)\src\monodroid\jni\*.include.generated" />
<_BuildStatusFiles Include="$(XamarinAndroidSourcePath)\src\monodroid\jni\*.include.diff" />
</ItemGroup>
<ItemGroup>
<_TestResultFiles Include="$(XamarinAndroidSourcePath)TestResult-*.xml" />
Expand Down
94 changes: 94 additions & 0 deletions build-tools/scripts/generate-pinvoke-tables.sh
@@ -0,0 +1,94 @@
#!/bin/bash -e
MY_DIR="$(dirname $0)"
HOST="$(uname | tr A-Z a-z)"

MONODROID_SOURCE_DIR="${MY_DIR}/../../src/monodroid/jni"
GENERATOR_SOURCE="${MONODROID_SOURCE_DIR}/generate-pinvoke-tables.cc"
GENERATOR_BINARY="${MONODROID_SOURCE_DIR}/generate-pinvoke-tables"
TARGET_FILE="${MONODROID_SOURCE_DIR}/pinvoke-tables.include"
GENERATED_FILE="${TARGET_FILE}.generated"
DIFF_FILE="${TARGET_FILE}.diff"

function die()
{
echo "$@"
exit 1
}

function usage()
{
cat <<EOF
Usage: ${MY_NAME} [OPTIONS]
where OPTIONS are one or more of:
-c|--ci indicates that the script runs on one of the Xamarin.Android CI build
servers. This affects selection of the compiler
-t|--test-only indicate that the script should not replace the target file but merely
test whether the file is different to the newly generated one
-h|--help show this help screen
EOF

exit 0
}

RUNNING_ON_CI="no"
TEST_ONLY="no"

while (( "$#" )); do
case "$1" in
-c|--ci) RUNNING_ON_CI="yes"; shift ;;
-t|--test-only) TEST_ONLY="yes"; shift ;;
-h|--help) usage ;;
*) shift ;;
esac
done

case ${HOST} in
linux)
if [ "${RUNNING_ON_CI}" == "no" ]; then
COMPILER="g++"
else
COMPILER="g++-10"
fi ;;

darwin)
if [ "${RUNNING_ON_CI}" == "no" ]; then
COMPILER="clang++"
else
COMPILER="g++-11"
fi ;;

*) die Unsupported OS ;;
esac

${COMPILER} -O2 -std=c++20 "${GENERATOR_SOURCE}" -o "${GENERATOR_BINARY}"
"${GENERATOR_BINARY}" "${GENERATED_FILE}"

FILES_DIFFER="no"
cmp "${GENERATED_FILE}" "${TARGET_FILE}" > /dev/null 2>&1 || FILES_DIFFER="yes"

RETVAL=0
if [ "${TEST_ONLY}" == "no" ]; then
if [ "${FILES_DIFFER}" == "yes" ]; then
mv "${GENERATED_FILE}" "${TARGET_FILE}"
else
rm "${GENERATED_FILE}"
fi
else
if [ "${FILES_DIFFER}" == "yes" ]; then
echo "Generated p/invokes table file differs from the current one"
diff -U3 -Narp "${TARGET_FILE}" "${GENERATED_FILE}" > "${DIFF_FILE}"

echo "Diff file saved in: ${DIFF_FILE}"
echo "------ DIFF START ------"
cat "${DIFF_FILE}"
echo "------ DIFF END ------"
echo
RETVAL=1
else
echo Generated file is identical to the current one
fi
fi

exit ${RETVAL}
Expand Up @@ -2,34 +2,34 @@
"Comment": null,
"Entries": {
"AndroidManifest.xml": {
"Size": 2584
"Size": 2604
},
"assemblies/Java.Interop.dll": {
"Size": 57802
"Size": 57924
},
"assemblies/Mono.Android.dll": {
"Size": 83219
"Size": 83268
},
"assemblies/rc.bin": {
"Size": 946
},
"assemblies/System.Linq.dll": {
"Size": 11292
"Size": 11294
},
"assemblies/System.Private.CoreLib.dll": {
"Size": 533257
"Size": 534326
},
"assemblies/System.Runtime.dll": {
"Size": 2832
},
"assemblies/UnnamedProject.dll": {
"Size": 3535
"Size": 3539
},
"classes.dex": {
"Size": 345240
},
"lib/arm64-v8a/libmonodroid.so": {
"Size": 338784
"Size": 369312
},
"lib/arm64-v8a/libmonosgen-2.0.so": {
"Size": 3163680
Expand All @@ -44,7 +44,7 @@
"Size": 150024
},
"lib/arm64-v8a/libxamarin-app.so": {
"Size": 11504
"Size": 11824
},
"META-INF/ANDROIDD.RSA": {
"Size": 1213
Expand Down Expand Up @@ -77,5 +77,5 @@
"Size": 1724
}
},
"PackageSize": 2697043
"PackageSize": 2709331
}

0 comments on commit 38aa561

Please sign in to comment.