[CoreCLR] Remove assembly store decompression cache - #12780
simonrozsival wants to merge 1 commit into
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>
simonrozsival
added
the
ready-to-review
|
/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!
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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.
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