Repository navigation
[CoreCLR] Remove assembly store decompression cache - #12780
Conversation
The cache regresses startup in current CoreCLR Release builds and adds persistence, validation, and configuration complexity. Restore direct Zstd decompression and assembly store format version 3. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/review |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Add CoreCLR reader coverage and MonoVM regression coverage before approval.
Get a fresh assessment by requesting another Copilot review.
Review tier: Lite
Findings: 1
Open findings (1)
What changed in this PR
Removes the CoreCLR decompressed assembly-store cache and restores assembly-store format version 3 across CoreCLR and MonoVM.
Changes:
- Removes cache persistence, native handling, configuration, startup plumbing, and device coverage.
- Removes content IDs and updates generators, runtimes, readers, and headers.
- Updates documentation and related tests.
Outstanding review items:
- Add CoreCLR v3 reader fixture coverage.
- Add MonoVM coverage to
CreateAssemblyStoreTests.
| File | Summary |
|---|---|
tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs |
Removes cache integration coverage. |
src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets |
Removes cache MSBuild plumbing. |
src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs |
Emits version-3 headers without content IDs. |
src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.Classes.cs |
Updates managed header layout. |
src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGeneratorCLR.cs |
Removes cache configuration emission. |
src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigCLR.cs |
Removes the cache configuration field. |
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs |
Updates configuration parsing expectations. |
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateNativeApplicationConfigSourcesTests.cs |
Removes cache configuration tests. |
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/CreateAssemblyStoreTests.cs |
Validates version-3 header layout. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs |
Removes the cache task property. |
src/native/mono/xamarin-app-stub/xamarin-app.hh |
Restores MonoVM format version 3. |
src/native/clr/xamarin-app-stub/application_dso_stub.cc |
Updates the CLR configuration stub. |
src/native/clr/include/xamarin-app.hh |
Restores CoreCLR format version 3. |
src/native/clr/include/runtime-base/android-system.hh |
Removes code-cache state. |
src/native/clr/include/host/assembly-store.hh |
Removes content-ID state. |
src/native/clr/include/constants.hh |
Removes the code-cache directory index. |
src/native/clr/host/host.cc |
Removes code-cache initialization. |
src/native/clr/host/assembly-store.cc |
Removes cache implementation. |
src/java-runtime/java/mono/android/clr/MonoPackageManager.java |
Removes code-cache path plumbing. |
Documentation/project-docs/AssemblyStores.md |
Documents the version-3 header format. |
Documentation/building/configuration.md |
Removes the obsolete cache property. |
.github/skills/read-assembly-store/src/AssemblyStore/StoreReader_V2.cs |
Removes version-4 and content-ID handling. |
.github/skills/read-assembly-store/src/AssemblyStore/StoreReader_V2.Classes.cs |
Updates reader header definitions. |
| { | ||
| [Test] | ||
| public void ContentIdMatchesStoreContents () | ||
| public void CoreCLRStoreUsesVersionThreeHeader () |
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
⚠️ Needs Changes
Findings: 0 errors · 1 warning · 0 suggestions
The runtime, generator, application-config, Java bootstrap, tests, and documentation consistently remove the decompression cache and restore the v3 wire layout. The benchmark rationale is clear, and the cache-specific plumbing is removed comprehensively.
The assembly-store inspection skill should retain read compatibility with v4 artifacts already produced by local and CI builds; I left that as an inline warning. I did not duplicate the existing review comment requesting MonoVM generator coverage.
CI is still running: the completed Android Tools and Java.Interop lanes plus license/cla are green, while the Linux, macOS, and Windows build lanes and aggregate pipeline have not completed.
Generated by Android PR Reviewer for #12780 · copilot · gpt56 · 156.4 AIC · ⌖ 11.3 AIC · ⊞ 21K
Comment /review to run again
| - **CoreCLR** (store format version `4`): the hash is a 32-bit | ||
| - **CoreCLR** (store format version `3`): the hash is a 32-bit |
There was a problem hiding this comment.
Should we increment to version 5, or are we sure it was the same as before?
There was a problem hiding this comment.
It's the same it was before. It was disabled so nobody used it anyway. I would treat v4 as never shipped.
|
Test failure is caused by a bug in MAUI: dotnet/maui#38538 |
dotnet/android#12780 removed the opt-in decompression cache and restored assembly store format version 3 for CoreCLR, so v4 only ever existed in .NET 11 previews. Correct the vendored file headers and ATTRIBUTION, which claimed v4 ships in .NET 11. .NET 11 GA emits v3 with CoreCLR's 32-bit CRC32 name hashes on 64-bit ABIs - a shape the synthetic store tests did not cover. Add it. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* feat: Accept Android assembly store v4 versions and read content_id header v4 stores (CoreCLR, .NET 11) add a content_id field to the header, so the index starts 8 bytes later. Header.NativeSize is now derived from the format number rather than being a constant. Refs #5454 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: Decompress Zstandard-compressed Android assemblies .NET 11 compresses assemblies with Zstandard (XAZS) instead of LZ4 (XALZ). The 12-byte header is unchanged, so only the magic and the codec differ. Zstandard uses the BCL ZstandardDecoder on net11.0; the net10.0 build throws NotSupportedException, since only .NET 11 apps produce XAZS. Refs #5346 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Accept API verifier changes * Accept API verifier changes * feat: Derive Android assembly store index entry size from the header The index entry layout was inferred from the ABI bitness (plus a compile-time TFM check for the ignore flag). CoreCLR v4 stores use 32-bit CRC32 name hashes on every ABI, so on 64-bit ABIs the reader read the index 4 bytes per entry too far and failed in Prepare(). The entry size is now index_size / index_entry_count, and anything other than the two v3/v4 layouts is rejected as corrupt. With this, .NET 11 APKs are readable, so the net11 APK tests are re-enabled. Refs #5454 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: Refresh Android assembly reader upstream attribution Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: Don't compile ArchiveUtilsTests for Android Assembly.Location is empty on Android, so the static initializer threw and every test in the class failed on the device runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: Trim comments Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: Resolve debug images for Android assemblies without a file location On .NET 11 (CoreCLR) Android, assemblies loaded from the assembly store report Module.FullyQualifiedName as <Unknown>, so DebugStackTrace bailed out before calling the assembly reader and events had no debug images. When an assembly reader is configured, fall back to Module.ScopeName, which is the file name the store is indexed by. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: Trim comments Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: Decompress Zstandard-compressed Android assemblies .NET 11 compresses assemblies with Zstandard (XAZS) instead of LZ4 (XALZ). The 12-byte header is unchanged, so only the magic and the codec differ. Zstandard uses the BCL ZstandardDecoder on net11.0; the net10.0 build throws NotSupportedException, since only .NET 11 apps produce XAZS. Refs #5346 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: Don't compile ArchiveUtilsTests for Android Assembly.Location is empty on Android, so the static initializer threw and every test in the class failed on the device runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: Trim comments Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: Address review feedback on Zstandard decompression Magic numbers become internal constants the tests share, and the DecompressZstandard helper is inlined - its assemblyName parameter was unused on .NET 11. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: Address review feedback on .NET 11 assembly store support Names the magic values in the test fixtures, shares the format number mask with the reader, skips the output buffer allocation when a Zstandard payload can't be decompressed, and logs why reading an assembly failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ref: Record that assembly store v4 never shipped in .NET 11 (#5609) dotnet/android#12780 removed the opt-in decompression cache and restored assembly store format version 3 for CoreCLR, so v4 only ever existed in .NET 11 previews. Correct the vendored file headers and ATTRIBUTION, which claimed v4 ships in .NET 11. .NET 11 GA emits v3 with CoreCLR's 32-bit CRC32 name hashes on 64-bit ABIs - a shape the synthetic store tests did not cover. Add it. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>

The optional decompressed assembly-store cache was introduced in #11967 after an early prototype showed promise, improving mean startup by about 49 ms. Those measurements predated the final cache format, integrity scan, and bounded background writer, and the original PR noted that the completed implementation still needed to be remeasured.
Remeasuring the current implementation on CoreCLR + trimmable typemap + Release builds shows the opposite result:
These results cover 40 cache-warm launches per variant across two balanced rounds. Native timing attributed roughly 173 ms to the cache-hit path versus 48 ms for direct Zstd decompression.
The cache therefore no longer delivers its intended startup speedup and instead adds persistence, mapping, integrity-validation, storage, testing, and configuration complexity. Since it is opt-in and the associated store format was never released, this removes the feature rather than carrying that complexity forward.
This change:
AndroidEnableAssemblyStoreDecompressionCacheand its application-config plumbingValidation:
dotnet build src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csprojCreateAssemblyStoreTestsandGenerateNativeApplicationConfigSourcesTests.github/skills/read-assembly-store/tests/AssemblyStore.Tests/AssemblyStore.Tests.csprojonnet10.0andnet11.0