Replace AssemblyStore compression algorithm LZ4 with Zstd#11730
Replace AssemblyStore compression algorithm LZ4 with Zstd#11730Copilot wants to merge 9 commits into
Conversation
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Refresh BuildReleaseArm64 CoreCLR APK descriptors from PR CI after switching assembly-store compression to Zstd. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Switch the net10.0 diagnostic tools from DllImport to source-generated LibraryImport for Zstd decompression entry points. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Retarget the diagnostic tools to net11.0 so they can use System.IO.Compression.ZstandardDecoder instead of native Zstd P/Invokes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Remove the small decompress-assemblies helper and call ZstandardDecoder.TryDecompress directly at the compressed payload site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
Pull request overview
This PR migrates the AssemblyStore per-assembly compression pipeline from LZ4 to Zstandard (Zstd), removing the managed LZ4 dependency and updating managed tooling + native runtimes to read/decompress the new payloads.
Changes:
- Switched build-time assembly compression to Zstd and updated compressed blob magic (
XALZ→XAZS) and intermediate file extension (.lz4→.zst). - Updated MonoVM/CoreCLR native runtime code paths to decompress assemblies via
ZSTD_decompressfromlibSystem.IO.Compression.Native, and removed the internal LZ4 native library wiring. - Updated diagnostic tools (
tmt,decompress-assemblies) to detect the new magic and decompress usingSystem.IO.Compression.ZstandardDecoder; refreshed APK descriptor baselines.
Show a summary per file
| File | Description |
|---|---|
| tools/tmt/tmt.csproj | Moves tool to $(DotNetTargetFramework) and removes K4os LZ4 dependency. |
| tools/tmt/ApkManagedTypeResolver.cs | Updates magic and decompression implementation to Zstd decoder. |
| tools/decompress-assemblies/main.cs | Updates magic and decompression implementation to Zstd decoder. |
| tools/decompress-assemblies/decompress-assemblies.csproj | Moves tool to $(DotNetTargetFramework) and removes K4os LZ4 dependency. |
| tools/assembly-store-reader/assembly-store-reader.csproj | Removes K4os LZ4 package reference. |
| tools/assembly-store-reader-mk2/assembly-store-reader.csproj | Removes K4os LZ4 package reference. |
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Routes compression intermediate output to android\\zstd\\ and updates target comment. |
| src/Xamarin.Android.Build.Tasks/Utilities/Zstd.cs | Introduces managed P/Invoke wrapper for Zstd entry points in System.IO.Compression.Native. |
| src/Xamarin.Android.Build.Tasks/Utilities/NativeRuntimeComponents.cs | Drops libxa-lz4 archive from runtime component list. |
| src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreAssemblyInfo.cs | Updates filename normalization for .zst compressed artifacts. |
| src/Xamarin.Android.Build.Tasks/Utilities/AssemblyCompression.cs | Uses Zstd wrapper for compression and emits .zst outputs + new magic. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.CoreCLR.apkdesc | Updates expected APK contents/sizes after runtime + store changes. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc | Updates expected APK contents/sizes after runtime + store changes. |
| src/Xamarin.Android.Build.Tasks/Tasks/CompressAssemblies.cs | Updates task summary comment to reflect Zstd. |
| src/native/native.targets | Removes LZ4 CMake and source inputs from native build wiring. |
| src/native/mono/xamarin-app-stub/xamarin-app.hh | Updates COMPRESSED_DATA_MAGIC constant to XAZS. |
| src/native/mono/monodroid/embedded-assemblies.cc | Switches runtime decompression from LZ4 to Zstd and removes HAVE_LZ4 gating. |
| src/native/mono/monodroid/CMakeLists.txt | Removes HAVE_LZ4 define and links -lSystem.IO.Compression.Native. |
| src/native/common/lz4/CMakeLists.txt | Deletes internal LZ4 static library definition. |
| src/native/common/include/runtime-base/zstd.hh | Adds minimal Zstd entry point declarations for native runtime usage. |
| src/native/common/include/runtime-base/timing-internal.hh | Updates timing description string to “Zstd decompression”. |
| src/native/CMakeLists.txt | Removes add_subdirectory(common/lz4). |
| src/native/clr/include/xamarin-app.hh | Updates COMPRESSED_DATA_MAGIC constant to XAZS. |
| src/native/clr/host/CMakeLists.txt | Removes HAVE_LZ4 define and links -lSystem.IO.Compression.Native. |
| src/native/clr/host/assembly-store.cc | Switches runtime decompression from LZ4 to Zstd and removes HAVE_LZ4 gating. |
| Configuration.props | Removes LZ4 source directory/full-path properties. |
Copilot's findings
- Files reviewed: 26/26 changed files
- Comments generated: 6
There was a problem hiding this comment.
🤖 Android PR Review — Replace AssemblyStore LZ4 with Zstd
Overall this is a clean, well-executed change. I formed an independent assessment and verified the load-bearing assumptions:
- ✅ The ZSTD exports are real on .NET 11.
libSystem.IO.Compression.Nativein the .NET 11 runtime pack exports the raw, unprefixedZSTD_*entry points — dotnet/runtime's ownInterop.Zstd.csP/Invokes the same symbols fromSystem.IO.Compression.Native, andSystem.IO.Compression.ZstandardDecoderexists. The TFM bumps fordecompress-assemblies/tmt(DotNetStableTargetFramework→DotNetTargetFramework) are required for that decoder and are correct. (Note: this firmly ties the feature to .NET 11+; .NET 10 does not export these.) - ✅ Magic value change is correct and consistent.
XALZ(0x5A4C4158) →XAZS(0x535A4158), applied across the managed writer, both native headers (CLR + mono stub), and the tools. The 12-byte header layout is unchanged. - ✅ Native decompression is sound. Both
assembly-store.cc(CLR) andembedded-assemblies.cc(Mono) feeddata_size - sizeof(CompressedAssemblyHeader)as the compressed size, checkZSTD_isError, surfaceZSTD_getErrorName, and validate the produced size againstcad.uncompressed_file_size. The#if defined(HAVE_LZ4) && defined(RELEASE)→#if defined(RELEASE)guard change tracks theHAVE_LZ4removal. - ✅ LZ4 removal is clean. No dangling
K4os/LZ4Codecreferences remain in the changed scope; theassembly-store-reader*tools only had unused package refs removed, andexternal/lz4is correctly retained for FastDev.
A few non-blocking 💡 suggestions (posted inline on Zstd.cs):
- Add a fast round-trip unit test for the new
Zstdinterop so an ABI break is caught quickly in CI rather than only in RELEASE apkdesc/device runs. - Check the
ZSTD_CCtx_setParameterreturn value so a failed level-set doesn't silently fall back to the default level. - Use a file-scoped namespace in the new file to match its siblings.
- Confirm the build-time cost of the ultra compression level (22) on the Release hot path.
CI: builds were green / in-progress at review time with no failures attributable to these changes; the documented Release size-threshold artifacts reflect the smaller libassembly-store.so, not build breaks. Worth a final green CI before merge.
Nice work — the approach is correct and the diff is tidy. None of the above is blocking.
Generated by Android PR Reviewer for issue #11730 · 1.7K AIC · ⌖ 48.6 AIC · ⊞ 37.9K
Comment /review to run again
|
@copilot address code review - let's reduce the quality level to reduce time needed for decompression. I also like the suggestions regarding testing and code organization. Make sure you address all of the comments. |
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Addressed in commit 78d6a04. Implemented all review-thread fixes:
Validation run: affected projects build successfully; targeted |
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
@copilot Solid, well-scoped swap from LZ4 to Zstandard — and nice hardening in response to the earlier review round (Cdecl on the P/Invokes, -1/EncodingFailed instead of crashing on missing exports, pooled-buffer return + ReadFully).
Verified
- Header layout unchanged; the magic/extension swap (
XALZ→XAZS,0x5A4C4158→0x535A4158,.lz4→.zst) is applied consistently acrossAssemblyCompression.cs, the CLR/Mono native readers (assembly-store.cc,embedded-assemblies.cc), and thetmt/decompress-assembliestools. No staleXALZ/LZ4 references remain outside the intentionally-retained fastdev path. - Both native readers map
ZSTD_decompress (dst, dstCapacity = uncompressed_length, src, srcSize = compressed_size)correctly and checkZSTD_isErrorbefore use. - The managed
ZstdP/Invokes and the native-lSystem.IO.Compression.Nativelink target the raw, unprefixedZSTD_*exports of the runtime pack — matching how dotnet/runtime's ownInterop.Zstdbinds them — so the mechanism is sound.ZstandardDecoder.TryDecompress(net11) is used correctly, and thenetstandard2.0build-task wrapper is justified (the BCL Zstandard types aren't reachable fromnetstandard2.0).
Comments
⚠️ Inline (Zstd.cs): theCompressXML doc still says "maximum compression level" while the code uses level 3 (zstd's default). Please reconcile the doc with the chosen level.- 💡 I couldn't find a fast managed round-trip test (
Zstd.Compress→ZstandardDecoder.TryDecompress) — re-raising the earlier suggestion, since binding to internal runtime-pack exports means an ABI break would otherwise only surface late in a Release apkdesc or device run.
CI: Azure DevOps build 1482888 is still in_progress across all 38 checks, so this isn't validated green yet — worth confirming the Release apkdesc diffs and device suites pass before merge.
Submitting as comments only (not approving, per review policy).
Generated by Android PR Reviewer for issue #11730 · 1.9K AIC · ⌖ 48 AIC · ⊞ 40K
Comment /review to run again
The size-regression baselines committed for the Zstd switch were generated in a different environment and did not match the artifacts produced by CI, causing BuildReleaseArm64 (CoreCLR) to fail the apkdiff regression check. Replace the Simple and XForms CoreCLR .apkdesc files with the descriptors produced by the failing CI run (build 1482888). Compared to the LZ4 baselines on main, Zstd still shrinks libassembly-store.so (Simple: 3,461,344 -> 2,903,016; XForms: 14,137,920 -> 11,658,648); the previous committed values were simply too optimistic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The XML doc said compression used "the maximum compression level," but Compress sets ZstdCompressionLevel = 3, which is zstd's default level (not its maximum, 22). This was a leftover from an earlier ZSTD_maxCLevel () version. Update the doc to describe the actual behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Switches the per-assembly AssemblyStore compression from LZ4 (dotnet/lz4 + the K4os managed wrapper) to Zstandard, using the
ZSTD_*entry points already exported bylibSystem.IO.Compression.Nativein the .NET runtime pack. This drops an external dependency and trades for a better compression ratio.The 12-byte compressed-blob header (magic / descriptor index / uncompressed length) is unchanged; only the algorithm and magic value change. No
FORMAT_VERSIONbump is needed since the store and runtime are always built together by the same SDK.Managed (build-time)
Zstd.csP/Invoke wrapper overlibSystem.IO.Compression.Native(ZSTD_compress2via aZSTD_CCtx) using zstd's default compression level (3).AssemblyCompression.cscompresses throughZstdinstead ofLZ4Codec; intermediate output extension.lz4→.zst.CallingConvention.CdeclonZSTD_*imports.ZSTD_*error returns.MaximumOutputSize) and controlled encoding failure paths.Magic value
COMPRESSED_DATA_MAGIC/ managic constantXALZ(0x5A4C4158) →XAZS(0x535A4158), applied consistently across managed code, both native headers, and the diagnostic tools.Native (load-time)
runtime-base/zstd.hhdeclares the minimal entry points (ZSTD_decompress,ZSTD_isError,ZSTD_getErrorName) instead of pulling inzstd.h.assembly-store.cc) and Mono (embedded-assemblies.cc) decompress viaZSTD_decompress, with error handling onZSTD_isError.Build wiring
-lSystem.IO.Compression.Nativeinstead ofxa::lz4; removedHAVE_LZ4, thecommon/lz4CMake library, thelibxa-lz4archive inNativeRuntimeComponents, and the LZ4 source references inConfiguration.props/native.targets.Tooling
decompress-assembliesandtmtdetect the new magic and decompress usingSystem.IO.Compression.ZstandardDecoder; unusedK4os.Compression.LZ4references removed from the affected tool projects.tmtfollow-up fixes from review feedback:Out of scope
FastDeploy/xamarin.sync) still uses LZ4 for fast-deploy-over-adb. As a resultexternal/lz4and theexternal/xamarin-android-toolsK4os reference are intentionally retained; fully removing the LZ4 dependency requires a follow-up to migrate fastdev.Validation notes
BuildReleaseArm64{Simple,XForms}DotNet.CoreCLR.apkdesc) were refreshed from the CI-produced APK descriptors. Versus the LZ4 baselines onmain, Zstd shrinkslibassembly-store.so(Simple: 3,461,344 → 2,903,016 bytes; XForms: 14,137,920 → 11,658,648 bytes).net11.0and useSystem.IO.Compression.ZstandardDecoderfor Zstd payloads, avoiding native Zstd P/Invokes in the tools.Benchmark: .NET MAUI app on device
Measured a Release .NET MAUI app (
Microsoft.Maui.Controls11.0.0-preview.2.26152.10,net11.0-android,android-arm64, CoreCLR, profiled AOT) built against two locally-built SDKs: this PR (Zstd) vs its merge-base onmain(LZ4). Only the AssemblyStore compression differs. Device: Samsung Galaxy A16 (SM-A165F), Android 16 (API 36), arm64-v8a.main)libassembly-store.so(uncompressed)extractNativeLibs=false)TotalTime, medianTotalTime, meanadb shell am start -S -Wsamples per variant (alternating launches to neutralise thermal drift; first launch discarded as warmup). Zstd's slightly slower cold start reflects Zstd's higher decompression cost vs LZ4, traded for the ~3 MB smaller store.extractNativeLibs=false, which stores the store.souncompressed and page-aligned — what Google Play serves for.aabon API 26+. With the defaultextractNativeLibs=true, the raw.apkis a poor proxy: the APK zip re-Deflates the store, and Deflate shrinks the more-redundant LZ4 payload further, so the naive.apksize can look larger for Zstd despite the smaller on-device footprint.XALZ(LZ4,main) vsXAZS(Zstd, this PR).