[CoreCLR] On-device decompressed AssemblyStore cache#11967
[CoreCLR] On-device decompressed AssemblyStore cache#11967simonrozsival wants to merge 8 commits into
Conversation
9d831a5 to
f1de798
Compare
simonrozsival
left a comment
There was a problem hiding this comment.
Strong foundation: the cache is lazy and per-assembly, cache misses fall back cleanly, MAP_PRIVATE preserves the runtime’s writable-image requirement, and the private snapshot fixes the writer/runtime data race. The A/B switch and benchmark write-up are unusually honest about the small latency effect.
Before this could move beyond a prototype, the durability path and cache key need correction, and the first-run memory/I/O cost needs a bounded design plus measurement. CI build 1492915 is green, but there is no targeted test proving miss → persist → hit, invalidation, corruption fallback, or satellite-assembly behavior.
|
@copilot resolve the merge conflicts in this pull request |
Resolved. There was one conflict in |
## Summary The CoreCLR host used to locate the assembly store by **parsing the APK ZIP central directory**, `mmap`-ing the wrapper `.so`, and **walking its ELF section headers by hand** to find the payload. Since the store is already a valid ELF shared library (#9154), this PR instead makes its payload a **loadable** section (`SHF_ALLOC`, in a `PT_LOAD` segment) exposed via an exported **dynamic symbol**, so the host just: ```c void *h = dlopen("libassembly-store.so", RTLD_NOW | RTLD_LOCAL); void *start = dlsym(h, "_assembly_store"); // -> AssemblyStoreHeader ``` …and lets the platform dynamic linker locate + map it out of the APK. The hash-index lookup (`AssemblyStore::open_assembly`) is unchanged. **This is CoreCLR-only. MonoVM is untouched** and keeps the classic `DSOWrapperGenerator` + ZIP-scan store. The mechanism is **hardcoded by runtime and cannot be overridden** (`_AndroidDlopenAssemblyStore` = true for CoreCLR, false for MonoVM), because after this change the CoreCLR host has no ZIP fallback and MonoVM's host doesn't understand the loadable-symbol layout. ## What's removed (CoreCLR) - `src/native/clr/startup/zip.cc` **in its entirety** (the ZIP central-directory parser) + the `xamarin-startup` static library. - `Host::zip_scan_callback`, `AssemblyStore::map(fd/offset)`, and `Util::get_wrapper_dso_payload_pointer_and_size` usage (the ELF section-header walk). - The `DSOApkEntry {fd,offset}` native-library fast path in `MonodroidDl` (it was populated by the same ZIP scan). Bundled native libraries now load via `dlopen`-by-name, which works because they're uncompressed + page-aligned in `lib/{ABI}/` (`extractNativeLibs=false`). The now-dead `DSOApkEntry` struct, its `dso_apk_entries[]` emission in `ApplicationConfigNativeAssemblyGeneratorCLR`, and the stub definition are removed too (MonoVM keeps its own copy). Net: **889 lines deleted, 197 added (−692 net)**. The arm64 CoreCLR host (`libnet-android.release.so`) shrinks **34,784 B (−2.73%)** in a clean same-tree isolation (referenced code removed; arm −3.05%, x64 −3.34%); the actual shipped `main`-vs-PR APK host delta is **−28,640 B** (see the Size table below). ## Why this hasn't been done before (history) `dlopen`-based loading was never used, and this is **not** a regression from a nicer past design — ZIP-scan + `mmap` is the original and only design, three layers accreted over the years: | When | PR | What changed | Loading | |------|----|--------------|---------| | 2019 | #2570 | assemblies `mmap`-ed in place from the APK (never extracted) | ZIP-scan + `mmap` | | 2021 | #6311 | assembly store introduced (`assemblies.blob`, O(n)→O(1)) | raw blob, ZIP-scan + `mmap` | | 2024 | #8478 | all assemblies RID-specific; store moved to `lib/{ABI}/`, renamed `lib*.so` (not real ELF) | ZIP-scan + `mmap` | | 2024 | #9154 | wrapped in a **real ELF** (`objcopy --add-section`, non-loadable, 16 KB-aligned) for the Android 16 KB page-size requirement | ZIP-scan + `mmap` + **ELF section-header walk** | | 2025 | #9778, #9572 | CoreCLR host re-implements the same approach | same | `dlopen` only became **possible** in #9154 (the store became real ELF), and even then the payload was deliberately kept **non-loadable** so the existing `mmap` loader could be reused — on the *unverified* assumption (see [`ApkSharedLibraries.md`](https://github.com/dotnet/android/blob/main/Documentation/project-docs/ApkSharedLibraries.md): *"…without having to load the ELF image into memory"*) that `mmap`-the-section beats loading the ELF. This PR measures that assumption. ## Benchmark results Measured on a physical **Samsung A16 (SM-A165F)**, `net11.0-android` **MAUI sample-content** app (`dotnet new maui --sample-content`), Release, arm64, CoreCLR, `extractNativeLibs=false`, 30 **interleaved** cold-starts (`am start -W` TotalTime). ### Apples-to-apples (both built from the same local P7 SDK — isolates the store mechanism) Three independent 30-interleaved rounds, `main` (classic ZIP-scan) vs this PR (dlopen). Rounds 1–2 reused a fixed dlopen APK; round 3 rebuilt `latest` **fresh from the committed code** (verified store exports `_assembly_store`, section `payload` is loadable, host has no `DT_NEEDED`): | Round | main mean | dlopen mean | Δ (dlopen − main) | paired test | |---|---|---|---|---| | 1 | 2059.7 ms | 2076.1 ms | **+16.4 ms (+0.80%)** | t=2.31, p≈0.03, CI [+1.9, +31.0] | | 2 | 2061.1 ms | 2054.4 ms | **−6.6 ms (−0.32%)** | t=−1.00, p≈0.32, CI [−19.7, +6.4] | | 3 (fresh) | 2056.0 ms | 2032.3 ms | **−23.6 ms (−1.15%)** | t=−3.95, p≈0.0004, CI [−35.4, −11.9] | The three rounds span **+16 → −24 ms** (a ~40 ms between-session spread that exceeds any single round's effect) ⇒ the cross-session noise floor dominates. **dlopen is startup-neutral — within noise, if anything trending faster, and never a regression.** ### Size (arm64, `extractNativeLibs=false`, all `Stored`/uncompressed in the APK; round-3 fresh build) | Component | main | dlopen | Δ | |---|---|---|---| | APK total | 29,670,166 | 29,637,398 | **−32,768 B** | | `libmonodroid.so` (host) | 1,264,464 | 1,235,824 | **−28,640 B (−2.27%)** | | `libassembly-store.so` | 7,674,048 | 7,674,784 | +736 B (negligible) | (The host shrinks because the ZIP parser + ELF section-header walk + `DSOApkEntry` path are gone. Clean referenced-code isolation on the same tree: arm64 −34,784 B, arm −3.05%, x64 −3.34%.) ### Is there a real difference? No — it's noise The `dlopen`+`dlsym` call itself was measured on-device against the **real 7.6 MB store**: ``` dlopen = 0.14–0.49 ms dlsym = ~0.006 ms (reads magic 0x41424158 "XABA") ``` That's **~0.02% of a ~2050 ms cold start** — far below the ±20–40 ms cross-session variance. Deleting the ZIP scan + ELF walk is provably not a startup regression. >⚠️ An earlier naive comparison (stock **global .NET 11 preview 5** vs this local **preview 7** build) showed +72 ms — but that was **confounded** by the P5→P7 delta and official-vs-local-build differences, not the dlopen change. Building `main` with the *same* local P7 toolchain isolates it to the ~0 ms above. (The residual local-P7-vs-global-P5 gap is likely partly #11730, the LZ4→Zstd store-compression switch merged after P5.) ## Can we optimize the dlopen further? No — there's nothing to skip The store `.so` is inspected with `readelf`: **0 relocations, no `DT_NEEDED`, no `DT_INIT`/init-array**, two `PT_LOAD` segments. The linker therefore does **no** relocation, dependency resolution, or constructor work — it just `mmap`s the (demand-paged) segments + minor bookkeeping. Concretely: - `android_dlopen_ext` has **no** "skip relocations/init" flag — and there's nothing to skip anyway. - `RTLD_LAZY` vs `RTLD_NOW` is moot (no code → no PLT relocations). We pass `RTLD_LOCAL` (we only `dlsym` our own handle, so there's no need for `RTLD_GLOBAL`'s extra global-scope bookkeeping), though the difference is immaterial at <0.5 ms. - The bionic `apk!/lib/{abi}/lib*.so` zip-path syntax (how `extractNativeLibs=false` is implemented) works, but measured **slower** on-device (~0.52–0.98 ms — the linker still opens the zip + does a CD lookup for the entry) and would re-introduce runtime APK-path / split-config handling. The by-soname `dlopen` is simpler and already at the floor. ## Follow-ups (out of scope) - Pairs naturally with #11967 (on-device decompressed store cache): the decompressed cache can itself be the dlopen-able wrapped `.so`, so warm starts skip both Zstd decompression and any ZIP work; `android_dlopen_ext(USE_LIBRARY_FD[_OFFSET])` is the right tool to load it by fd. - The now-orphaned `Util::get_wrapper_dso_payload_pointer_and_size` definition can be deleted as pure dead-code cleanup. - The runtime-config blob uses the same wrapper mechanism and could adopt the same symbol-based loading. ## Validation - CoreCLR app built with **no flag** auto-produces a store exporting `_assembly_store` (hardcode works) and cold-launches cleanly (Status: ok). - Store is `Stored` (uncompressed) + 16 KB-aligned; `dlsym` returns a pointer to a valid `AssemblyStoreHeader` (magic `XABA`). - The wrapper's payload lives in a section named `payload` (matching the MonoVM `DSOWrapperGenerator` and `tools/assembly-store-reader-mk2`), so the `CoreClrTrimmableTypeMap*` build tests that inspect the APK through the reader keep working. - The whole read-only store pointer chain (`configure_from_payload(const void*)`, `data_start`, `image_data`/`debug_info_data`/`config_data`, `assemblies`, `descriptor`, `assembly_store_hashes`) is `const` — no `const_cast` on the store path. ## Reference PRs - #6311, #8478, #9154, #9382, #9778, #9572 - LZ4→Zstd store compression: #11730 · on-device store cache: #11967 - [`AssemblyStores.md`](https://github.com/dotnet/android/blob/main/Documentation/project-docs/AssemblyStores.md) · [`ApkSharedLibraries.md`](https://github.com/dotnet/android/blob/main/Documentation/project-docs/ApkSharedLibraries.md) · [Android 16 KB pages](https://developer.android.com/guide/practices/page-sizes) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1ad2fd8 to
8dc4118
Compare
Prototype exploring caching decompressed assemblies on-device so that subsequent launches skip zstd decompression and load the data via a file-backed mmap instead of dirty anonymous memory. - assembly-store.cc: on a decompression cache miss, a single background thread atomically writes the decompressed bytes to <codeCacheDir>/decompressed-assembly-cache/<Assembly.dll> (temp -> fsync -> rename). On the next launch the file is mmap'd (MAP_PRIVATE, COW) and decompression is skipped. Per-assembly, only assemblies actually touched are cached. Staleness guarded by an 8-byte footer holding an xxhash of the compressed payload. - Plumb codeCacheDir (Context.getCodeCacheDir()) through Java initInternal -> appDirs[3] -> AndroidSystem, so a stale cache is auto-wiped by Android on app/platform update. - Runtime A/B toggle via `debug.net.asmcache` system property (and XA_DISABLE_ASSEMBLY_CACHE env var). Experimental only: no MSBuild opt-in, no assembly-store version stamp, CoreCLR only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The background writer thread read directly from the shared uncompressed_assemblies_data_buffer, but on a cache miss that same buffer is handed to the runtime once the decompress lock is released, and the runtime may write into the assembly image (the reason the cache-hit path maps the file MAP_PRIVATE / COW). Concurrent writes could persist a torn or post-mutation image; since the staleness footer only hashes the *compressed* payload, that corrupt image would then be reloaded from cache as if pristine on the next launch. Take a private snapshot of the decompressed bytes in enqueue_write, while the caller still holds assembly_decompress_mutex and before the buffer is exposed to the runtime, so the writer only ever touches immutable memory it owns. On allocation failure we skip caching that assembly rather than aborting. Trade-off: this adds one memcpy per newly-cached assembly on the first-launch (cache-miss) path and holds the queued snapshots (up to the touched working set) transiently until the writer drains them. Subsequent launches hit the mmap path and never enqueue. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tidy up the file-writing path without pulling <fstream>/iostreams into the runtime .so (only the build-time pinvoke-table generator uses those; the runtime deliberately sticks to raw syscalls to keep the library small and startup cheap). - Lay out the full on-disk image ([payload][8-byte token footer]) in the snapshot buffer at enqueue time, so the writer emits it in a single contiguous write. This drops the separate footer write (and with it a bug: that write didn't handle EINTR/partial writes) and lets WriteRequest lose its token field. - Extract a write_fully() helper for the EINTR/partial-write retry loop, leaving writer_loop as open -> write_fully -> fsync -> close -> rename. No behavior change: the cache file format is identical, so existing cache files remain valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move the open/write/fsync/close/rename ceremony into a dedicated write_cache_file() method so writer_loop() only owns the concurrency concerns (waiting on the queue, dequeuing under the lock) and delegates the actual persistence. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make the CoreCLR cache opt-in, content-addressed, checksum-validated, and memory-bounded. Add assembly-store v4 content IDs, reader support, documentation, and host/device regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7be7d34c-aff3-4338-b15b-6061bc7a3ba9
8dc4118 to
0c94c52
Compare
There was a problem hiding this comment.
Pull request overview
Experimental, opt-in prototype to cache decompressed AssemblyStore entries on-device for CoreCLR Release apps, aiming to reduce repeat-launch decompression work and improve memory composition by mapping persisted decompressed payloads from codeCacheDir.
Changes:
- Adds a deterministic store content ID to the AssemblyStore header and updates store tooling/docs to understand it.
- Threads a new MSBuild/app-config switch (
AndroidEnableAssemblyStoreDecompressionCache) through build outputs into the CoreCLR runtime config. - Implements the native CoreCLR-side cache (persist + validate + mmap) plus integration/unit test coverage.
Show a summary per file
| File | Description |
|---|---|
| tools/assembly-store-reader-mk2/AssemblyStore/StoreReader_V2.cs | Reads optional content_id for v4 stores; uses per-header NativeSize when seeking past the header. |
| tools/assembly-store-reader-mk2/AssemblyStore/StoreReader_V2.Classes.cs | Extends header model to include content_id and a version-dependent native size. |
| tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs | Adds device integration test validating persistence, corruption fallback, and file-backed mappings. |
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Introduces MSBuild property default and passes it into native application-config generation. |
| src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs | Writes content_id into the store header after hashing the store payload. |
| src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.Classes.cs | Updates managed header layout to include content_id. |
| src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGeneratorCLR.cs | Emits assembly_store_decompression_cache_enabled into CoreCLR application config. |
| src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigCLR.cs | Adds the new config field to the managed representation. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs | Updates CoreCLR application-config parsing/verification to include the new field. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateNativeApplicationConfigSourcesTests.cs | Adds unit test verifying the new app-config setting is emitted correctly. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/CreateAssemblyStoreTests.cs | Adds unit test asserting the store content_id matches a hash of everything after the header. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs | Plumbs the MSBuild property into the CoreCLR config generator state. |
| src/native/mono/xamarin-app-stub/xamarin-app.hh | Bumps MonoVM store format version and adds content_id to the native header struct. |
| src/native/clr/xamarin-app-stub/application_dso_stub.cc | Initializes the new config field in the CoreCLR app stub. |
| src/native/clr/include/xamarin-app.hh | Adds content_id to CoreCLR store header and adds the new config flag. |
| src/native/clr/include/runtime-base/android-system.hh | Stores and exposes the app code-cache directory path. |
| src/native/clr/include/host/assembly-store.hh | Tracks the mapped store’s content_id for cache directory naming/validation. |
| src/native/clr/include/constants.hh | Adds index constant for code-cache dir within the app-dirs array. |
| src/native/clr/host/host.cc | Captures code-cache dir during runtime init. |
| src/native/clr/host/assembly-store.cc | Implements the decompressed-assembly cache (lookup/validate/mmap + background persistence). |
| src/java-runtime/java/mono/android/clr/MonoPackageManager.java | Passes codeCacheDir into native runtime init. |
| Documentation/project-docs/AssemblyStores.md | Documents the new content_id field in store headers. |
| Documentation/building/configuration.md | Documents AndroidEnableAssemblyStoreDecompressionCache. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:425
- 🤖 ❌ error Testing — The CoreCLR
application_configparser is now out of sync with the generated structure: it adds a new field but usescase 21(skipping 20) and still assertsApplicationConfigFieldCount_CoreCLR = 20. With the newassembly_store_decompression_cache_enabledfield appended, the correct next index should be 20 and the expected field count should be updated accordingly, otherwise this parser will ignore the new field and fail the field-count assertion.
case 21: // assembly_store_decompression_cache_enabled: bool / .byte
AssertFieldType (envFile.Path, parser.SourceFilePath, ".byte", field [0], item.LineNumber);
ret.assembly_store_decompression_cache_enabled = ConvertFieldToBool ("assembly_store_decompression_cache_enabled", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
break;
}
fieldCount++;
}
Assert.AreEqual (1, pointers.Count, $"Invalid number of string pointers in 'application_config' structure in environment file '{envFile.Path}'");
NativeAssemblyParser.AssemblerSymbol androidPackageNameSymbol = GetRequiredSymbol (pointers[0], envFile, parser);
ret.android_package_name = GetStringContents (androidPackageNameSymbol, envFile, parser);
Assert.AreEqual (ApplicationConfigFieldCount_CoreCLR, fieldCount, $"Invalid 'application_config' field count in environment file '{envFile.Path}'");
- Files reviewed: 23/23 changed files
- Comments generated: 3
The stream is already flushed after WriteNames, and the intervening position check writes nothing, so the second Flush() before ComputeContentId was a no-op left over from the rebase merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 31cb6076-baba-4b69-a86f-114e108fe8c4
- Bump MonoVM assembly store format version 3 -> 4 to match the native ASSEMBLY_STORE_FORMAT_VERSION constant and the new on-disk header layout (the header now unconditionally includes content_id). - Use a per-process unique temp file name (`.tmp.<pid>`) in the decompressed assembly cache writer so concurrent processes can't clobber a shared temp file before rename(); drop the now-obsolete ENOENT/Skipped shared-temp path. - Filter `find` output down to actual `.bin` paths in the device test so merged stderr lines can't skew the cache-file count/selection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 78d2d813-1f6e-435d-88cb-71bfb71e2c35
Adding the `assembly_store_decompression_cache_enabled` field grew the CoreCLR application_config from 20 to 21 fields, but the test helper was not fully updated, failing 16 CoreCLR build tests with "Invalid 'application_config' field count Expected: 20 But was: 21". - Bump ApplicationConfigFieldCount_CoreCLR 20 -> 21. - Fix the parser case for the new field: the switch is on the 0-indexed fieldCount (have_assembly_store is case 19), so the new field is case 20, not case 21 (which never matched and silently skipped validation). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 78d2d813-1f6e-435d-88cb-71bfb71e2c35
Warning
Experimental, opt-in prototype — not ready to merge. The cache is CoreCLR-only and defaults off. Its memory, storage, first-launch I/O, and energy trade-offs still need broader measurement.
Builds on the Zstd AssemblyStore compression introduced by #11730. This prototype caches decompressed assemblies on-device so subsequent launches can skip Zstd decompression and use clean, file-backed mappings instead of dirty anonymous memory.
Design
AndroidEnableAssemblyStoreDecompressionCache=truein a CoreCLR Release build. The default isfalse.<codeCacheDir>/decompressed-assembly-cache-v1/<store-content-id>/<descriptor-index>.bin.fr/App.resources.dll.mmap(PROT_READ | PROT_WRITE, MAP_PRIVATE)so runtime writes become COW while untouched pages remain clean and file-backed.<descriptor-index>.bin.tmp.<pid>) followed by atomicrename, so concurrent processes cannot clobber a shared temp file. The cache is intentionally disposable: it does notfsync; incomplete or power-loss-damaged entries fail checksum validation and fall back to decompression.debug.net.asmcache=0|1remains available as an internal A/B override;XA_DISABLE_ASSEMBLY_CACHEforces it off.Android deletes
codeCacheDircontents on app and platform updates. The store content ID, cache format version, exact-size checks, and payload checksum provide additional invalidation and corruption protection.Coverage
Performance status
The earlier prototype measured a small warm-launch improvement on a Samsung Galaxy A16:
Those numbers predate the payload-integrity scan, bounded writer, and new cache format, so they must be re-measured. The expected stronger justification remains memory composition rather than latency: dirty/private PSS should move toward clean file-backed pages, but that has not yet been quantified.
Before considering a default-on product feature, measure: