From 4bcb28c0cbca22b0a06cc22f206836d640187dad Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 2 Jul 2026 13:10:08 +0200 Subject: [PATCH 1/9] [experiment] On-device decompressed assembly store cache (CoreCLR) 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 /decompressed-assembly-cache/ (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> --- .../mono/android/clr/MonoPackageManager.java | 3 +- src/native/clr/host/assembly-store.cc | 305 ++++++++++++++++-- src/native/clr/host/host.cc | 1 + src/native/clr/include/constants.hh | 1 + .../include/runtime-base/android-system.hh | 11 + 5 files changed, 295 insertions(+), 26 deletions(-) diff --git a/src/java-runtime/java/mono/android/clr/MonoPackageManager.java b/src/java-runtime/java/mono/android/clr/MonoPackageManager.java index 4cfa0ea2169..5592cc2b68e 100644 --- a/src/java-runtime/java/mono/android/clr/MonoPackageManager.java +++ b/src/java-runtime/java/mono/android/clr/MonoPackageManager.java @@ -52,6 +52,7 @@ public static void LoadApplication (Context context) String language = locale.getLanguage () + "-" + locale.getCountry (); String filesDir = context.getFilesDir ().getAbsolutePath (); String cacheDir = context.getCacheDir ().getAbsolutePath (); + String codeCacheDir = context.getCodeCacheDir ().getAbsolutePath (); String dataDir = getNativeLibraryPath (context); ClassLoader loader = context.getClassLoader (); String runtimeDir = getNativeLibraryPath (runtimePackage); @@ -66,7 +67,7 @@ public static void LoadApplication (Context context) // // Should the order change here, src/native/clr/include/constants.hh must be updated accordingly // - String[] appDirs = new String[] {filesDir, cacheDir, dataDir}; + String[] appDirs = new String[] {filesDir, cacheDir, dataDir, codeCacheDir}; boolean haveSplitApks = runtimePackage.splitSourceDirs != null && runtimePackage.splitSourceDirs.length > 0; System.loadLibrary("monodroid"); diff --git a/src/native/clr/host/assembly-store.cc b/src/native/clr/host/assembly-store.cc index c000b5679f2..05bda3807f8 100644 --- a/src/native/clr/host/assembly-store.cc +++ b/src/native/clr/host/assembly-store.cc @@ -1,8 +1,19 @@ +#include +#include #include +#include +#include #include +#include + +#include +#include +#include +#include #include #include +#include #include #include #include @@ -33,8 +44,226 @@ namespace { return false; } -} + // ------------------------------------------------------------------------- + // EXPERIMENTAL: on-device cache of decompressed assemblies. + // + // The first time a compressed assembly is decompressed we queue its + // uncompressed bytes to be written (on a single background thread) to a + // per-assembly file in the app cache directory. On subsequent launches the + // file is mmap'd directly, skipping decompression entirely: no ZSTD cost, + // and the pages are file-backed / copy-on-write rather than dirty anonymous + // memory. + // + // This is a prototype: + // * There is no MSBuild opt-in yet (always on in RELEASE on this branch; + // set the XA_DISABLE_ASSEMBLY_CACHE env var to turn it off for A/B runs). + // * There is no assembly-store version stamp yet. Per-assembly staleness + // is guarded by an 8-byte footer holding a hash of the *compressed* + // payload, so any change to an assembly (e.g. after an app update) + // invalidates just that assembly's cache file. + // * We only ever cache the assemblies that were actually touched, since + // decompression is lazy and driven by the runtime's assembly probe. + // ------------------------------------------------------------------------- + namespace asm_cache { + constexpr std::string_view CACHE_DIR_NAME = "decompressed-assembly-cache"sv; + constexpr size_t FOOTER_SIZE = sizeof (uint64_t); + + struct WriteRequest final + { + std::string path; + const uint8_t *data; // stable pointer into uncompressed_assemblies_data_buffer + size_t size; + uint64_t token; + }; + + std::mutex state_lock; + std::condition_variable queue_cv; + std::deque write_queue; + std::string cache_dir; + bool initialized = false; + bool enabled = false; + bool writer_running = false; + + // Runtime-only, per-compressed-assembly pointer to the resolved + // uncompressed data (either the mmap'd cache file or the shared + // decompression buffer). Indexed by CompressedAssemblyHeader.descriptor_index. + uint8_t **tracking = nullptr; + + [[gnu::cold]] + void writer_loop () noexcept + { + for (;;) { + WriteRequest req; + { + std::unique_lock lock (state_lock); + queue_cv.wait (lock, [] { return !write_queue.empty (); }); + req = std::move (write_queue.front ()); + write_queue.pop_front (); + } + + std::string tmp_path = req.path; + tmp_path.append (".tmp"sv); + + int fd = open (tmp_path.c_str (), O_WRONLY | O_CREAT | O_TRUNC, 0600); + if (fd < 0) { + continue; + } + + bool ok = true; + size_t off = 0; + while (off < req.size) { + ssize_t written = write (fd, req.data + off, req.size - off); + if (written < 0) { + if (errno == EINTR) { + continue; + } + ok = false; + break; + } + off += static_cast(written); + } + + if (ok) { + ssize_t written = write (fd, &req.token, FOOTER_SIZE); + ok = (written == static_cast(FOOTER_SIZE)); + } + + if (ok) { + fsync (fd); + } + close (fd); + + // Atomic publish: a half-written temp file never becomes visible + // under the final name. + if (!ok || rename (tmp_path.c_str (), req.path.c_str ()) != 0) { + unlink (tmp_path.c_str ()); + } + } + } + + // Must be called while holding assembly_decompress_mutex. + void ensure_initialized () noexcept + { + if (initialized) { + return; + } + initialized = true; + + // Allow disabling at runtime (without rebuilding) for A/B benchmarking: + // adb shell setprop debug.net.asmcache 0 # off + // adb shell setprop debug.net.asmcache 1 # on (default) + if (getenv ("XA_DISABLE_ASSEMBLY_CACHE") != nullptr) { + return; + } + { + dynamic_local_property_string prop_value; + if (AndroidSystem::monodroid_get_system_property ("debug.net.asmcache"sv, prop_value) > 0 && + prop_value.get () != nullptr && prop_value.get ()[0] == '0') { + log_debug (LOG_ASSEMBLY, "On-device decompressed-assembly cache disabled via debug.net.asmcache"sv); + return; + } + } + + // The app code-cache directory (Context.getCodeCacheDir()) is wiped by + // Android on both app and platform updates, so a stale cache from a + // previous build cannot survive an update. + std::string const& code_cache_dir = AndroidSystem::get_app_code_cache_dir (); + if (code_cache_dir.empty ()) { + return; + } + + cache_dir.assign (code_cache_dir); + cache_dir.append ("/"); + cache_dir.append (CACHE_DIR_NAME); + mkdir (cache_dir.c_str (), 0700); // ignore errors (e.g. EEXIST) + + if (compressed_assembly_count > 0) { + tracking = new (std::nothrow) uint8_t*[compressed_assembly_count](); + } + + enabled = (tracking != nullptr); + } + + auto build_path (std::string_view name) noexcept -> std::string + { + std::string path = cache_dir; + path.append ("/"); + path.append (name); + return path; + } + + // Attempts to mmap a previously cached, decompressed assembly. Returns + // nullptr on any miss (absent, wrong size, stale footer, mmap failure). + // Must be called while holding assembly_decompress_mutex. + auto try_load (std::string_view name, uint32_t expected_size, uint64_t token) noexcept -> uint8_t* + { + if (!enabled) { + return nullptr; + } + + std::string path = build_path (name); + int fd = open (path.c_str (), O_RDONLY); + if (fd < 0) { + return nullptr; + } + + struct stat st {}; + if (fstat (fd, &st) != 0 || + static_cast(st.st_size) != static_cast(expected_size) + FOOTER_SIZE) { + close (fd); + return nullptr; + } + + size_t map_size = static_cast(expected_size) + FOOTER_SIZE; + // PROT_WRITE + MAP_PRIVATE: copy-on-write so the CLR/MAUI can write + // into the image (see the r/w HACK in the uncompressed path) while + // pages stay clean/file-backed until actually modified. + void *mapped = mmap (nullptr, map_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); + close (fd); + if (mapped == MAP_FAILED) { + return nullptr; + } + + uint64_t stored_token = 0; + memcpy (&stored_token, static_cast(mapped) + expected_size, FOOTER_SIZE); + if (stored_token != token) { + munmap (mapped, map_size); + return nullptr; + } + + return static_cast(mapped); + } + + // Queues a freshly decompressed assembly to be persisted. The data + // pointer must remain valid and immutable for the process lifetime + // (it points into uncompressed_assemblies_data_buffer). + // Must be called while holding assembly_decompress_mutex. + void enqueue_write (std::string_view name, const uint8_t *data, size_t size, uint64_t token) noexcept + { + if (!enabled) { + return; + } + + WriteRequest req { + .path = build_path (name), + .data = data, + .size = size, + .token = token, + }; + + { + std::lock_guard lock (state_lock); + write_queue.push_back (std::move (req)); + if (!writer_running) { + writer_running = true; + std::thread (writer_loop).detach (); + } + } + queue_cv.notify_one (); + } + } // namespace asm_cache +} // anonymous namespace [[gnu::always_inline]] void AssemblyStore::set_assembly_data_and_size (uint8_t* source_assembly_data, uint32_t source_assembly_data_size, uint8_t*& dest_assembly_data, uint32_t& dest_assembly_data_size) noexcept { @@ -101,11 +330,22 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co } uint8_t *data_buffer = uncompressed_assemblies_data_buffer + cad.buffer_offset; + uint32_t const descriptor_index = header->descriptor_index; + + // Resolves to the mmap'd cache file when this assembly was loaded from + // the on-device cache, otherwise to the shared decompression buffer. + auto resolve_data = [descriptor_index, data_buffer]() noexcept -> uint8_t* { + if (asm_cache::tracking != nullptr && asm_cache::tracking[descriptor_index] != nullptr) { + return asm_cache::tracking[descriptor_index]; + } + return data_buffer; + }; + if (!cad.loaded) { StartupAwareLock decompress_lock (assembly_decompress_mutex); if (cad.loaded) { - set_assembly_data_and_size (data_buffer, cad.uncompressed_file_size, assembly_data, assembly_data_size); + set_assembly_data_and_size (resolve_data (), cad.uncompressed_file_size, assembly_data, assembly_data_size); if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); @@ -118,6 +358,8 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co return {assembly_data, assembly_data_size}; } + asm_cache::ensure_initialized (); + if (header->uncompressed_length != cad.uncompressed_file_size) { if (header->uncompressed_length > cad.uncompressed_file_size) { Helpers::abort_application ( @@ -136,30 +378,43 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co } const char *data_start = pointer_add(e.image_data, sizeof(CompressedAssemblyHeader)); - size_t ret = ZSTD_decompress (data_buffer, cad.uncompressed_file_size, data_start, assembly_data_size); - - if (ZSTD_isError (ret)) { - Helpers::abort_application ( - LOG_ASSEMBLY, - std::format ( - "Decompression of assembly {} failed: {}"sv, - name, - ZSTD_getErrorName (ret) - ) - ); - } + uint64_t payload_token = static_cast(crc32_hash (data_start, assembly_data_size)); + + uint8_t *cached = asm_cache::try_load (name, cad.uncompressed_file_size, payload_token); + if (cached != nullptr) { + log_debug (LOG_ASSEMBLY, "Loaded decompressed assembly '{}' from the on-device cache"sv, name); + if (asm_cache::tracking != nullptr) { + asm_cache::tracking[descriptor_index] = cached; + } + } else { + size_t ret = ZSTD_decompress (data_buffer, cad.uncompressed_file_size, data_start, assembly_data_size); - if (ret != cad.uncompressed_file_size) { - Helpers::abort_application ( - LOG_ASSEMBLY, - std::format ( - "Decompression of assembly {} yielded a different size (expected {}, got {})"sv, - name, - cad.uncompressed_file_size, - static_cast(ret) - ) - ); + if (ZSTD_isError (ret)) { + Helpers::abort_application ( + LOG_ASSEMBLY, + std::format ( + "Decompression of assembly {} failed: {}"sv, + name, + ZSTD_getErrorName (ret) + ) + ); + } + + if (ret != cad.uncompressed_file_size) { + Helpers::abort_application ( + LOG_ASSEMBLY, + std::format ( + "Decompression of assembly {} yielded a different size (expected {}, got {})"sv, + name, + cad.uncompressed_file_size, + static_cast(ret) + ) + ); + } + + asm_cache::enqueue_write (name, data_buffer, cad.uncompressed_file_size, payload_token); } + cad.loaded = true; if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); @@ -167,7 +422,7 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co } } - set_assembly_data_and_size (data_buffer, cad.uncompressed_file_size, assembly_data, assembly_data_size); + set_assembly_data_and_size (resolve_data (), cad.uncompressed_file_size, assembly_data, assembly_data_size); } else #endif // def RELEASE { diff --git a/src/native/clr/host/host.cc b/src/native/clr/host/host.cc index 65645dff0b0..033f8425cf6 100644 --- a/src/native/clr/host/host.cc +++ b/src/native/clr/host/host.cc @@ -325,6 +325,7 @@ void Host::Java_mono_android_Runtime_initInternal ( AndroidSystem::detect_embedded_dso_mode (applicationDirs); AndroidSystem::set_running_in_emulator (isEmulator); AndroidSystem::set_primary_override_dir (files_dir); + AndroidSystem::set_app_code_cache_dir (applicationDirs[Constants::APP_DIRS_CODE_CACHE_DIR_INDEX]); AndroidSystem::create_update_dir (AndroidSystem::get_primary_override_dir ()); AndroidSystem::setup_environment (); Logger::init_reference_logging (AndroidSystem::get_primary_override_dir ()); diff --git a/src/native/clr/include/constants.hh b/src/native/clr/include/constants.hh index d9764c9cefb..5de37b8cfd8 100644 --- a/src/native/clr/include/constants.hh +++ b/src/native/clr/include/constants.hh @@ -114,6 +114,7 @@ namespace xamarin::android { static constexpr size_t APP_DIRS_FILES_DIR_INDEX = 0uz; static constexpr size_t APP_DIRS_CACHE_DIR_INDEX = 1uz; static constexpr size_t APP_DIRS_DATA_DIR_INDEX = 2uz; + static constexpr size_t APP_DIRS_CODE_CACHE_DIR_INDEX = 3uz; static inline constexpr size_t PROPERTY_VALUE_BUFFER_LEN = PROP_VALUE_MAX + 1uz; diff --git a/src/native/clr/include/runtime-base/android-system.hh b/src/native/clr/include/runtime-base/android-system.hh index b60871a93c4..3ddaee861b6 100644 --- a/src/native/clr/include/runtime-base/android-system.hh +++ b/src/native/clr/include/runtime-base/android-system.hh @@ -70,6 +70,16 @@ namespace xamarin::android { primary_override_dir = determine_primary_override_dir (home); } + static auto get_app_code_cache_dir () noexcept -> std::string const& + { + return app_code_cache_dir; + } + + static void set_app_code_cache_dir (jstring_wrapper& code_cache_dir) noexcept + { + app_code_cache_dir.assign (code_cache_dir.get_cstr ()); + } + static auto get_native_libraries_dir () noexcept -> std::string const& { return native_libraries_dir; @@ -146,6 +156,7 @@ namespace xamarin::android { static inline bool embedded_dso_mode_enabled = false; static inline std::string primary_override_dir; static inline std::string native_libraries_dir; + static inline std::string app_code_cache_dir; #if defined (DEBUG) static inline std::unordered_map bundled_properties; From 74d202d05251a36337cf67b9a4a34ca051501c09 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 3 Jul 2026 09:56:16 +0200 Subject: [PATCH 2/9] Fix data race in decompressed-assembly cache writer 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> --- src/native/clr/host/assembly-store.cc | 35 ++++++++++++++++++++------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/native/clr/host/assembly-store.cc b/src/native/clr/host/assembly-store.cc index 05bda3807f8..6a90564c54d 100644 --- a/src/native/clr/host/assembly-store.cc +++ b/src/native/clr/host/assembly-store.cc @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -71,10 +72,15 @@ namespace { struct WriteRequest final { - std::string path; - const uint8_t *data; // stable pointer into uncompressed_assemblies_data_buffer - size_t size; - uint64_t token; + std::string path; + // Private snapshot of the decompressed bytes, taken at enqueue time + // while the shared decompression buffer is still pristine. Owning a + // copy (rather than pointing into uncompressed_assemblies_data_buffer) + // avoids racing with the runtime, which may write into the image it + // receives once the decompress lock is released. + std::unique_ptr data; + size_t size; + uint64_t token; }; std::mutex state_lock; @@ -113,7 +119,7 @@ namespace { bool ok = true; size_t off = 0; while (off < req.size) { - ssize_t written = write (fd, req.data + off, req.size - off); + ssize_t written = write (fd, req.data.get () + off, req.size - off); if (written < 0) { if (errno == EINTR) { continue; @@ -235,9 +241,12 @@ namespace { return static_cast(mapped); } - // Queues a freshly decompressed assembly to be persisted. The data - // pointer must remain valid and immutable for the process lifetime - // (it points into uncompressed_assemblies_data_buffer). + // Queues a freshly decompressed assembly to be persisted. Takes a private + // snapshot of the bytes up front: the caller holds assembly_decompress_mutex + // and has not yet handed the shared buffer to the runtime, so the data is + // still pristine here. Copying now (rather than letting the background + // thread read the shared buffer later) avoids racing with the runtime, + // which may write into the image once the lock is released. // Must be called while holding assembly_decompress_mutex. void enqueue_write (std::string_view name, const uint8_t *data, size_t size, uint64_t token) noexcept { @@ -245,9 +254,17 @@ namespace { return; } + auto snapshot = std::unique_ptr (new (std::nothrow) uint8_t[size]); + if (snapshot == nullptr) { + // Out of memory: skip caching this assembly. Not fatal — the next + // launch simply decompresses it again. + return; + } + memcpy (snapshot.get (), data, size); + WriteRequest req { .path = build_path (name), - .data = data, + .data = std::move (snapshot), .size = size, .token = token, }; From e560897514151439dcd3c90255a05b10030937cc Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 3 Jul 2026 10:18:10 +0200 Subject: [PATCH 3/9] Simplify decompressed-assembly cache writer Tidy up the file-writing path without pulling /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> --- src/native/clr/host/assembly-store.cc | 64 ++++++++++++++------------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/src/native/clr/host/assembly-store.cc b/src/native/clr/host/assembly-store.cc index 6a90564c54d..fb6219db2ba 100644 --- a/src/native/clr/host/assembly-store.cc +++ b/src/native/clr/host/assembly-store.cc @@ -73,14 +73,15 @@ namespace { struct WriteRequest final { std::string path; - // Private snapshot of the decompressed bytes, taken at enqueue time - // while the shared decompression buffer is still pristine. Owning a - // copy (rather than pointing into uncompressed_assemblies_data_buffer) - // avoids racing with the runtime, which may write into the image it - // receives once the decompress lock is released. + // The complete file image to persist: the decompressed bytes followed + // by the 8-byte token footer, laid out exactly as it lives on disk. + // This is a private snapshot taken at enqueue time while the shared + // decompression buffer is still pristine (owning a copy, rather than + // pointing into uncompressed_assemblies_data_buffer, avoids racing with + // the runtime, which may write into the image once the decompress lock + // is released). std::unique_ptr data; size_t size; - uint64_t token; }; std::mutex state_lock; @@ -96,6 +97,25 @@ namespace { // decompression buffer). Indexed by CompressedAssemblyHeader.descriptor_index. uint8_t **tracking = nullptr; + // write(2) can write fewer bytes than requested or be interrupted by a + // signal (EINTR); loop until the whole buffer is written or an + // unrecoverable error occurs. + bool write_fully (int fd, const uint8_t *buf, size_t len) noexcept + { + size_t off = 0; + while (off < len) { + ssize_t n = write (fd, buf + off, len - off); + if (n < 0) { + if (errno == EINTR) { + continue; + } + return false; + } + off += static_cast(n); + } + return true; + } + [[gnu::cold]] void writer_loop () noexcept { @@ -116,25 +136,7 @@ namespace { continue; } - bool ok = true; - size_t off = 0; - while (off < req.size) { - ssize_t written = write (fd, req.data.get () + off, req.size - off); - if (written < 0) { - if (errno == EINTR) { - continue; - } - ok = false; - break; - } - off += static_cast(written); - } - - if (ok) { - ssize_t written = write (fd, &req.token, FOOTER_SIZE); - ok = (written == static_cast(FOOTER_SIZE)); - } - + bool ok = write_fully (fd, req.data.get (), req.size); if (ok) { fsync (fd); } @@ -254,19 +256,21 @@ namespace { return; } - auto snapshot = std::unique_ptr (new (std::nothrow) uint8_t[size]); + // Build the full on-disk image up front: [payload][8-byte token footer]. + size_t total = size + FOOTER_SIZE; + auto snapshot = std::unique_ptr (new (std::nothrow) uint8_t[total]); if (snapshot == nullptr) { // Out of memory: skip caching this assembly. Not fatal — the next // launch simply decompresses it again. return; } memcpy (snapshot.get (), data, size); + memcpy (snapshot.get () + size, &token, FOOTER_SIZE); WriteRequest req { - .path = build_path (name), - .data = std::move (snapshot), - .size = size, - .token = token, + .path = build_path (name), + .data = std::move (snapshot), + .size = total, }; { From 34974a6fe97242f0f9d12199df1231bbea0310e5 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 3 Jul 2026 10:23:31 +0200 Subject: [PATCH 4/9] Split cache file persistence out of writer_loop 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> --- src/native/clr/host/assembly-store.cc | 47 ++++++++++++++++----------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/native/clr/host/assembly-store.cc b/src/native/clr/host/assembly-store.cc index fb6219db2ba..eb8edd10998 100644 --- a/src/native/clr/host/assembly-store.cc +++ b/src/native/clr/host/assembly-store.cc @@ -116,6 +116,33 @@ namespace { return true; } + // Persists one request to disk: write to a temp file, fsync, then + // atomically rename it into place. A half-written temp file never becomes + // visible under the final name, and on any failure the temp is removed. + void write_cache_file (WriteRequest const& req) noexcept + { + std::string tmp_path = req.path; + tmp_path.append (".tmp"sv); + + int fd = open (tmp_path.c_str (), O_WRONLY | O_CREAT | O_TRUNC, 0600); + if (fd < 0) { + return; + } + + bool ok = write_fully (fd, req.data.get (), req.size); + if (ok) { + fsync (fd); + } + close (fd); + + // Atomic publish: a half-written temp file never becomes visible + // under the final name. + if (!ok || rename (tmp_path.c_str (), req.path.c_str ()) != 0) { + unlink (tmp_path.c_str ()); + } + } + + // Single background thread: dequeues requests and persists them. [[gnu::cold]] void writer_loop () noexcept { @@ -128,25 +155,7 @@ namespace { write_queue.pop_front (); } - std::string tmp_path = req.path; - tmp_path.append (".tmp"sv); - - int fd = open (tmp_path.c_str (), O_WRONLY | O_CREAT | O_TRUNC, 0600); - if (fd < 0) { - continue; - } - - bool ok = write_fully (fd, req.data.get (), req.size); - if (ok) { - fsync (fd); - } - close (fd); - - // Atomic publish: a half-written temp file never becomes visible - // under the final name. - if (!ok || rename (tmp_path.c_str (), req.path.c_str ()) != 0) { - unlink (tmp_path.c_str ()); - } + write_cache_file (req); } } From 0c94c520cd05dc18873db708f83e61b51e30ec73 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 10 Jul 2026 17:01:38 +0200 Subject: [PATCH 5/9] [assembly-store] Harden decompression cache 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 --- Documentation/building/configuration.md | 6 + Documentation/project-docs/AssemblyStores.md | 4 + .../GenerateNativeApplicationConfigSources.cs | 2 + .../Tasks/CreateAssemblyStoreTests.cs | 57 +++ ...rateNativeApplicationConfigSourcesTests.cs | 33 ++ .../Utilities/EnvironmentHelper.cs | 7 + .../Utilities/ApplicationConfigCLR.cs | 1 + ...icationConfigNativeAssemblyGeneratorCLR.cs | 2 + .../AssemblyStoreGenerator.Classes.cs | 10 +- .../Utilities/AssemblyStoreGenerator.cs | 32 +- .../Xamarin.Android.Common.targets | 2 + src/native/clr/host/assembly-store.cc | 441 +++++++++++++----- src/native/clr/include/host/assembly-store.hh | 1 + src/native/clr/include/xamarin-app.hh | 3 + .../xamarin-app-stub/application_dso_stub.cc | 1 + .../mono/xamarin-app-stub/xamarin-app.hh | 4 +- .../Tests/InstallAndRunTests.cs | 70 +++ .../AssemblyStore/StoreReader_V2.Classes.cs | 8 +- .../AssemblyStore/StoreReader_V2.cs | 6 +- 19 files changed, 556 insertions(+), 134 deletions(-) create mode 100644 src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/CreateAssemblyStoreTests.cs diff --git a/Documentation/building/configuration.md b/Documentation/building/configuration.md index 9e409096049..c605a3dc277 100644 --- a/Documentation/building/configuration.md +++ b/Documentation/building/configuration.md @@ -118,6 +118,12 @@ Overridable MSBuild properties include: assemblies placed in the APK will be compressed in `Release` builds. `Debug` builds are not affected. + * `$(AndroidEnableAssemblyStoreDecompressionCache)`: Defaults to `False`. When + enabled for a CoreCLR `Release` build, decompressed assemblies are cached in + the app's Android code-cache directory and mapped from there on subsequent + launches. The cache consumes additional on-device storage and is rebuilt + after app or platform updates. + ## Options suitable for local development ### Native runtime (`src/native`) diff --git a/Documentation/project-docs/AssemblyStores.md b/Documentation/project-docs/AssemblyStores.md index a18e126ca39..117d8760f04 100644 --- a/Documentation/project-docs/AssemblyStores.md +++ b/Documentation/project-docs/AssemblyStores.md @@ -111,6 +111,7 @@ The header is a fixed-size structure at the beginning of each assembly store fil - **ENTRY_COUNT** (`uint32_t`) - Number of assemblies in the store - **INDEX_ENTRY_COUNT** (`uint32_t`) - Number of entries in the index (typically `ENTRY_COUNT * 2`) - **INDEX_SIZE** (`uint32_t`) - Index size in bytes +- **CONTENT_ID** (`uint64_t`) - Deterministic xxHash3 of everything after the header ## [INDEX] @@ -168,6 +169,7 @@ All kinds of stores share the following header format: uint32_t entry_count; uint32_t index_entry_count; uint32_t index_size; // index size in bytes + uint64_t content_id; }; Individual fields have the following meanings: @@ -179,6 +181,7 @@ Individual fields have the following meanings: table, see below) - `index_entry_count`: number of entries in the index - `index_size`: index size in bytes + - `content_id`: deterministic xxHash3 of the index, descriptors, names, and assembly data ## Assembly descriptor table @@ -284,6 +287,7 @@ struct [[gnu::packed]] AssemblyStoreHeader final uint32_t entry_count; uint32_t index_entry_count; uint32_t index_size; // index size in bytes + uint64_t content_id; }; ``` diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs index dd2788f30a1..06c70e6f479 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs @@ -59,6 +59,7 @@ public class GenerateNativeApplicationConfigSources : AndroidTask public bool EnableMarshalMethods { get; set; } public bool EnableManagedMarshalMethodsLookup { get; set; } + public bool AndroidEnableAssemblyStoreDecompressionCache { get; set; } public string? RuntimeConfigBinFilePath { get; set; } public string ProjectRuntimeConfigFilePath { get; set; } = String.Empty; public string? BoundExceptionType { get; set; } @@ -286,6 +287,7 @@ static bool ShouldSkipAssembly (ITaskItem assembly) ManagedMarshalMethodsLookupEnabled = EnableManagedMarshalMethodsLookup, IgnoreSplitConfigs = ShouldIgnoreSplitConfigs (), HaveAssemblyStore = UseAssemblyStore, + AssemblyStoreDecompressionCacheEnabled = AndroidEnableAssemblyStoreDecompressionCache, }; } else { appConfigAsmGen = new ApplicationConfigNativeAssemblyGenerator (envBuilder.EnvironmentVariables, envBuilder.SystemProperties, Log) { diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/CreateAssemblyStoreTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/CreateAssemblyStoreTests.cs new file mode 100644 index 00000000000..466f52e15df --- /dev/null +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/CreateAssemblyStoreTests.cs @@ -0,0 +1,57 @@ +#nullable enable +using System; +using System.Collections.Generic; +using System.IO; +using System.IO.Hashing; +using System.Linq; + +using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; +using NUnit.Framework; +using Xamarin.Android.Tasks; + +namespace Xamarin.Android.Build.Tests.Tasks; + +[TestFixture] +public class CreateAssemblyStoreTests : BaseTest +{ + [Test] + public void ContentIdMatchesStoreContents () + { + string testDirectory = Path.Combine (Root, "temp", nameof (ContentIdMatchesStoreContents)); + Directory.CreateDirectory (testDirectory); + + string assemblyPath = Path.Combine (testDirectory, "Example.dll.zst"); + byte [] assemblyData = [1, 3, 3, 7, 9, 11, 17, 23]; + File.WriteAllBytes (assemblyPath, assemblyData); + + var metadata = new Dictionary { + ["Abi"] = "arm64-v8a", + }; + var task = new CreateAssemblyStore { + BuildEngine = new MockBuildEngine (TestContext.Out), + AppSharedLibrariesDir = Path.Combine (testDirectory, "stores"), + ResolvedFrameworkAssemblies = [], + ResolvedUserAssemblies = [new TaskItem (assemblyPath, metadata)], + SupportedAbis = ["arm64-v8a"], + TargetRuntime = "CoreCLR", + UseAssemblyStore = true, + }; + + Assert.IsTrue (task.Execute (), "CreateAssemblyStore should succeed."); + + string storePath = task.AssembliesToAddToArchive.Single ().ItemSpec; + byte [] store = File.ReadAllBytes (storePath); + using var reader = new BinaryReader (new MemoryStream (store)); + Assert.AreEqual (0x41424158u, reader.ReadUInt32 (), "Unexpected assembly store magic."); + Assert.AreEqual (0x80010004u, reader.ReadUInt32 (), "Unexpected arm64 assembly store version."); + reader.BaseStream.Seek (3 * sizeof (uint), SeekOrigin.Current); + ulong contentId = reader.ReadUInt64 (); + + Assert.AreEqual ( + XxHash3.HashToUInt64 (store.AsSpan (5 * sizeof (uint) + sizeof (ulong))), + contentId, + "The content ID should hash everything after the assembly store header." + ); + } +} diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateNativeApplicationConfigSourcesTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateNativeApplicationConfigSourcesTests.cs index 803cc00bc32..04cd7982543 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateNativeApplicationConfigSourcesTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateNativeApplicationConfigSourcesTests.cs @@ -42,4 +42,37 @@ public void HaveAssemblyStoreIsEmittedForCoreCLR (bool haveAssemblyStore) var config = (EnvironmentHelper.ApplicationConfig_CoreCLR)EnvironmentHelper.ReadApplicationConfig (environmentFiles, AndroidRuntime.CoreCLR); Assert.AreEqual (haveAssemblyStore, config.have_assembly_store); } + + [TestCase (false)] + [TestCase (true)] + public void AssemblyStoreDecompressionCacheSettingIsEmitted (bool enabled) + { + string outputRoot = Path.Combine (Root, "temp", $"{nameof (AssemblyStoreDecompressionCacheSettingIsEmitted)}-{enabled}"); + string monoAndroidPath = Path.Combine (TestEnvironment.MonoAndroidFrameworkDirectory, "Mono.Android.dll"); + FileAssert.Exists (monoAndroidPath); + + var task = new GenerateNativeApplicationConfigSources { + BuildEngine = new MockBuildEngine (TestContext.Out), + ResolvedAssemblies = [new TaskItem (monoAndroidPath)], + EnvironmentOutputDirectory = Path.Combine (outputRoot, "android"), + SupportedAbis = ["arm64-v8a"], + AndroidPackageName = "com.microsoft.android.cachetest", + EnablePreloadAssembliesDefault = false, + TargetsCLR = true, + AndroidRuntime = "CoreCLR", + UseAssemblyStore = true, + AndroidEnableAssemblyStoreDecompressionCache = enabled, + }; + + Assert.IsTrue (task.Execute (), "GenerateNativeApplicationConfigSources should succeed."); + + var environmentFiles = EnvironmentHelper.GatherEnvironmentFiles ( + outputRoot, + "arm64-v8a", + required: true, + runtime: AndroidRuntime.CoreCLR + ); + var config = (EnvironmentHelper.ApplicationConfig_CoreCLR)EnvironmentHelper.ReadApplicationConfig (environmentFiles, AndroidRuntime.CoreCLR); + Assert.AreEqual (enabled, config.assembly_store_decompression_cache_enabled); + } } diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs index d8c2ab96b50..2cdc0e19b03 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs @@ -62,6 +62,7 @@ public sealed class ApplicationConfig_CoreCLR : IApplicationConfig public string android_package_name = String.Empty; public bool managed_marshal_methods_lookup_enabled; public bool have_assembly_store; + public bool assembly_store_decompression_cache_enabled; } const uint ApplicationConfigFieldCount_CoreCLR = 20; @@ -407,6 +408,11 @@ static IApplicationConfig ReadApplicationConfig_CoreCLR (EnvironmentFile envFile AssertFieldType (envFile.Path, parser.SourceFilePath, ".byte", field [0], item.LineNumber); ret.have_assembly_store = ConvertFieldToBool ("have_assembly_store", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]); break; + + 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++; } @@ -772,6 +778,7 @@ static void AssertApplicationConfigIsIdentical (ApplicationConfig_CoreCLR firstA Assert.AreEqual (firstAppConfig.system_property_count, secondAppConfig.system_property_count, $"Field 'system_property_count' has different value in environment file '{secondEnvFile}' than in environment file '{firstEnvFile}'"); Assert.AreEqual (firstAppConfig.android_package_name, secondAppConfig.android_package_name, $"Field 'android_package_name' has different value in environment file '{secondEnvFile}' than in environment file '{firstEnvFile}'"); Assert.AreEqual (firstAppConfig.have_assembly_store, secondAppConfig.have_assembly_store, $"Field 'have_assembly_store' has different value in environment file '{secondEnvFile}' than in environment file '{firstEnvFile}'"); + Assert.AreEqual (firstAppConfig.assembly_store_decompression_cache_enabled, secondAppConfig.assembly_store_decompression_cache_enabled, $"Field 'assembly_store_decompression_cache_enabled' has different value in environment file '{secondEnvFile}' than in environment file '{firstEnvFile}'"); } static void AssertApplicationConfigIsIdentical (ApplicationConfig_MonoVM firstAppConfig, string firstEnvFile, ApplicationConfig_MonoVM secondAppConfig, string secondEnvFile) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigCLR.cs index 832c37299d7..5a0d0f4cee5 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigCLR.cs @@ -51,4 +51,5 @@ sealed class ApplicationConfigCLR public string android_package_name = String.Empty; public bool managed_marshal_methods_lookup_enabled; public bool have_assembly_store; + public bool assembly_store_decompression_cache_enabled; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGeneratorCLR.cs index 4f977d2d71f..e9a4475bee4 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGeneratorCLR.cs @@ -198,6 +198,7 @@ sealed class DsoCacheState public bool ManagedMarshalMethodsLookupEnabled { get; set; } public bool IgnoreSplitConfigs { get; set; } public bool HaveAssemblyStore { get; set; } + public bool AssemblyStoreDecompressionCacheEnabled { get; set; } public ApplicationConfigNativeAssemblyGeneratorCLR (IDictionary environmentVariables, IDictionary systemProperties, IDictionary? runtimeProperties, TaskLoggingHelper log) @@ -284,6 +285,7 @@ protected override void Construct (LlvmIrModule module) jni_remapping_replacement_method_index_entry_count = (uint)JniRemappingReplacementMethodIndexEntryCount, android_package_name = AndroidPackageName, have_assembly_store = HaveAssemblyStore, + assembly_store_decompression_cache_enabled = AssemblyStoreDecompressionCacheEnabled, }; application_config = new StructureInstance (applicationConfigStructureInfo, app_cfg); module.AddGlobalVariable ("application_config", application_config); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.Classes.cs b/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.Classes.cs index 9df30db6303..7a30ec231b8 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.Classes.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.Classes.cs @@ -5,7 +5,7 @@ partial class AssemblyStoreGenerator { sealed class AssemblyStoreHeader { - public const uint NativeSize = 5 * sizeof (uint); + public const uint NativeSize = 5 * sizeof (uint) + sizeof (ulong); public readonly uint magic = ASSEMBLY_STORE_MAGIC; public readonly uint version; @@ -14,17 +14,19 @@ sealed class AssemblyStoreHeader // Index size in bytes public readonly uint index_size; + public readonly ulong content_id; - public AssemblyStoreHeader (uint version, uint entry_count, uint index_entry_count, uint index_size) + public AssemblyStoreHeader (uint version, uint entry_count, uint index_entry_count, uint index_size, ulong content_id) { this.version = version; this.entry_count = entry_count; this.index_entry_count = index_entry_count; this.index_size = index_size; + this.content_id = content_id; } #if XABT_TESTS - public AssemblyStoreHeader (uint magic, uint version, uint entry_count, uint index_entry_count, uint index_size) - : this (version, entry_count, index_entry_count, index_size) + public AssemblyStoreHeader (uint magic, uint version, uint entry_count, uint index_entry_count, uint index_size, ulong content_id) + : this (version, entry_count, index_entry_count, index_size, content_id) { this.magic = magic; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs index 42dace7c726..a8644b10d5f 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.IO.Hashing; using Microsoft.Android.Build.Tasks; using Microsoft.Build.Utilities; @@ -28,6 +29,7 @@ namespace Xamarin.Android.Tasks; // [ENTRY_COUNT] uint; number of entries in the store // [INDEX_ENTRY_COUNT] uint; number of entries in the index // [INDEX_SIZE] uint; index size in bytes +// [CONTENT_ID] ulong: deterministic hash of everything after the header // // INDEX (variable size, HEADER.ENTRY_COUNT*2 entries, for assembly names with and without the extension) // [NAME_HASH] uint CRC32 for CoreCLR; uint/ulong xxhash for MonoVM depending on target bitness @@ -129,7 +131,7 @@ string Generate (string baseOutputDirectory, AndroidTargetArch arch, List 0) { + hash.Append (buffer.AsSpan (0, bytesRead)); + } + + return hash.GetCurrentHashAsUInt64 (); + } + void CopyData (FileInfo? src, Stream dest, string storePath) { if (src == null) { @@ -262,6 +286,7 @@ void WriteHeader (BinaryWriter writer, AssemblyStoreHeader header) writer.Write (header.entry_count); writer.Write (header.index_entry_count); writer.Write (header.index_size); + writer.Write (header.content_id); } #if XABT_TESTS AssemblyStoreHeader ReadHeader (BinaryReader reader) @@ -272,8 +297,9 @@ AssemblyStoreHeader ReadHeader (BinaryReader reader) uint entry_count = reader.ReadUInt32 (); uint index_entry_count = reader.ReadUInt32 (); uint index_size = reader.ReadUInt32 (); + ulong content_id = reader.ReadUInt64 (); - return new AssemblyStoreHeader (magic, version, entry_count, index_entry_count, index_size); + return new AssemblyStoreHeader (magic, version, entry_count, index_entry_count, index_size, content_id); } #endif diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index 8e0b5004150..c0235f7c06c 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -167,6 +167,7 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved. Android.App.Fragment True <_AndroidAssemblyStoreCompressionLevel Condition=" '$(_AndroidAssemblyStoreCompressionLevel)' == '' ">3 + False False <_AndroidCheckedBuild Condition=" '$(_AndroidCheckedBuild)' == '' "> @@ -1809,6 +1810,7 @@ because xbuild doesn't support framework reference assemblies. BoundExceptionType="$(AndroidBoundExceptionType)" RuntimeConfigBinFilePath="$(_BinaryRuntimeConfigPath)" UseAssemblyStore="$(_AndroidUseAssemblyStore)" + AndroidEnableAssemblyStoreDecompressionCache="$(AndroidEnableAssemblyStoreDecompressionCache)" EnableMarshalMethods="$(_AndroidUseMarshalMethods)" EnableManagedMarshalMethodsLookup="$(_AndroidUseManagedMarshalMethodsLookup)" CustomBundleConfigFile="$(AndroidBundleConfigurationFile)" diff --git a/src/native/clr/host/assembly-store.cc b/src/native/clr/host/assembly-store.cc index eb8edd10998..90e6bf824f2 100644 --- a/src/native/clr/host/assembly-store.cc +++ b/src/native/clr/host/assembly-store.cc @@ -1,13 +1,13 @@ #include #include #include -#include #include #include +#include #include -#include #include +#include #include #include #include @@ -46,60 +46,54 @@ namespace { return false; } - // ------------------------------------------------------------------------- - // EXPERIMENTAL: on-device cache of decompressed assemblies. - // - // The first time a compressed assembly is decompressed we queue its - // uncompressed bytes to be written (on a single background thread) to a - // per-assembly file in the app cache directory. On subsequent launches the - // file is mmap'd directly, skipping decompression entirely: no ZSTD cost, - // and the pages are file-backed / copy-on-write rather than dirty anonymous - // memory. - // - // This is a prototype: - // * There is no MSBuild opt-in yet (always on in RELEASE on this branch; - // set the XA_DISABLE_ASSEMBLY_CACHE env var to turn it off for A/B runs). - // * There is no assembly-store version stamp yet. Per-assembly staleness - // is guarded by an 8-byte footer holding a hash of the *compressed* - // payload, so any change to an assembly (e.g. after an app update) - // invalidates just that assembly's cache file. - // * We only ever cache the assemblies that were actually touched, since - // decompression is lazy and driven by the runtime's assembly probe. - // ------------------------------------------------------------------------- namespace asm_cache { - constexpr std::string_view CACHE_DIR_NAME = "decompressed-assembly-cache"sv; - constexpr size_t FOOTER_SIZE = sizeof (uint64_t); + constexpr std::string_view CACHE_DIR_NAME = "decompressed-assembly-cache-v1"sv; + constexpr uint32_t CACHE_FILE_MAGIC = 0x43434158; // 'XACC', little-endian + constexpr uint32_t CACHE_FILE_FORMAT_VERSION = 1; + constexpr size_t MAX_QUEUED_BYTES = 32uz * 1024uz * 1024uz; + + struct [[gnu::packed]] CacheFileFooter final + { + uint32_t magic; + uint32_t version; + uint64_t store_id; + uint64_t payload_hash; + uint32_t descriptor_index; + uint32_t payload_size; + }; + + static_assert (sizeof (CacheFileFooter) == 32uz); struct WriteRequest final { std::string path; - // The complete file image to persist: the decompressed bytes followed - // by the 8-byte token footer, laid out exactly as it lives on disk. - // This is a private snapshot taken at enqueue time while the shared - // decompression buffer is still pristine (owning a copy, rather than - // pointing into uncompressed_assemblies_data_buffer, avoids racing with - // the runtime, which may write into the image once the decompress lock - // is released). std::unique_ptr data; size_t size; }; - std::mutex state_lock; - std::condition_variable queue_cv; - std::deque write_queue; - std::string cache_dir; - bool initialized = false; - bool enabled = false; - bool writer_running = false; - - // Runtime-only, per-compressed-assembly pointer to the resolved - // uncompressed data (either the mmap'd cache file or the shared - // decompression buffer). Indexed by CompressedAssemblyHeader.descriptor_index. - uint8_t **tracking = nullptr; - - // write(2) can write fewer bytes than requested or be interrupted by a - // signal (EINTR); loop until the whole buffer is written or an - // unrecoverable error occurs. + enum class WriteResult + { + Succeeded, + Skipped, + Failed, + }; + + std::mutex state_lock; + std::deque write_queue; + std::string cache_dir; + std::unique_ptr tracking; + size_t queued_bytes = 0; + uint64_t store_id = 0; + bool initialized = false; + bool enabled = false; + bool writes_enabled = false; + bool writer_running = false; + + auto hash_payload (const uint8_t *data, size_t size) noexcept -> uint64_t + { + return static_cast(crc32_hash (reinterpret_cast(data), size)); + } + bool write_fully (int fd, const uint8_t *buf, size_t len) noexcept { size_t off = 0; @@ -111,80 +105,191 @@ namespace { } return false; } + if (n == 0) { + errno = EIO; + return false; + } off += static_cast(n); } return true; } - // Persists one request to disk: write to a temp file, fsync, then - // atomically rename it into place. A half-written temp file never becomes - // visible under the final name, and on any failure the temp is removed. - void write_cache_file (WriteRequest const& req) noexcept + void log_file_error (std::string_view operation, std::string const& path, int error) noexcept + { + log_debug (LOG_ASSEMBLY, "Decompressed-assembly cache {} failed for '{}': {}"sv, operation, path, std::strerror (error)); + } + + auto write_cache_file (WriteRequest const& req) noexcept -> WriteResult { std::string tmp_path = req.path; tmp_path.append (".tmp"sv); - int fd = open (tmp_path.c_str (), O_WRONLY | O_CREAT | O_TRUNC, 0600); + int fd; + do { + fd = open (tmp_path.c_str (), O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, 0600); + } while (fd < 0 && errno == EINTR); if (fd < 0) { - return; + log_file_error ("temporary-file creation"sv, req.path, errno); + return WriteResult::Failed; } bool ok = write_fully (fd, req.data.get (), req.size); - if (ok) { - fsync (fd); + int error = ok ? 0 : errno; + if (close (fd) != 0 && ok) { + ok = false; + error = errno; } - close (fd); - // Atomic publish: a half-written temp file never becomes visible - // under the final name. - if (!ok || rename (tmp_path.c_str (), req.path.c_str ()) != 0) { + if (!ok) { + log_file_error ("write"sv, req.path, error); + unlink (tmp_path.c_str ()); + return WriteResult::Failed; + } + + int rename_result; + do { + rename_result = rename (tmp_path.c_str (), req.path.c_str ()); + } while (rename_result != 0 && errno == EINTR); + + if (rename_result != 0) { + error = errno; + // Another process can publish the shared, content-identical temp + // file first. Its rename consumes the temp path, so this writer + // has nothing left to publish. + if (error == ENOENT) { + return WriteResult::Skipped; + } + + log_file_error ("publish"sv, req.path, error); unlink (tmp_path.c_str ()); + return WriteResult::Failed; } + + return WriteResult::Succeeded; + } + + void clear_write_queue_locked () noexcept + { + for (WriteRequest const& request : write_queue) { + queued_bytes -= request.size; + } + write_queue.clear (); } - // Single background thread: dequeues requests and persists them. [[gnu::cold]] - void writer_loop () noexcept + auto writer_loop ([[maybe_unused]] void *arg) noexcept -> void* { - for (;;) { - WriteRequest req; + while (true) { + WriteRequest request; { - std::unique_lock lock (state_lock); - queue_cv.wait (lock, [] { return !write_queue.empty (); }); - req = std::move (write_queue.front ()); + std::lock_guard lock (state_lock); + if (write_queue.empty ()) { + writer_running = false; + return nullptr; + } + + request = std::move (write_queue.front ()); write_queue.pop_front (); } - write_cache_file (req); + size_t request_size = request.size; + WriteResult write_result = write_cache_file (request); + request.data.reset (); + + { + std::lock_guard lock (state_lock); + queued_bytes -= request_size; + if (write_result == WriteResult::Failed) { + writes_enabled = false; + clear_write_queue_locked (); + writer_running = false; + log_debug (LOG_ASSEMBLY, "Disabling decompressed-assembly cache writes after a persistence failure"sv); + return nullptr; + } + } } } - // Must be called while holding assembly_decompress_mutex. - void ensure_initialized () noexcept + bool start_writer_locked () noexcept + { + pthread_attr_t attributes; + int result = pthread_attr_init (&attributes); + bool attributes_initialized = result == 0; + if (result == 0) { + result = pthread_attr_setdetachstate (&attributes, PTHREAD_CREATE_DETACHED); + } + + pthread_t writer_thread; + if (result == 0) { + result = pthread_create (&writer_thread, &attributes, writer_loop, nullptr); + } + + if (attributes_initialized) { + pthread_attr_destroy (&attributes); + } + if (result != 0) { + log_debug (LOG_ASSEMBLY, "Failed to start decompressed-assembly cache writer: {}"sv, std::strerror (result)); + return false; + } + + return true; + } + + bool ensure_directory (std::string const& path) noexcept + { + if (mkdir (path.c_str (), 0700) == 0) { + return true; + } + + int error = errno; + if (error != EEXIST) { + log_file_error ("directory creation"sv, path, error); + return false; + } + + struct stat st {}; + if (lstat (path.c_str (), &st) != 0) { + log_file_error ("directory validation"sv, path, errno); + return false; + } + if (!S_ISDIR (st.st_mode)) { + log_file_error ("directory validation"sv, path, ENOTDIR); + return false; + } + + return true; + } + + void ensure_initialized (uint64_t assembly_store_id) noexcept { if (initialized) { return; } initialized = true; - // Allow disabling at runtime (without rebuilding) for A/B benchmarking: + bool cache_requested = application_config.assembly_store_decompression_cache_enabled; + + // Allow overriding the build setting at runtime for A/B benchmarking: // adb shell setprop debug.net.asmcache 0 # off - // adb shell setprop debug.net.asmcache 1 # on (default) + // adb shell setprop debug.net.asmcache 1 # on if (getenv ("XA_DISABLE_ASSEMBLY_CACHE") != nullptr) { return; } { dynamic_local_property_string prop_value; - if (AndroidSystem::monodroid_get_system_property ("debug.net.asmcache"sv, prop_value) > 0 && - prop_value.get () != nullptr && prop_value.get ()[0] == '0') { - log_debug (LOG_ASSEMBLY, "On-device decompressed-assembly cache disabled via debug.net.asmcache"sv); - return; + if (AndroidSystem::monodroid_get_system_property ("debug.net.asmcache"sv, prop_value) > 0 && prop_value.get () != nullptr) { + if (prop_value.get ()[0] == '0') { + cache_requested = false; + } else if (prop_value.get ()[0] == '1') { + cache_requested = true; + } } } - // The app code-cache directory (Context.getCodeCacheDir()) is wiped by - // Android on both app and platform updates, so a stale cache from a - // previous build cannot survive an update. + if (!cache_requested) { + return; + } + std::string const& code_cache_dir = AndroidSystem::get_app_code_cache_dir (); if (code_cache_dir.empty ()) { return; @@ -193,104 +298,184 @@ namespace { cache_dir.assign (code_cache_dir); cache_dir.append ("/"); cache_dir.append (CACHE_DIR_NAME); - mkdir (cache_dir.c_str (), 0700); // ignore errors (e.g. EEXIST) + if (!ensure_directory (cache_dir)) { + return; + } + + store_id = assembly_store_id; + cache_dir.append ("/"); + cache_dir.append (std::format ("{:x}", store_id)); + if (!ensure_directory (cache_dir)) { + return; + } if (compressed_assembly_count > 0) { - tracking = new (std::nothrow) uint8_t*[compressed_assembly_count](); + tracking.reset (new (std::nothrow) uint8_t*[compressed_assembly_count]()); } enabled = (tracking != nullptr); + if (!enabled) { + return; + } + + { + std::lock_guard lock (state_lock); + writes_enabled = true; + } + + log_debug ( + LOG_ASSEMBLY, + "Enabled decompressed-assembly cache at '{}'; store ID 0x{:x}; write queue limit {} bytes"sv, + cache_dir, + store_id, + MAX_QUEUED_BYTES + ); } - auto build_path (std::string_view name) noexcept -> std::string + auto build_path (uint32_t descriptor_index) noexcept -> std::string { std::string path = cache_dir; path.append ("/"); - path.append (name); + path.append (std::to_string (descriptor_index)); + path.append (".bin"sv); return path; } - // Attempts to mmap a previously cached, decompressed assembly. Returns - // nullptr on any miss (absent, wrong size, stale footer, mmap failure). - // Must be called while holding assembly_decompress_mutex. - auto try_load (std::string_view name, uint32_t expected_size, uint64_t token) noexcept -> uint8_t* + auto try_load (uint32_t descriptor_index, std::string_view name, uint32_t expected_size) noexcept -> uint8_t* { if (!enabled) { return nullptr; } - std::string path = build_path (name); - int fd = open (path.c_str (), O_RDONLY); + std::string path = build_path (descriptor_index); + int fd = open (path.c_str (), O_RDONLY | O_CLOEXEC | O_NOFOLLOW); if (fd < 0) { return nullptr; } struct stat st {}; if (fstat (fd, &st) != 0 || - static_cast(st.st_size) != static_cast(expected_size) + FOOTER_SIZE) { + !S_ISREG (st.st_mode) || + static_cast(st.st_size) != static_cast(expected_size) + sizeof (CacheFileFooter)) { close (fd); return nullptr; } - size_t map_size = static_cast(expected_size) + FOOTER_SIZE; - // PROT_WRITE + MAP_PRIVATE: copy-on-write so the CLR/MAUI can write - // into the image (see the r/w HACK in the uncompressed path) while - // pages stay clean/file-backed until actually modified. + size_t map_size = static_cast(expected_size) + sizeof (CacheFileFooter); + // The runtime may modify the image, so keep those changes private while + // retaining clean file-backed pages until they are actually written. void *mapped = mmap (nullptr, map_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); close (fd); if (mapped == MAP_FAILED) { return nullptr; } - uint64_t stored_token = 0; - memcpy (&stored_token, static_cast(mapped) + expected_size, FOOTER_SIZE); - if (stored_token != token) { + CacheFileFooter footer {}; + memcpy (&footer, static_cast(mapped) + expected_size, sizeof (footer)); + if (footer.magic != CACHE_FILE_MAGIC || + footer.version != CACHE_FILE_FORMAT_VERSION || + footer.store_id != store_id || + footer.descriptor_index != descriptor_index || + footer.payload_size != expected_size || + footer.payload_hash != hash_payload (static_cast(mapped), expected_size)) { munmap (mapped, map_size); + log_debug (LOG_ASSEMBLY, "Ignoring invalid decompressed-assembly cache entry for '{}'"sv, name); return nullptr; } return static_cast(mapped); } - // Queues a freshly decompressed assembly to be persisted. Takes a private - // snapshot of the bytes up front: the caller holds assembly_decompress_mutex - // and has not yet handed the shared buffer to the runtime, so the data is - // still pristine here. Copying now (rather than letting the background - // thread read the shared buffer later) avoids racing with the runtime, - // which may write into the image once the lock is released. - // Must be called while holding assembly_decompress_mutex. - void enqueue_write (std::string_view name, const uint8_t *data, size_t size, uint64_t token) noexcept + void enqueue_write (uint32_t descriptor_index, std::string_view name, const uint8_t *data, size_t size) noexcept { if (!enabled) { return; } - // Build the full on-disk image up front: [payload][8-byte token footer]. - size_t total = size + FOOTER_SIZE; + if (size > SIZE_MAX - sizeof (CacheFileFooter)) { + return; + } + size_t total = size + sizeof (CacheFileFooter); + + size_t bytes_queued = 0; + bool queue_full = false; + { + std::lock_guard lock (state_lock); + if (!writes_enabled) { + return; + } + if (total > MAX_QUEUED_BYTES || queued_bytes > MAX_QUEUED_BYTES - total) { + queue_full = true; + bytes_queued = queued_bytes; + } else { + queued_bytes += total; + } + } + + if (queue_full) { + if (total > MAX_QUEUED_BYTES) { + log_debug ( + LOG_ASSEMBLY, + "Skipping decompressed-assembly cache write for '{}': {} bytes exceed the {}-byte queue limit"sv, + name, + total, + MAX_QUEUED_BYTES + ); + } else { + log_debug ( + LOG_ASSEMBLY, + "Skipping decompressed-assembly cache write for '{}': {} of {} queue bytes are in use"sv, + name, + bytes_queued, + MAX_QUEUED_BYTES + ); + } + return; + } + auto snapshot = std::unique_ptr (new (std::nothrow) uint8_t[total]); if (snapshot == nullptr) { - // Out of memory: skip caching this assembly. Not fatal — the next - // launch simply decompresses it again. + std::lock_guard lock (state_lock); + queued_bytes -= total; return; } + // The runtime can modify the shared decompression buffer after this + // method returns, so the background writer needs an immutable copy. memcpy (snapshot.get (), data, size); - memcpy (snapshot.get () + size, &token, FOOTER_SIZE); + + CacheFileFooter footer { + .magic = CACHE_FILE_MAGIC, + .version = CACHE_FILE_FORMAT_VERSION, + .store_id = store_id, + .payload_hash = hash_payload (snapshot.get (), size), + .descriptor_index = descriptor_index, + .payload_size = static_cast(size), + }; + memcpy (snapshot.get () + size, &footer, sizeof (footer)); WriteRequest req { - .path = build_path (name), + .path = build_path (descriptor_index), .data = std::move (snapshot), .size = total, }; { std::lock_guard lock (state_lock); + if (!writes_enabled) { + queued_bytes -= total; + return; + } + write_queue.push_back (std::move (req)); if (!writer_running) { writer_running = true; - std::thread (writer_loop).detach (); + if (!start_writer_locked ()) { + writer_running = false; + writes_enabled = false; + clear_write_queue_locked (); + } } } - queue_cv.notify_one (); } } // namespace asm_cache } // anonymous namespace @@ -310,7 +495,7 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co #if defined (RELEASE) auto header = reinterpret_cast(e.image_data); if (header->magic == COMPRESSED_DATA_MAGIC) { - log_debug (LOG_ASSEMBLY, "Decompressing assembly '{}' from the assembly store"sv, name); + log_debug (LOG_ASSEMBLY, "Resolving compressed assembly '{}' from the assembly store"sv, name); if (FastTiming::enabled ()) [[unlikely]] { internal_timing.start_event (TimingEventKind::AssemblyDecompression); @@ -361,6 +546,9 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co uint8_t *data_buffer = uncompressed_assemblies_data_buffer + cad.buffer_offset; uint32_t const descriptor_index = header->descriptor_index; + auto is_loaded = [&cad]() noexcept -> bool { + return __atomic_load_n (&cad.loaded, __ATOMIC_ACQUIRE); + }; // Resolves to the mmap'd cache file when this assembly was loaded from // the on-device cache, otherwise to the shared decompression buffer. @@ -371,10 +559,10 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co return data_buffer; }; - if (!cad.loaded) { + if (!is_loaded ()) { StartupAwareLock decompress_lock (assembly_decompress_mutex); - if (cad.loaded) { + if (is_loaded ()) { set_assembly_data_and_size (resolve_data (), cad.uncompressed_file_size, assembly_data, assembly_data_size); if (FastTiming::enabled ()) [[unlikely]] { @@ -388,7 +576,7 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co return {assembly_data, assembly_data_size}; } - asm_cache::ensure_initialized (); + asm_cache::ensure_initialized (assembly_store_content_id); if (header->uncompressed_length != cad.uncompressed_file_size) { if (header->uncompressed_length > cad.uncompressed_file_size) { @@ -408,15 +596,17 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co } const char *data_start = pointer_add(e.image_data, sizeof(CompressedAssemblyHeader)); - uint64_t payload_token = static_cast(crc32_hash (data_start, assembly_data_size)); - uint8_t *cached = asm_cache::try_load (name, cad.uncompressed_file_size, payload_token); + bool loaded_from_cache = false; + uint8_t *cached = asm_cache::try_load (descriptor_index, name, cad.uncompressed_file_size); if (cached != nullptr) { + loaded_from_cache = true; log_debug (LOG_ASSEMBLY, "Loaded decompressed assembly '{}' from the on-device cache"sv, name); if (asm_cache::tracking != nullptr) { asm_cache::tracking[descriptor_index] = cached; } } else { + log_debug (LOG_ASSEMBLY, "Decompressing assembly '{}' from the assembly store"sv, name); size_t ret = ZSTD_decompress (data_buffer, cad.uncompressed_file_size, data_start, assembly_data_size); if (ZSTD_isError (ret)) { @@ -442,13 +632,19 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co ); } - asm_cache::enqueue_write (name, data_buffer, cad.uncompressed_file_size, payload_token); + asm_cache::enqueue_write (descriptor_index, name, data_buffer, cad.uncompressed_file_size); } - cad.loaded = true; + __atomic_store_n (&cad.loaded, true, __ATOMIC_RELEASE); if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); - internal_timing.add_more_info (name); + + dynamic_local_string msg; + msg.append (name); + if (loaded_from_cache) { + msg.append (" (decompressed cache hit)"sv); + } + internal_timing.add_more_info (msg); } } @@ -603,6 +799,7 @@ void AssemblyStore::configure_from_payload (const void *payload_start, const std constexpr size_t header_size = sizeof(AssemblyStoreHeader); + assembly_store_content_id = header->content_id; assembly_store.data_start = static_cast(payload_start); assembly_store.assembly_count = header->entry_count; assembly_store.index_entry_count = header->index_entry_count; @@ -625,4 +822,6 @@ void AssemblyStore::configure_from_payload (const void *payload_start, const std assembly_store_names[i] = std::string_view (reinterpret_cast(names_cursor), name_length); names_cursor += name_length; } + + log_debug (LOG_ASSEMBLY, "Mapped assembly store {}; content ID 0x{:x}"sv, get_full_store_path (), assembly_store_content_id); } diff --git a/src/native/clr/include/host/assembly-store.hh b/src/native/clr/include/host/assembly-store.hh index 973dc37e917..6a529a11376 100644 --- a/src/native/clr/include/host/assembly-store.hh +++ b/src/native/clr/include/host/assembly-store.hh @@ -36,6 +36,7 @@ namespace xamarin::android { // Assembly names indexed by `AssemblyStoreIndexEntry::descriptor_index`, used to disambiguate // CRC32 hash collisions in the store index. Built once when the store is mapped. static inline std::string_view *assembly_store_names = nullptr; + static inline uint64_t assembly_store_content_id = 0; static inline std::mutex assembly_decompress_mutex {}; }; } diff --git a/src/native/clr/include/xamarin-app.hh b/src/native/clr/include/xamarin-app.hh index fd0ea6a12fd..30eed68978c 100644 --- a/src/native/clr/include/xamarin-app.hh +++ b/src/native/clr/include/xamarin-app.hh @@ -137,6 +137,7 @@ struct CompressedAssemblyDescriptor // [ENTRY_COUNT] uint; number of entries in the store // [INDEX_ENTRY_COUNT] uint; number of entries in the index // [INDEX_SIZE] uint; index size in bytes +// [CONTENT_ID] ulong: deterministic hash of everything after the header // // INDEX (variable size, HEADER.ENTRY_COUNT*2 entries, for assembly names with and without the extension) // [NAME_HASH] uint; CRC32 of the assembly name @@ -168,6 +169,7 @@ struct [[gnu::packed]] AssemblyStoreHeader final uint32_t entry_count; uint32_t index_entry_count; uint32_t index_size; // index size in bytes + uint64_t content_id; }; struct [[gnu::packed]] AssemblyStoreIndexEntry final @@ -232,6 +234,7 @@ struct ApplicationConfig const char *android_package_name; bool managed_marshal_methods_lookup_enabled; bool have_assembly_store; + bool assembly_store_decompression_cache_enabled; }; struct DSOCacheEntry diff --git a/src/native/clr/xamarin-app-stub/application_dso_stub.cc b/src/native/clr/xamarin-app-stub/application_dso_stub.cc index 3d2f2eae958..147ea889d0d 100644 --- a/src/native/clr/xamarin-app-stub/application_dso_stub.cc +++ b/src/native/clr/xamarin-app-stub/application_dso_stub.cc @@ -69,6 +69,7 @@ const ApplicationConfig application_config = { .android_package_name = android_package_name, .managed_marshal_methods_lookup_enabled = false, .have_assembly_store = false, + .assembly_store_decompression_cache_enabled = false, }; // TODO: migrate to std::string_view for these two diff --git a/src/native/mono/xamarin-app-stub/xamarin-app.hh b/src/native/mono/xamarin-app-stub/xamarin-app.hh index 2a21c1e827b..1a912d272d5 100644 --- a/src/native/mono/xamarin-app-stub/xamarin-app.hh +++ b/src/native/mono/xamarin-app-stub/xamarin-app.hh @@ -34,7 +34,7 @@ static constexpr uint32_t ASSEMBLY_STORE_ABI = 0x00040000; #endif // Increase whenever an incompatible change is made to the assembly store format -static constexpr uint32_t ASSEMBLY_STORE_FORMAT_VERSION = 3 | ASSEMBLY_STORE_64BIT_FLAG | ASSEMBLY_STORE_ABI; +static constexpr uint32_t ASSEMBLY_STORE_FORMAT_VERSION = 4 | ASSEMBLY_STORE_64BIT_FLAG | ASSEMBLY_STORE_ABI; static constexpr uint32_t MODULE_MAGIC_NAMES = 0x53544158; // 'XATS', little-endian static constexpr uint32_t MODULE_INDEX_MAGIC = 0x49544158; // 'XATI', little-endian @@ -145,6 +145,7 @@ struct XamarinAndroidBundledAssembly // [ENTRY_COUNT] uint; number of entries in the store // [INDEX_ENTRY_COUNT] uint; number of entries in the index // [INDEX_SIZE] uint; index size in bytes +// [CONTENT_ID] ulong: deterministic hash of everything after the header // // INDEX (variable size, HEADER.ENTRY_COUNT*2 entries, for assembly names with and without the extension) // [NAME_HASH] uint on 32-bit platforms, ulong on 64-bit platforms; xxhash of the assembly name @@ -176,6 +177,7 @@ struct [[gnu::packed]] AssemblyStoreHeader final uint32_t entry_count; uint32_t index_entry_count; uint32_t index_size; // index size in bytes + uint64_t content_id; }; struct [[gnu::packed]] AssemblyStoreIndexEntry final diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs index 27bcdd668b7..d02d09888f3 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs @@ -660,6 +660,76 @@ public void DeployToDevice ([Values] bool isRelease, [Values (AndroidRuntime.Cor Assert.IsTrue (didLaunch, "Activity should have started."); } + [Test] + public void AssemblyStoreDecompressionCacheMapsPersistedAssemblies () + { + if (IgnoreUnsupportedConfiguration (AndroidRuntime.CoreCLR, release: true)) { + return; + } + + var app = new XamarinAndroidApplicationProject (packageName: PackageUtils.MakePackageName (AndroidRuntime.CoreCLR, "assemblycache")) { + IsRelease = true, + }; + app.SetRuntime (AndroidRuntime.CoreCLR); + app.SetRuntimeIdentifiers (new [] { DeviceAbi }); + app.SetProperty ("AndroidEnableAssemblyStoreDecompressionCache", "true"); + app.AndroidManifest = app.AndroidManifest.Replace (" (uint)(5 * sizeof (uint) + ((version & ASSEMBLY_STORE_FORMAT_NUMBER_MASK) >= 4 ? sizeof (ulong) : 0)); - public Header (uint magic, uint version, uint entry_count, uint index_entry_count, uint index_size) + public Header (uint magic, uint version, uint entry_count, uint index_entry_count, uint index_size, ulong content_id) { this.magic = magic; this.version = version; this.entry_count = entry_count; this.index_entry_count = index_entry_count; this.index_size = index_size; + this.content_id = content_id; } } diff --git a/tools/assembly-store-reader-mk2/AssemblyStore/StoreReader_V2.cs b/tools/assembly-store-reader-mk2/AssemblyStore/StoreReader_V2.cs index f31e44c7a70..d8de129f727 100644 --- a/tools/assembly-store-reader-mk2/AssemblyStore/StoreReader_V2.cs +++ b/tools/assembly-store-reader-mk2/AssemblyStore/StoreReader_V2.cs @@ -16,6 +16,7 @@ partial class StoreReader_V2 : AssemblyStoreReader const uint ASSEMBLY_STORE_FORMAT_VERSION_CORECLR_64BIT = 0x80000004; // Must match the ASSEMBLY_STORE_FORMAT_VERSION native constant const uint ASSEMBLY_STORE_FORMAT_VERSION_CORECLR_32BIT = 0x00000004; const uint ASSEMBLY_STORE_FORMAT_VERSION_MASK = 0xF0000000; + const uint ASSEMBLY_STORE_FORMAT_NUMBER_MASK = 0x0000FFFF; const uint ASSEMBLY_STORE_ABI_AARCH64 = 0x00010000; const uint ASSEMBLY_STORE_ABI_ARM = 0x00020000; @@ -130,8 +131,9 @@ protected override bool IsSupported () uint entry_count = reader.ReadUInt32 (); uint index_entry_count = reader.ReadUInt32 (); uint index_size = reader.ReadUInt32 (); + ulong content_id = (version & ASSEMBLY_STORE_FORMAT_NUMBER_MASK) >= 4 ? reader.ReadUInt64 () : 0; - header = new Header (magic, version, entry_count, index_entry_count, index_size); + header = new Header (magic, version, entry_count, index_entry_count, index_size, content_id); return true; } @@ -153,7 +155,7 @@ protected override void Prepare () AssemblyCount = header.entry_count; IndexEntryCount = header.index_entry_count; - StoreStream.Seek ((long)elfOffset + Header.NativeSize, SeekOrigin.Begin); + StoreStream.Seek ((long)elfOffset + header.NativeSize, SeekOrigin.Begin); using var reader = CreateReader (); uint indexEntrySize = GetIndexEntrySize (); From 6424e954f4d162d2cf8d086087b3857fefa05b0e Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 15 Jul 2026 17:32:27 +0200 Subject: [PATCH 6/9] [assembly-store] Remove redundant writer.Flush before ComputeContentId 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 --- .../Utilities/AssemblyStoreGenerator.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs index a8644b10d5f..8b80700c934 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs @@ -187,7 +187,6 @@ string Generate (string baseOutputDirectory, AndroidTargetArch arch, List Date: Wed, 15 Jul 2026 18:46:11 +0200 Subject: [PATCH 7/9] [assembly-store] Address code review feedback - 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.`) 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 --- .../Utilities/AssemblyStoreGenerator.cs | 4 ++-- src/native/clr/host/assembly-store.cc | 11 ++--------- .../Tests/InstallAndRunTests.cs | 5 ++++- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs index 8b80700c934..1241a805f25 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs @@ -55,8 +55,8 @@ partial class AssemblyStoreGenerator const uint ASSEMBLY_STORE_MAGIC = 0x41424158; // 'XABA', little-endian, must match the BUNDLED_ASSEMBLIES_BLOB_MAGIC native constant // Bit 31 is set for 64-bit platforms, cleared for the 32-bit ones - const uint ASSEMBLY_STORE_FORMAT_VERSION_MONOVM_64BIT = 0x80000003; // Must match the ASSEMBLY_STORE_FORMAT_VERSION native constant - const uint ASSEMBLY_STORE_FORMAT_VERSION_MONOVM_32BIT = 0x00000003; + const uint ASSEMBLY_STORE_FORMAT_VERSION_MONOVM_64BIT = 0x80000004; // Must match the ASSEMBLY_STORE_FORMAT_VERSION native constant + const uint ASSEMBLY_STORE_FORMAT_VERSION_MONOVM_32BIT = 0x00000004; const uint ASSEMBLY_STORE_FORMAT_VERSION_CORECLR_64BIT = 0x80000004; // Must match the ASSEMBLY_STORE_FORMAT_VERSION native constant const uint ASSEMBLY_STORE_FORMAT_VERSION_CORECLR_32BIT = 0x00000004; diff --git a/src/native/clr/host/assembly-store.cc b/src/native/clr/host/assembly-store.cc index 90e6bf824f2..8bcaacd5b41 100644 --- a/src/native/clr/host/assembly-store.cc +++ b/src/native/clr/host/assembly-store.cc @@ -74,7 +74,6 @@ namespace { enum class WriteResult { Succeeded, - Skipped, Failed, }; @@ -122,7 +121,8 @@ namespace { auto write_cache_file (WriteRequest const& req) noexcept -> WriteResult { std::string tmp_path = req.path; - tmp_path.append (".tmp"sv); + tmp_path.append (".tmp."sv); + tmp_path.append (std::to_string (getpid ())); int fd; do { @@ -153,13 +153,6 @@ namespace { if (rename_result != 0) { error = errno; - // Another process can publish the shared, content-identical temp - // file first. Its rename consumes the temp path, so this writer - // has nothing left to publish. - if (error == ENOENT) { - return WriteResult::Skipped; - } - log_file_error ("publish"sv, req.path, error); unlink (tmp_path.c_str ()); return WriteResult::Failed; diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs index d02d09888f3..1d81abde3e7 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs @@ -695,7 +695,10 @@ public void AssemblyStoreDecompressionCacheMapsPersistedAssemblies () Thread.Sleep (250); cacheFiles = RunAdbCommand ( $"shell run-as {app.PackageName} find code_cache/decompressed-assembly-cache-v1 -type f -name '*.bin'" - ).Split (new [] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); + ) + .Split (new [] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries) + .Where (line => line.EndsWith (".bin", StringComparison.Ordinal)) + .ToArray (); } Assert.That (cacheFiles.Length, Is.GreaterThanOrEqualTo (2), "The first launch should persist multiple decompressed assemblies."); From 1811d9040de69073fc88372bcf3b4f2e430612b1 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 15 Jul 2026 18:46:21 +0200 Subject: [PATCH 8/9] [assembly-store] Fix CoreCLR application_config field count in tests 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 --- .../Utilities/EnvironmentHelper.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs index 2cdc0e19b03..8c511a1311f 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs @@ -65,7 +65,7 @@ public sealed class ApplicationConfig_CoreCLR : IApplicationConfig public bool assembly_store_decompression_cache_enabled; } - const uint ApplicationConfigFieldCount_CoreCLR = 20; + const uint ApplicationConfigFieldCount_CoreCLR = 21; // This must be identical to the ApplicationConfig structure in src/native/mono/xamarin-app-stub/xamarin-app.hh public sealed class ApplicationConfig_MonoVM : IApplicationConfig @@ -409,7 +409,7 @@ static IApplicationConfig ReadApplicationConfig_CoreCLR (EnvironmentFile envFile ret.have_assembly_store = ConvertFieldToBool ("have_assembly_store", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]); break; - case 21: // assembly_store_decompression_cache_enabled: bool / .byte + case 20: // 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; From 2405abab2756382f4643f3dda213be7832169983 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 15 Jul 2026 23:32:17 +0200 Subject: [PATCH 9/9] [assembly-store] Address decompression cache review feedback Two follow-ups from PR review: * Reclaim stale staging files. If Android kills the detached cache writer between creating a `.bin.tmp.` file and renaming it into place, the temp file is never reclaimed and repeated startup kills could accumulate partial files outside the 32 MiB queue bound. Sweep and unlink leftover `.tmp.*` files when initializing the store cache directory. * Strengthen the corruption-fallback device test. Previously it only checked that some cache entry was mapped after corrupting one file, which passes even if the corrupted entry is wrongly accepted. Now it hashes the target file, confirms corruption changed it, and asserts the exact corrupted file is rewritten with its original valid contents (deterministic decompress + footer) after fallback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 31cb6076-baba-4b69-a86f-114e108fe8c4 --- src/native/clr/host/assembly-store.cc | 29 +++++++++++++++++++ .../Tests/InstallAndRunTests.cs | 19 ++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/native/clr/host/assembly-store.cc b/src/native/clr/host/assembly-store.cc index 8bcaacd5b41..3b34cf87092 100644 --- a/src/native/clr/host/assembly-store.cc +++ b/src/native/clr/host/assembly-store.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -253,6 +254,32 @@ namespace { return true; } + // Best-effort removal of staging files left behind by a previous process whose writer was + // killed (e.g. by Android) between creating a `.tmp.` file and renaming it into place. + // Such files are never reclaimed otherwise and would accumulate outside the queue bound. + void remove_stale_temp_files (std::string const& dir) noexcept + { + DIR *handle = opendir (dir.c_str ()); + if (handle == nullptr) { + return; + } + + std::string prefix = dir; + prefix.append ("/"); + for (dirent *entry = readdir (handle); entry != nullptr; entry = readdir (handle)) { + std::string_view name { entry->d_name }; + if (name.find (".tmp."sv) == std::string_view::npos) { + continue; + } + + std::string path = prefix; + path.append (name); + unlink (path.c_str ()); + } + + closedir (handle); + } + void ensure_initialized (uint64_t assembly_store_id) noexcept { if (initialized) { @@ -302,6 +329,8 @@ namespace { return; } + remove_stale_temp_files (cache_dir); + if (compressed_assembly_count > 0) { tracking.reset (new (std::nothrow) uint8_t*[compressed_assembly_count]()); } diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs index 1d81abde3e7..6bfdb7d5b63 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs @@ -704,9 +704,18 @@ public void AssemblyStoreDecompressionCacheMapsPersistedAssemblies () RunAdbCommand ($"shell am force-stop --user all {app.PackageName}"); string cacheFileToCorrupt = cacheFiles.First (); + string ValidFileHash () => RunAdbCommand ( + $"shell run-as {app.PackageName} md5sum {cacheFileToCorrupt}" + ).Split (new [] { ' ', '\r', '\n', '\t' }, StringSplitOptions.RemoveEmptyEntries).FirstOrDefault () ?? ""; + + string validHash = ValidFileHash (); + Assert.That (validHash, Is.Not.Empty, $"Should be able to hash the persisted cache file '{cacheFileToCorrupt}'."); + RunAdbCommand ( $"shell run-as {app.PackageName} dd if=/dev/zero of={cacheFileToCorrupt} bs=1 count=1 conv=notrunc" ); + Assert.That (ValidFileHash (), Is.Not.EqualTo (validHash), "Corrupting the cache file should change its contents."); + ClearAdbLogcat (); AdbStartActivity ($"{app.PackageName}/{app.JavaPackageName}.MainActivity"); Assert.IsTrue ( @@ -719,6 +728,16 @@ public void AssemblyStoreDecompressionCacheMapsPersistedAssemblies () "Second launch should succeed." ); + // A corrupted entry must be rejected (footer hash mismatch) and re-decompressed, which + // re-persists a byte-identical file. Verify the *exact* corrupted file is healed rather + // than merely checking that some other valid entry is still mapped. + bool rewritten = false; + for (int attempt = 0; attempt < 40 && !rewritten; attempt++) { + Thread.Sleep (250); + rewritten = ValidFileHash () == validHash; + } + Assert.IsTrue (rewritten, $"The corrupted cache file '{cacheFileToCorrupt}' should be rewritten with valid contents after fallback."); + string [] pids = RunAdbCommand ($"shell pidof {app.PackageName}") .Split (new [] { ' ', '\r', '\n', '\t' }, StringSplitOptions.RemoveEmptyEntries); Assert.IsNotEmpty (pids, "The application process should be running after the second launch.");