[experiment] On-device decompressed AssemblyStore cache (CoreCLR)#11967
[experiment] On-device decompressed AssemblyStore cache (CoreCLR)#11967simonrozsival wants to merge 5 commits into
Conversation
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>
7cac3de to
9d831a5
Compare
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>
9d831a5 to
f1de798
Compare
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>
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.
|
|
||
| bool ok = write_fully (fd, req.data.get (), req.size); | ||
| if (ok) { | ||
| fsync (fd); |
There was a problem hiding this comment.
🤖 ❌ Resource management — fsync() is part of the publication contract here, but its result is discarded. A successful write() only means the bytes reached the page cache; fsync() can fail with EINTR, ENOSPC, or EIO, after which this still renames and publishes the file. Because the footer authenticates the compressed source rather than the cached payload, a damaged payload can still pass the current size/token checks on the next launch. Retry EINTR, fold the final fsync() result into ok, and only rename after it succeeds.
Rule: Check return codes on platform APIs (Postmortem #8)
| std::string path = cache_dir; | ||
| path.append ("/"); | ||
| path.append (name); | ||
| return path; |
There was a problem hiding this comment.
🤖 name is not guaranteed to be a flat filename. Satellite entries are stored and probed as culture/Assembly.resources.dll (see AssemblyStoreAssemblyInfo), but only the top-level cache directory is created, so both cache reads and writes for those assemblies fail with ENOENT. Treating the probe name as a path also admits separators and length/collision edge cases. Please key files by a path-safe invariant such as the descriptor index plus token, rather than the raw assembly name.
Rule: Path traversal / collision-proof names
|
|
||
| { | ||
| std::lock_guard lock (state_lock); | ||
| write_queue.push_back (std::move (req)); |
There was a problem hiding this comment.
🤖 fsync per assembly. Startup can enqueue faster than storage drains, transiently duplicating the entire touched assembly working set and generating many durability flushes; the detached worker then remains alive for the process lifetime. Before enabling this by default, bound or batch the persistence work and measure first-run peak RSS, bytes/fsync count, completion time, and energy—not only subsequent-launch latency.
Rule: Quantify process-lifetime resource costs (Postmortem #12)
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
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.rename. 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: