[CoreCLR] Remove robin-map from p/invoke override - #12010
Merged
jonathanpeppers merged 6 commits intoJul 13, 2026
Merged
Conversation
Remove the CoreCLR p/invoke override dependency on robin-map by deleting the custom cache layer and generated precompiled p/invoke table. The precompiled CoreCLR path now resolves only the small internal symbol set explicitly, lets CoreCLR handle BCL native libraries, and falls back directly through the dotnet/android loader for other libraries. NativeAOT remains free of robin-map references, and MonoVM keeps its existing robin-map usage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
/review |
Contributor
|
✅ Android PR Reviewer completed successfully! |
Refresh BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc for the intended CoreCLR native size reduction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Separate the precompiled CoreCLR p/invoke override resolver by logical library: java-interop, xa-internal-api, and liblog. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🤖 Android PR Review
Independent pass over the native CoreCLR p/invoke-override changes — I verified the mechanics before reading the description. The core rewrite is correct.
Verified ✅
- Symbol completeness —
load_internal_symbolcovers all 28 entries from the removed generatedinternal_pinvokestable and matches theextern "C"set ininternal-pinvokes.hh; every string literal matches its identifier, nothing dropped. - Library-name matching — the new
== "java-interop" / "xa-internal-api" / "liblog" / "libSystem.*"comparisons are exactly the strings the deleted generator hashed into the old*_hashconstants. Behavior-equivalent. handle_jni_on_loadremoval — dead code on the CLR path (no callers;Java_JNI_OnLoadinclr/host/host.ccnever used it). NativeAOT keeps its own live copy. Clean.- robin-map removal — every remaining
ROBIN_MAP_DIR/tsl::reference is confined tosrc/native/mono/under the newif(IS_MONO_RUNTIME)guard; no dangling references anywhere insrc/native/clr/. - Cache removal — relying on CoreCLR's per-method p/invoke caching is sound, and build-time-known DSOs are still handle-cached inside
monodroid_dlopen, so droppingother_pinvoke_mapcauses no meaningfuldlopenrefcount regression. - Tabs/formatting, added includes, and inline macros all look correct.
Non-blocking suggestions 💡 (inline)
load_internal_symbolreintroduces a hand-maintained table with the same drift →abort_applicationfailure mode this PR removes for the BCL libs (#11530). Worth astatic_assert/cross-reference guard.- Test coverage gap — the only test targets
libmonosgen-2.0(MonoVM-only), so the rewritten CoreCLR fallback path has no on-device coverage.
CI
Checks are still pending on 00428d9 (draft PR) — I could not confirm green. Please make sure the CoreCLR and NativeAOT native builds plus the on-device p/invoke tests pass before merging.
Verdict: No blocking code issues found — 2 optional suggestions above. Nice cleanup: −633/+143 and one fewer external dependency on the CoreCLR path. 👍 (Confirm CI is green before un-drafting.)
Generated by Android PR Reviewer for #12010 · 383.9 AIC · ⌖ 27.3 AIC · ⊞ 6.8K
Comment /review to run again
simonrozsival
marked this pull request as ready for review
July 7, 2026 22:58
simonrozsival
added
the
ready-to-review
Move the internal p/invoke missing-symbol abort into the library-specific resolvers so the dispatcher can return directly for each internal library. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR simplifies the CoreCLR native p/invoke override by removing the robin-map dependency and the custom p/invoke cache/table, relying instead on explicit handling for a small set of internal symbols plus the existing dotnet/android loader fallback for other libraries.
Changes:
- Remove CoreCLR’s robin-map-based p/invoke cache and associated include/compile plumbing.
- Replace the CoreCLR precompiled override’s generated internal table with explicit per-library symbol resolution, and route “other” libraries through
MonodroidDl::monodroid_dlopen/monodroid_dlsym. - Update size reference (
*.apkdesc) and adjust test commentary accordingly.
Show a summary per file
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Mono.Android-Tests/Android.Runtime/PInvokeOverrideTests.cs | Updates test commentary to reflect new fallback resolution (no robin-map cache). |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc | Updates expected size baselines for CoreCLR build artifacts. |
| src/native/CMakeLists.txt | Makes ROBIN_MAP_DIR conditional on Mono runtime builds only. |
| src/native/clr/pinvoke-override/precompiled.cc | Reworks precompiled CoreCLR override to explicit symbol handling + loader fallback; removes generated include usage. |
| src/native/clr/pinvoke-override/pinvoke-tables.include | Deletes the generated CoreCLR internal-pinvoke table include. |
| src/native/clr/pinvoke-override/generate-pinvoke-tables.cc | Removes the CoreCLR pinvoke table generator source. |
| src/native/clr/pinvoke-override/dynamic.cc | Simplifies dynamic override path and routes unknown libraries via load_library_symbol. |
| src/native/clr/pinvoke-override/CMakeLists.txt | Removes robin-map include paths and compile definitions. |
| src/native/clr/include/host/pinvoke-override.hh | Removes robin-map types/caching and narrows the public API to load_library_symbol + override entrypoint. |
| src/native/clr/include/host/pinvoke-override-impl.hh | Removes robin-map caching logic; keeps direct dlopen/dlsym-based resolution via MonodroidDl. |
| src/native/clr/host/CMakeLists.txt | Drops robin-map include directories from CoreCLR host build. |
| .gitignore | Removes ignore entries for deleted CoreCLR pinvoke generator artifacts. |
Copilot's findings
- Files reviewed: 11/12 changed files
- Comments generated: 1
simonrozsival
added
ready-to-review
…map-from-coreclr # Conflicts: # src/native/clr/pinvoke-override/dynamic.cc # src/native/clr/pinvoke-override/precompiled.cc
Comment on lines
+20
to
+21
| // Handle p/invokes of the form [DllImport ("liblog")] or [DllImport ("log")] | ||
| // TODO: try modifying the name to contain both the `log` prefix and the `.so` suffix |
Member
There was a problem hiding this comment.
Will this actually break something?
Someone might be doing: [DllImport ("libfoo")] or [DllImport ("foo")]
Member
Author
There was a problem hiding this comment.
it does not as far as I know, but I'll confirm it.
Member
Author
There was a problem hiding this comment.
No, neither spelling breaks. load_library_symbol normalizes the name before dlopen:
[DllImport ("foo")]→ trieslibfoo.so[DllImport ("libfoo")]→ trieslibfoo.so[DllImport ("libfoo.so")]→ used as-is
…and if the normalized name fails to load, it falls back to dlopen of the original library_name. This normalize-and-fallback behavior is unchanged by this PR — the diff only removed the robin_map cache that wrapped it, not the name handling itself.
I added on-device coverage for exactly these spellings in PInvokeOverrideTests: a new JNI-free rt_get_pinvoke_test_value export in libreuse-threads.so (bundled as an <AndroidNativeLibrary/>, so present under both MonoVM and CoreCLR) is invoked via reuse-threads, libreuse-threads, and libreuse-threads.so, asserting all three resolve to the same function.
Member
Author
There was a problem hiding this comment.
I think we can drop or improve the TODO comment in a follow-up PR
jonathanpeppers
approved these changes
Jul 13, 2026
simonrozsival
added a commit
that referenced
this pull request
Jul 15, 2026
## Goal Remove the CoreCLR native dependency on xxHash/constexpr-xxh3 in the .NET for Android lookup paths while preserving runtime behavior and startup performance. The primary improvement is dependency removal; app size and startup time are unchanged within measurement noise. ## Summary - Replace CoreCLR LLVM typemap xxHash lookups with 32-bit CRC-32 (`System.IO.Hashing.Crc32` in the managed generators, matched by a small `runtime-base/crc32.hh` in the native runtime). - Replace CoreCLR assembly-store name hashes with 32-bit CRC-32 and bump the CoreCLR store format to v4 (MonoVM stays on v3/xxHash). - Replace CoreCLR DSO APK/cache hashes with 32-bit CRC-32. - Replace the CoreCLR preserved-p/invoke (`find_pinvoke`) generator hashes with 32-bit CRC-32. - Keep MonoVM assembly stores and runtime lookup data on xxHash; move the xxHash-specific `Search` helpers to MonoVM so CoreCLR no longer pulls in `shared/xxhash.hh` through the common search header. - Remove `external/xxHash` and `constexpr-xxh3` include paths from the **CoreCLR** native CMake targets (both remain for MonoVM/NativeAOT, which still use xxHash). - Remove CoreCLR-only dead paths exposed by the cleanup: AOT DSO cache, debug typemap string-fallback switch, duplicate runtime-property hash index, and the runtime-property host callback. - Update generated-layout test parsing and the mk2 assembly-store reader for the new CoreCLR layouts. ### Collision safety CRC-32 is 32-bit, so collisions between two names in the same app — while extremely unlikely — are possible where 64-bit xxHash effectively never collided. Every CoreCLR lookup that matches on these hashes now verifies the actual name after a hash match: - **Typemap** — compares the original managed/Java name (`same_string`). - **Assembly store** — `find_assembly_store_entry` walks the run of entries sharing a hash and compares the requested name against the real assembly name from the store's `ASSEMBLY_NAMES` section. - **DSO cache** — `find_dso_cache_entry` re-derives the name mutations of the matched entry's real library name (mirroring managed `AddNameMutations`) and confirms the requested name is one of them. - **DSO APK table** (`find_dso_apk_entry`) intentionally stays hash-only: `DSOApkEntry` stores no name, and it's only reached with the `real_name_hash` of an already-name-verified DSO cache entry, so the residual risk is limited to a CRC-32 collision between two shared libraries in the same APK — a deliberate 64→32-bit trade-off. The empty-input CRC-32 is a shared `UINT32_MAX` sentinel between the managed generators and the native runtime, documented in `crc32.hh`. ### Relationship to #12010 Rebased on top of #12010 (`[CoreCLR] Remove robin-map from p/invoke override`), now in `main`. #12010 removed the CoreCLR p/invoke override's hand-maintained tables and cache entirely, so the earlier CRC-32 conversion of those tables here is obsolete and was dropped in the merge. CoreCLR p/invoke resolution now falls back through `MonodroidDl::monodroid_dlopen`, which makes the DSO-cache name verification above the load-bearing path. ## Benchmark CoreCLR MAUI Release/R2R sample, Android arm64, Samsung SM-A165F, interleaved runs, no reboot. These numbers were measured against #12010 as the xxHash baseline (before it merged) to isolate the CRC-32/dependency-removal change; they predate the collision-safety additions, which add only a name comparison on hash matches. ### Size | Variant | APK size | libmonodroid.so uncompressed | | --- | ---: | ---: | | #12010 baseline / xxHash | 30,693,198 B | 1,247,192 B | | #12013 current / CRC32 | 30,709,582 B | 1,250,616 B | ### Startup, 30x mixed cold + hot, IQR outliers removed | Mode | #12010 baseline mean / median | #12013 current mean / median | Delta | | --- | ---: | ---: | ---: | | Cold WaitTime | 2005.0 / 1998 ms | 1998.7 / 1995 ms | -6.3 ms mean, -3 ms median | | Hot WaitTime | 148.2 / 146.5 ms | 148.1 / 148 ms | -0.1 ms mean, +1.5 ms median | Result: app size and startup times are unchanged within measurement noise. The meaningful win is removing CoreCLR's dependency on the xxHash/constexpr-xxh3 native dependency chain. ## Validation - `dotnet build` of Xamarin.Android.Build.Tasks, the mk2 assembly-store reader, native-clr (arm64), native-mono (arm64), and the build test project. - Clean CoreCLR native arm64 configure/build after removing the xxHash include paths; no `external/xxHash` or `external/constexpr-xxh3` paths remain in the CoreCLR compile output. - Full `make all CONFIGURATION=Release` after merging #12010. - Direct `CreateAssemblyStore` probe generated a CoreCLR arm64 v4 store with 9-byte index entries that the mk2 reader opened successfully. - CoreCLR MAUI benchmark APKs built and launched on Samsung SM-A165F.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes the CoreCLR native p/invoke override dependency on
external/robin-map.The motivation is primarily dependency and code simplification, not size or startup wins. After #11537, CoreCLR no longer needs a hand-maintained BCL p/invoke table or a custom p/invoke cache in the precompiled override. CoreCLR already caches resolved p/invokes at the runtime/call-site level, and arbitrary fallback libraries can be resolved directly through the existing dotnet/android loader path.
CoreCLR now:
tsl::robin_map/external/robin-mapdependencyxa-internal-apiandliblognullptrfor BCL native libraries so CoreCLR's own resolver handles themMonodroidDl::monodroid_dlopen+monodroid_dlsymforjava-interopand other librariesNativeAOT remains free of robin-map references. MonoVM keeps its existing robin-map usage.
Size impact
This is not a major app-size change. The size reduction is small but in the expected direction.
BuildReleaseArm64SimpleDotNet.CoreCLR.apkdescdotnet new maui --sample-contentRelease/CoreCLR/R2Randroid-arm64libmonodroid.so-17,400 raw / -3,584 ZIPThe committed
BuildReleaseArm64SimpleDotNet.CoreCLR.apkdescupdate reflects the expected CI size-reference change.Startup performance
No measurable startup-time difference.
Device: Samsung Galaxy A16 (
SM-A165F,arm64-v8a, API 36). Method: 30 interleaved cold-start pairs usingam start -W, comparing default Release/CoreCLR/R2Rdotnet new maui --sample-contentAPKs built frommainand this PR.mainPaired
current - mainTotalTime delta across the 30 measured pairs:Conclusion: startup is unchanged within measurement noise.
Validation
make prepare && make allon this PRmake prepare && make allonorigin/maindotnet new maui --sample-contentRelease/CoreCLR/R2R APK build on both local SDKsSM-A165F, API 36)dotnet build src/native/native-clr.csproj --nologo -v:minimal -p:AndroidSupportedTargetJitAbis=arm64-v8a -p:AndroidSupportedTargetAotAbis=arm64 -p:EnableNativeAnalyzers=falsedotnet build src/native/native-nativeaot.csproj --nologo -v:minimal -p:AndroidSupportedTargetJitAbis=arm64-v8a -p:AndroidSupportedTargetAotAbis=arm64 -p:EnableNativeAnalyzers=false