From 37f60d5f320eef6664baeefc07a9dfee9f40d625 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 14 Sep 2026 12:05:10 +0200 Subject: [PATCH] [CoreCLR] Remove assembly store decompression cache 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> --- .../AssemblyStore/StoreReader_V2.Classes.cs | 6 +- .../src/AssemblyStore/StoreReader_V2.cs | 12 +- Documentation/building/configuration.md | 6 - Documentation/project-docs/AssemblyStores.md | 8 +- .../GenerateNativeApplicationConfigSources.cs | 2 - .../Tasks/CreateAssemblyStoreTests.cs | 31 +- ...rateNativeApplicationConfigSourcesTests.cs | 33 - .../Utilities/EnvironmentHelper.cs | 8 +- .../Utilities/ApplicationConfigCLR.cs | 1 - ...icationConfigNativeAssemblyGeneratorCLR.cs | 2 - .../AssemblyStoreGenerator.Classes.cs | 10 +- .../Utilities/AssemblyStoreGenerator.cs | 37 +- .../Xamarin.Android.Common.targets | 2 - .../mono/android/clr/MonoPackageManager.java | 3 +- src/native/clr/host/assembly-store.cc | 675 +----------------- src/native/clr/host/host.cc | 1 - src/native/clr/include/constants.hh | 1 - src/native/clr/include/host/assembly-store.hh | 1 - .../include/runtime-base/android-system.hh | 11 - src/native/clr/include/xamarin-app.hh | 5 +- .../xamarin-app-stub/application_dso_stub.cc | 1 - .../mono/xamarin-app-stub/xamarin-app.hh | 4 +- .../Tests/InstallAndRunTests.cs | 133 ---- 23 files changed, 59 insertions(+), 934 deletions(-) diff --git a/.github/skills/read-assembly-store/src/AssemblyStore/StoreReader_V2.Classes.cs b/.github/skills/read-assembly-store/src/AssemblyStore/StoreReader_V2.Classes.cs index be180f517e5..716a82847db 100644 --- a/.github/skills/read-assembly-store/src/AssemblyStore/StoreReader_V2.Classes.cs +++ b/.github/skills/read-assembly-store/src/AssemblyStore/StoreReader_V2.Classes.cs @@ -15,18 +15,16 @@ sealed class Header // Index size in bytes public readonly uint index_size; - public readonly ulong content_id; - public uint NativeSize => (uint)(5 * sizeof (uint) + ((version & ASSEMBLY_STORE_FORMAT_NUMBER_MASK) >= 4 ? sizeof (ulong) : 0)); + public const uint NativeSize = 5 * sizeof (uint); - public Header (uint magic, uint version, uint entry_count, uint index_entry_count, uint index_size, ulong content_id) + public Header (uint magic, uint version, uint entry_count, uint index_entry_count, uint index_size) { 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/.github/skills/read-assembly-store/src/AssemblyStore/StoreReader_V2.cs b/.github/skills/read-assembly-store/src/AssemblyStore/StoreReader_V2.cs index c8b162e442b..c1e12e0ea3c 100644 --- a/.github/skills/read-assembly-store/src/AssemblyStore/StoreReader_V2.cs +++ b/.github/skills/read-assembly-store/src/AssemblyStore/StoreReader_V2.cs @@ -15,10 +15,7 @@ partial class StoreReader_V2 : AssemblyStoreReader const uint ASSEMBLY_STORE_FORMAT_VERSION_32BIT_V2 = 0x00000002; const uint ASSEMBLY_STORE_FORMAT_VERSION_64BIT_V3 = 0x80000003; // Must match the ASSEMBLY_STORE_FORMAT_VERSION native constant const uint ASSEMBLY_STORE_FORMAT_VERSION_32BIT_V3 = 0x00000003; - const uint ASSEMBLY_STORE_FORMAT_VERSION_CORECLR_64BIT_V4 = 0x80000004; // Must match the ASSEMBLY_STORE_FORMAT_VERSION native constant - const uint ASSEMBLY_STORE_FORMAT_VERSION_CORECLR_32BIT_V4 = 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; @@ -90,10 +87,6 @@ public StoreReader_V2 (Stream store, string path) ASSEMBLY_STORE_FORMAT_VERSION_64BIT_V3 | ASSEMBLY_STORE_ABI_X64, ASSEMBLY_STORE_FORMAT_VERSION_32BIT_V3 | ASSEMBLY_STORE_ABI_ARM, ASSEMBLY_STORE_FORMAT_VERSION_32BIT_V3 | ASSEMBLY_STORE_ABI_X86, - ASSEMBLY_STORE_FORMAT_VERSION_CORECLR_64BIT_V4 | ASSEMBLY_STORE_ABI_AARCH64, - ASSEMBLY_STORE_FORMAT_VERSION_CORECLR_64BIT_V4 | ASSEMBLY_STORE_ABI_X64, - ASSEMBLY_STORE_FORMAT_VERSION_CORECLR_32BIT_V4 | ASSEMBLY_STORE_ABI_ARM, - ASSEMBLY_STORE_FORMAT_VERSION_CORECLR_32BIT_V4 | ASSEMBLY_STORE_ABI_X86, }; } @@ -140,9 +133,8 @@ 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, content_id); + header = new Header (magic, version, entry_count, index_entry_count, index_size); return true; } @@ -164,7 +156,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 (); diff --git a/Documentation/building/configuration.md b/Documentation/building/configuration.md index e12ffc427d5..5333389cdf6 100644 --- a/Documentation/building/configuration.md +++ b/Documentation/building/configuration.md @@ -118,12 +118,6 @@ 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 117d8760f04..9ac2195c96f 100644 --- a/Documentation/project-docs/AssemblyStores.md +++ b/Documentation/project-docs/AssemblyStores.md @@ -107,11 +107,10 @@ and aligned to a byte boundary. The header is a fixed-size structure at the beginning of each assembly store file: - **MAGIC** (`uint32_t`) - Magic value `0x41424158` ("XABA" in little-endian) -- **FORMAT_VERSION** (`uint32_t`) - Store format version number (includes ABI and 64-bit flags). Version `3` is used by MonoVM applications and version `4` by CoreCLR applications (see [Hash table format](#hash-table-format)) +- **FORMAT_VERSION** (`uint32_t`) - Store format version number (includes ABI and 64-bit flags). Version `3` is used by MonoVM and CoreCLR applications (see [Hash table format](#hash-table-format)) - **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] @@ -169,7 +168,6 @@ 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: @@ -181,7 +179,6 @@ 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 @@ -236,7 +233,7 @@ appending it in order to generate the hash for index lookup. The hashing algorithm depends on the runtime the application targets: - - **CoreCLR** (store format version `4`): the hash is a 32-bit + - **CoreCLR** (store format version `3`): the hash is a 32-bit [CRC32](https://en.wikipedia.org/wiki/Cyclic_redundancy_check) value, used on both 32-bit and 64-bit platforms. - **MonoVM** (store format version `3`): the hash is obtained using the @@ -287,7 +284,6 @@ 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 aeb51c46044..5091fd6a9f9 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs @@ -66,7 +66,6 @@ public class GenerateNativeApplicationConfigSources : AndroidTask /// public bool EmitLlvmIrComments { get; set; } - public bool AndroidEnableAssemblyStoreDecompressionCache { get; set; } public string? RuntimeConfigBinFilePath { get; set; } public string ProjectRuntimeConfigFilePath { get; set; } = String.Empty; public string? ProjectRuntimeConfigDevFilePath { get; set; } @@ -294,7 +293,6 @@ static bool ShouldSkipAssembly (ITaskItem assembly) MarshalMethodsEnabled = EnableMarshalMethods, 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 index 466f52e15df..2eb8f6b24e8 100644 --- 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 @@ -1,11 +1,8 @@ #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; @@ -16,14 +13,13 @@ namespace Xamarin.Android.Build.Tests.Tasks; public class CreateAssemblyStoreTests : BaseTest { [Test] - public void ContentIdMatchesStoreContents () + public void CoreCLRStoreUsesVersionThreeHeader () { - string testDirectory = Path.Combine (Root, "temp", nameof (ContentIdMatchesStoreContents)); + string testDirectory = Path.Combine (Root, "temp", nameof (CoreCLRStoreUsesVersionThreeHeader)); 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); + File.WriteAllBytes (assemblyPath, [1, 3, 3, 7, 9, 11, 17, 23]); var metadata = new Dictionary { ["Abi"] = "arm64-v8a", @@ -41,17 +37,16 @@ public void ContentIdMatchesStoreContents () 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)); + using var reader = new BinaryReader (File.OpenRead (storePath)); 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." - ); + Assert.AreEqual (0x80010003u, reader.ReadUInt32 (), "Unexpected arm64 assembly store version."); + uint assemblyCount = reader.ReadUInt32 (); + uint indexEntryCount = reader.ReadUInt32 (); + Assert.AreEqual (assemblyCount * 2, indexEntryCount, "Unexpected index entry count."); + uint indexSize = reader.ReadUInt32 (); + Assert.AreEqual (5 * sizeof (uint), reader.BaseStream.Position, "Unexpected assembly store header size."); + + reader.BaseStream.Seek (indexSize, SeekOrigin.Current); + Assert.AreEqual (0u, reader.ReadUInt32 (), "The first descriptor should immediately follow the index."); } } 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 04cd7982543..803cc00bc32 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,37 +42,4 @@ 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 23138661645..e2263de543e 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 @@ -61,10 +61,9 @@ public sealed class ApplicationConfig_CoreCLR : IApplicationConfig public uint jni_remapping_replacement_method_index_entry_count; public string android_package_name = String.Empty; public bool have_assembly_store; - public bool assembly_store_decompression_cache_enabled; } - const uint ApplicationConfigFieldCount_CoreCLR = 20; + const uint ApplicationConfigFieldCount_CoreCLR = 19; // This must be identical to the ApplicationConfig structure in src/native/mono/xamarin-app-stub/xamarin-app.hh public sealed class ApplicationConfig_MonoVM : IApplicationConfig @@ -402,10 +401,6 @@ static IApplicationConfig ReadApplicationConfig_CoreCLR (EnvironmentFile envFile ret.have_assembly_store = ConvertFieldToBool ("have_assembly_store", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]); break; - case 19: // 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++; } @@ -766,7 +761,6 @@ 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 23924aed6fc..2ea9f06e60a 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigCLR.cs @@ -50,5 +50,4 @@ sealed class ApplicationConfigCLR public uint jni_remapping_replacement_method_index_entry_count; public string android_package_name = String.Empty; 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 0e30744e1b7..83a0b50a6f6 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGeneratorCLR.cs @@ -199,7 +199,6 @@ sealed class DsoCacheState public bool MarshalMethodsEnabled { 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) @@ -287,7 +286,6 @@ 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 7a30ec231b8..9df30db6303 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) + sizeof (ulong); + public const uint NativeSize = 5 * sizeof (uint); public readonly uint magic = ASSEMBLY_STORE_MAGIC; public readonly uint version; @@ -14,19 +14,17 @@ 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, ulong content_id) + public AssemblyStoreHeader (uint version, uint entry_count, uint index_entry_count, uint index_size) { 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, ulong content_id) - : this (version, entry_count, index_entry_count, index_size, content_id) + 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) { this.magic = magic; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs index 1241a805f25..f197369bd53 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs @@ -2,7 +2,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.IO.Hashing; using Microsoft.Android.Build.Tasks; using Microsoft.Build.Utilities; @@ -29,7 +28,6 @@ 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 @@ -55,10 +53,10 @@ 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 = 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; + 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_CORECLR_64BIT = 0x80000003; // Must match the ASSEMBLY_STORE_FORMAT_VERSION native constant + const uint ASSEMBLY_STORE_FORMAT_VERSION_CORECLR_32BIT = 0x00000003; const uint ASSEMBLY_STORE_ABI_AARCH64 = 0x00010000; const uint ASSEMBLY_STORE_ABI_ARM = 0x00020000; @@ -166,7 +164,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) { @@ -285,7 +262,6 @@ 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) @@ -296,9 +272,8 @@ 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, content_id); + return new AssemblyStoreHeader (magic, version, entry_count, index_entry_count, index_size); } #endif diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index 2feb9d14bbf..a70c32a55d5 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -172,7 +172,6 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved. True <_AndroidAssemblyStoreCompressionLevel Condition=" '$(_AndroidAssemblyStoreCompressionLevel)' == '' And '$(Optimize)' == 'True' ">22 <_AndroidAssemblyStoreCompressionLevel Condition=" '$(_AndroidAssemblyStoreCompressionLevel)' == '' ">3 - False False <_AndroidCheckedBuild Condition=" '$(_AndroidCheckedBuild)' == '' "> @@ -1839,7 +1838,6 @@ because xbuild doesn't support framework reference assemblies. BoundExceptionType="$(AndroidBoundExceptionType)" RuntimeConfigBinFilePath="$(_BinaryRuntimeConfigPath)" UseAssemblyStore="$(_AndroidUseAssemblyStore)" - AndroidEnableAssemblyStoreDecompressionCache="$(AndroidEnableAssemblyStoreDecompressionCache)" EnableMarshalMethods="$(_AndroidUseMarshalMethods)" CustomBundleConfigFile="$(AndroidBundleConfigurationFile)" TargetsCLR="$(_AndroidUseCLR)" diff --git a/src/java-runtime/java/mono/android/clr/MonoPackageManager.java b/src/java-runtime/java/mono/android/clr/MonoPackageManager.java index aeb6e17ded8..0f59ebd590c 100644 --- a/src/java-runtime/java/mono/android/clr/MonoPackageManager.java +++ b/src/java-runtime/java/mono/android/clr/MonoPackageManager.java @@ -47,7 +47,6 @@ 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); @@ -62,7 +61,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, codeCacheDir}; + String[] appDirs = new String[] {filesDir, cacheDir, dataDir}; boolean haveSplitApks = runtimePackage.splitSourceDirs != null && runtimePackage.splitSourceDirs.length > 0; NativeLibraryHelper.loadLibrary ("monodroid", runtimePackage, apks); diff --git a/src/native/clr/host/assembly-store.cc b/src/native/clr/host/assembly-store.cc index 90280f8cf0a..3ea1839fb0b 100644 --- a/src/native/clr/host/assembly-store.cc +++ b/src/native/clr/host/assembly-store.cc @@ -1,21 +1,10 @@ -#include -#include #include -#include #include #include #include -#include -#include -#include -#include -#include -#include - #include #include -#include #include #include #include @@ -46,592 +35,6 @@ namespace { return false; } - - namespace asm_cache { - 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 - { - WriteRequest *next; - uint8_t *payload; - size_t size; - uint32_t descriptor_index; - }; - - enum class WriteResult - { - Succeeded, - Failed, - }; - - pthread_mutex_t state_lock = PTHREAD_MUTEX_INITIALIZER; - WriteRequest *write_queue_head = nullptr; - WriteRequest *write_queue_tail = nullptr; - char *cache_dir = nullptr; - uint8_t **tracking = nullptr; - 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 allocate_write_request (size_t payload_size) noexcept -> WriteRequest* - { - auto *request = static_cast(std::malloc (sizeof (WriteRequest))); - if (request == nullptr) [[unlikely]] { - return nullptr; - } - - request->payload = static_cast(std::malloc (payload_size)); - if (request->payload == nullptr) [[unlikely]] { - std::free (request); - return nullptr; - } - - request->next = nullptr; - return request; - } - - 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; - while (off < len) { - ssize_t n = write (fd, buf + off, len - off); - if (n < 0) { - if (errno == EINTR) { - continue; - } - return false; - } - if (n == 0) { - errno = EIO; - return false; - } - off += static_cast(n); - } - return true; - } - - void log_file_error (const char *operation, const char *path, int error) noexcept - { - log_debugf (LOG_ASSEMBLY, "Decompressed-assembly cache %s failed for '%s': %s", operation, path, std::strerror (error)); - } - - // Unlike Util::format_with_retry, cache path allocation must not abort the application on failure. - class CachePath final - { - public: - template - CachePath (const char *operation, const char *source, TFormatter formatter) noexcept - { - int length = formatter (stack_buffer, sizeof (stack_buffer)); - if (length < 0) [[unlikely]] { - log_file_error (operation, source, errno); - return; - } - if (static_cast(length) < sizeof (stack_buffer)) { - path = stack_buffer; - return; - } - - size_t capacity = static_cast(length) + 1uz; - char *heap_buffer = static_cast(std::malloc (capacity)); - if (heap_buffer == nullptr) [[unlikely]] { - log_file_error (operation, source, ENOMEM); - return; - } - - length = formatter (heap_buffer, capacity); - if (length < 0 || static_cast(length) >= capacity) [[unlikely]] { - int error = length < 0 ? errno : ENAMETOOLONG; - std::free (heap_buffer); - log_file_error (operation, source, error); - return; - } - path = heap_buffer; - } - - // `cache_dir` is immutable once enabled, including on the writer thread. - explicit CachePath (uint32_t descriptor_index) noexcept - : CachePath ("path formatting", cache_dir, [descriptor_index](char *buffer, size_t size) noexcept { - return snprintf (buffer, size, "%s/%u.bin", cache_dir, descriptor_index); - }) - {} - - CachePath (CachePath const&) = delete; - CachePath (CachePath&&) = delete; - auto operator= (CachePath const&) -> CachePath& = delete; - auto operator= (CachePath&&) -> CachePath& = delete; - - ~CachePath () noexcept - { - if (path != stack_buffer) { - std::free (path); - } - } - - auto get () const noexcept -> const char* - { - return path; - } - - private: - char stack_buffer[Util::LocalPathBufferSize]; - char *path = nullptr; - }; - - auto write_cache_file (WriteRequest *req) noexcept -> WriteResult - { - CachePath path { req->descriptor_index }; - if (path.get () == nullptr) [[unlikely]] { - return WriteResult::Failed; - } - - CachePath tmp_path { "temporary-file path formatting", path.get (), [&path](char *buffer, size_t size) noexcept { - return snprintf (buffer, size, "%s.tmp.%d", path.get (), getpid ()); - }}; - if (tmp_path.get () == nullptr) [[unlikely]] { - return WriteResult::Failed; - } - - int fd; - do { - fd = open (tmp_path.get (), O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, 0600); - } while (fd < 0 && errno == EINTR); - if (fd < 0) { - log_file_error ("temporary-file creation", path.get (), errno); - return WriteResult::Failed; - } - - bool ok = write_fully (fd, req->payload, req->size); - int error = ok ? 0 : errno; - if (close (fd) != 0 && ok) { - ok = false; - error = errno; - } - - if (!ok) { - log_file_error ("write", path.get (), error); - unlink (tmp_path.get ()); - return WriteResult::Failed; - } - - int rename_result; - do { - rename_result = rename (tmp_path.get (), path.get ()); - } while (rename_result != 0 && errno == EINTR); - - if (rename_result != 0) { - error = errno; - log_file_error ("publish", path.get (), error); - unlink (tmp_path.get ()); - return WriteResult::Failed; - } - - return WriteResult::Succeeded; - } - - // Discards every queued request without writing it. Must be called with `state_lock` held. - void clear_write_queue_locked () noexcept - { - WriteRequest *request = write_queue_head; - write_queue_head = nullptr; - write_queue_tail = nullptr; - - while (request != nullptr) { - WriteRequest *next = request->next; - queued_bytes -= request->size; - std::free (request->payload); - std::free (request); - request = next; - } - } - - [[gnu::cold]] - auto writer_loop ([[maybe_unused]] void *arg) noexcept -> void* - { - while (true) { - WriteRequest *request; - { - lock_guard lock (state_lock); - request = write_queue_head; - if (request == nullptr) { - writer_running = false; - return nullptr; - } - - write_queue_head = request->next; - if (write_queue_head == nullptr) { - write_queue_tail = nullptr; - } - } - - size_t request_size = request->size; - WriteResult write_result = write_cache_file (request); - std::free (request->payload); - std::free (request); - - { - 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_debugf (LOG_ASSEMBLY, "Disabling decompressed-assembly cache writes after a persistence failure"); - return nullptr; - } - } - } - } - - 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_debugf (LOG_ASSEMBLY, "Failed to start decompressed-assembly cache writer: %s", std::strerror (result)); - return false; - } - - return true; - } - - bool ensure_directory (const char *path) noexcept - { - if (mkdir (path, 0700) == 0) { - return true; - } - - int error = errno; - if (error != EEXIST) { - log_file_error ("directory creation", path, error); - return false; - } - - struct stat st {}; - if (lstat (path, &st) != 0) { - log_file_error ("directory validation", path, errno); - return false; - } - if (!S_ISDIR (st.st_mode)) { - log_file_error ("directory validation", path, ENOTDIR); - return false; - } - - 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 (const char *dir) noexcept - { - DIR *handle = opendir (dir); - if (handle == nullptr) { - return; - } - - for (dirent *entry = readdir (handle); entry != nullptr; entry = readdir (handle)) { - if (strstr (entry->d_name, ".tmp.") == nullptr) { - continue; - } - - CachePath path { "stale temporary-file path formatting", dir, [dir, entry](char *buffer, size_t size) noexcept { - return snprintf (buffer, size, "%s/%s", dir, entry->d_name); - }}; - if (path.get () != nullptr) { - unlink (path.get ()); - } - } - - closedir (handle); - } - - void ensure_initialized (uint64_t assembly_store_id) noexcept - { - if (initialized) { - return; - } - initialized = true; - - 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 - if (getenv ("XA_DISABLE_ASSEMBLY_CACHE") != nullptr) { - return; - } - { - char prop_value[Constants::PROPERTY_VALUE_BUFFER_LEN]; - const char *cache_prop = AndroidSystem::monodroid_get_system_property ("debug.net.asmcache", prop_value, sizeof (prop_value)); - if (cache_prop != nullptr) { - if (cache_prop [0] == '0') { - cache_requested = false; - } else if (cache_prop [0] == '1') { - cache_requested = true; - } - } - } - - if (!cache_requested) { - return; - } - - const char *code_cache_dir = AndroidSystem::get_app_code_cache_dir (); - if (*code_cache_dir == '\0') { - return; - } - - // The cache lives at `//`, with both levels created in turn. - CachePath root { "cache-directory path formatting", code_cache_dir, [code_cache_dir](char *buffer, size_t size) noexcept { - return snprintf (buffer, size, "%s/%.*s", code_cache_dir, static_cast(CACHE_DIR_NAME.length ()), CACHE_DIR_NAME.data ()); - }}; - if (root.get () == nullptr) [[unlikely]] { - return; - } - - if (!ensure_directory (root.get ())) { - return; - } - - store_id = assembly_store_id; - CachePath path { "store-directory path formatting", root.get (), [&root](char *buffer, size_t size) noexcept { - return snprintf (buffer, size, "%s/%" PRIx64, root.get (), store_id); - }}; - if (path.get () == nullptr) [[unlikely]] { - return; - } - - if (!ensure_directory (path.get ())) { - return; - } - - remove_stale_temp_files (path.get ()); - - if (compressed_assembly_count == 0) { - return; - } - - // Neither allocation is ever freed: both live for as long as the process does. - cache_dir = strdup (path.get ()); - if (cache_dir == nullptr) [[unlikely]] { - return; - } - - tracking = static_cast(std::calloc (compressed_assembly_count, sizeof (uint8_t*))); - if (tracking == nullptr) [[unlikely]] { - std::free (cache_dir); - cache_dir = nullptr; - return; - } - - enabled = true; - - { - lock_guard lock (state_lock); - writes_enabled = true; - } - - log_debugf ( - LOG_ASSEMBLY, - "Enabled decompressed-assembly cache at '%s'; store ID 0x%" PRIx64 "; write queue limit %zu bytes", - cache_dir, - store_id, - MAX_QUEUED_BYTES - ); - } - - auto try_load (uint32_t descriptor_index, std::string_view name, uint32_t expected_size) noexcept -> uint8_t* - { - if (!enabled) { - return nullptr; - } - - CachePath path { descriptor_index }; - if (path.get () == nullptr) [[unlikely]] { - return nullptr; - } - - int fd = open (path.get (), O_RDONLY | O_CLOEXEC | O_NOFOLLOW); - if (fd < 0) { - return nullptr; - } - - struct stat st {}; - if (fstat (fd, &st) != 0 || - !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) + 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; - } - - 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_debugf (LOG_ASSEMBLY, "Ignoring invalid decompressed-assembly cache entry for '%.*s'", static_cast(name.length ()), name.data ()); - return nullptr; - } - - return static_cast(mapped); - } - - void enqueue_write (uint32_t descriptor_index, std::string_view name, const uint8_t *data, size_t size) noexcept - { - if (!enabled) { - return; - } - - if (size > SIZE_MAX - sizeof (CacheFileFooter)) { - return; - } - size_t total = size + sizeof (CacheFileFooter); - - size_t bytes_queued = 0; - bool queue_full = false; - { - 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_debugf ( - LOG_ASSEMBLY, - "Skipping decompressed-assembly cache write for '%.*s': %zu bytes exceed the %zu-byte queue limit", - static_cast(name.length ()), - name.data (), - total, - MAX_QUEUED_BYTES - ); - } else { - log_debugf ( - LOG_ASSEMBLY, - "Skipping decompressed-assembly cache write for '%.*s': %zu of %zu queue bytes are in use", - static_cast(name.length ()), - name.data (), - bytes_queued, - MAX_QUEUED_BYTES - ); - } - return; - } - - WriteRequest *req = allocate_write_request (total); - if (req == nullptr) [[unlikely]] { - log_debugf ( - LOG_ASSEMBLY, - "Skipping decompressed-assembly cache write for '%.*s': unable to allocate the request or payload", - static_cast(name.length ()), - name.data () - ); - lock_guard lock (state_lock); - queued_bytes -= total; - return; - } - - req->size = total; - req->descriptor_index = descriptor_index; - - // The runtime can modify the shared decompression buffer after this - // method returns, so the background writer needs an immutable copy. - memcpy (req->payload, data, size); - - CacheFileFooter footer { - .magic = CACHE_FILE_MAGIC, - .version = CACHE_FILE_FORMAT_VERSION, - .store_id = store_id, - .payload_hash = hash_payload (req->payload, size), - .descriptor_index = descriptor_index, - .payload_size = static_cast(size), - }; - memcpy (req->payload + size, &footer, sizeof (footer)); - - { - lock_guard lock (state_lock); - if (!writes_enabled) { - queued_bytes -= total; - std::free (req->payload); - std::free (req); - return; - } - - if (write_queue_tail == nullptr) { - write_queue_head = req; - } else { - write_queue_tail->next = req; - } - write_queue_tail = req; - - if (!writer_running) { - writer_running = true; - if (!start_writer_locked ()) { - writer_running = false; - writes_enabled = false; - clear_write_queue_locked (); - } - } - } - } - } // 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 @@ -696,25 +99,15 @@ 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. - 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 (!is_loaded ()) { StartupAwareLock decompress_lock (assembly_decompress_mutex); if (is_loaded ()) { - set_assembly_data_and_size (resolve_data (), cad.uncompressed_file_size, assembly_data, assembly_data_size); + set_assembly_data_and_size (data_buffer, cad.uncompressed_file_size, assembly_data, assembly_data_size); if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); @@ -723,8 +116,6 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co return {assembly_data, assembly_data_size}; } - asm_cache::ensure_initialized (assembly_store_content_id); - if (header->uncompressed_length != cad.uncompressed_file_size) { if (header->uncompressed_length > cad.uncompressed_file_size) { Helpers::abort_applicationf ( @@ -743,53 +134,40 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co } const char *data_start = pointer_add(e.image_data, sizeof(CompressedAssemblyHeader)); + log_debugf (LOG_ASSEMBLY, "Decompressing assembly '%.*s' from the assembly store", static_cast(name.length ()), name.data ()); + size_t ret = ZSTD_decompress (data_buffer, cad.uncompressed_file_size, data_start, assembly_data_size); - 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_debugf (LOG_ASSEMBLY, "Loaded decompressed assembly '%.*s' from the on-device cache", static_cast(name.length ()), name.data ()); - if (asm_cache::tracking != nullptr) { - asm_cache::tracking[descriptor_index] = cached; - } - } else { - log_debugf (LOG_ASSEMBLY, "Decompressing assembly '%.*s' from the assembly store", static_cast(name.length ()), name.data ()); - size_t ret = ZSTD_decompress (data_buffer, cad.uncompressed_file_size, data_start, assembly_data_size); - - if (ZSTD_isError (ret)) { - Helpers::abort_applicationf ( - LOG_ASSEMBLY, - std::source_location::current (), - "Decompression of assembly %.*s failed: %s", - static_cast(name.length ()), - name.data (), - ZSTD_getErrorName (ret) - ); - } - - if (ret != cad.uncompressed_file_size) { - Helpers::abort_applicationf ( - LOG_ASSEMBLY, - std::source_location::current (), - "Decompression of assembly %.*s yielded a different size (expected %u, got %u)", - static_cast(name.length ()), - name.data (), - cad.uncompressed_file_size, - static_cast(ret) - ); - } + if (ZSTD_isError (ret)) { + Helpers::abort_applicationf ( + LOG_ASSEMBLY, + std::source_location::current (), + "Decompression of assembly %.*s failed: %s", + static_cast(name.length ()), + name.data (), + ZSTD_getErrorName (ret) + ); + } - asm_cache::enqueue_write (descriptor_index, name, data_buffer, cad.uncompressed_file_size); + if (ret != cad.uncompressed_file_size) { + Helpers::abort_applicationf ( + LOG_ASSEMBLY, + std::source_location::current (), + "Decompression of assembly %.*s yielded a different size (expected %u, got %u)", + static_cast(name.length ()), + name.data (), + cad.uncompressed_file_size, + static_cast(ret) + ); } __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, loaded_from_cache ? " (decompressed cache hit)"sv : ""sv); + internal_timing.add_more_info (name); } } - set_assembly_data_and_size (resolve_data (), cad.uncompressed_file_size, assembly_data, assembly_data_size); + set_assembly_data_and_size (data_buffer, cad.uncompressed_file_size, assembly_data, assembly_data_size); } else #endif // def RELEASE { @@ -934,7 +312,6 @@ void AssemblyStore::configure_from_payload (const void *payload_start, const cha 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; @@ -962,5 +339,5 @@ void AssemblyStore::configure_from_payload (const void *payload_start, const cha names_cursor += name_length; } - log_debugf (LOG_ASSEMBLY, "Mapped assembly store %s; content ID 0x%" PRIx64, optional_string (store_path), assembly_store_content_id); + log_debugf (LOG_ASSEMBLY, "Mapped assembly store %s", optional_string (store_path)); } diff --git a/src/native/clr/host/host.cc b/src/native/clr/host/host.cc index f82d71f37a2..1e93c54871c 100644 --- a/src/native/clr/host/host.cc +++ b/src/native/clr/host/host.cc @@ -337,7 +337,6 @@ 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 6320e55985d..cc7ba1b8ccf 100644 --- a/src/native/clr/include/constants.hh +++ b/src/native/clr/include/constants.hh @@ -108,7 +108,6 @@ 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/host/assembly-store.hh b/src/native/clr/include/host/assembly-store.hh index 034e996286e..d6929bcc23c 100644 --- a/src/native/clr/include/host/assembly-store.hh +++ b/src/native/clr/include/host/assembly-store.hh @@ -34,7 +34,6 @@ 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 pthread_mutex_t assembly_decompress_mutex = PTHREAD_MUTEX_INITIALIZER; }; } diff --git a/src/native/clr/include/runtime-base/android-system.hh b/src/native/clr/include/runtime-base/android-system.hh index ce37d54c7f5..8a9ac43631b 100644 --- a/src/native/clr/include/runtime-base/android-system.hh +++ b/src/native/clr/include/runtime-base/android-system.hh @@ -97,16 +97,6 @@ namespace xamarin::android { } #if !defined (XA_HOST_NATIVEAOT) - static auto get_app_code_cache_dir () noexcept -> const char* - { - return app_code_cache_dir; - } - - static void set_app_code_cache_dir (jstring_wrapper& code_cache_dir) noexcept - { - app_code_cache_dir = Util::duplicate_string (code_cache_dir.get_cstr ()); - } - static auto get_native_libraries_dir () noexcept -> const char* { return native_libraries_dir; @@ -238,7 +228,6 @@ namespace xamarin::android { static inline const char *primary_override_dir = ""; #if !defined (XA_HOST_NATIVEAOT) static inline const char *native_libraries_dir = ""; - static inline const char *app_code_cache_dir = ""; #if defined (DEBUG) static inline BundledProperty *bundled_properties = nullptr; diff --git a/src/native/clr/include/xamarin-app.hh b/src/native/clr/include/xamarin-app.hh index 1fdb13b78a4..c15776e2f27 100644 --- a/src/native/clr/include/xamarin-app.hh +++ b/src/native/clr/include/xamarin-app.hh @@ -30,7 +30,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 = 4 | ASSEMBLY_STORE_64BIT_FLAG | ASSEMBLY_STORE_ABI; +static constexpr uint32_t ASSEMBLY_STORE_FORMAT_VERSION = 3 | 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 @@ -128,7 +128,6 @@ 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 @@ -160,7 +159,6 @@ 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 @@ -224,7 +222,6 @@ struct ApplicationConfig uint32_t jni_remapping_replacement_method_index_entry_count; const char *android_package_name; 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 df7bbad9ffb..3890aef2639 100644 --- a/src/native/clr/xamarin-app-stub/application_dso_stub.cc +++ b/src/native/clr/xamarin-app-stub/application_dso_stub.cc @@ -66,7 +66,6 @@ const ApplicationConfig application_config = { .jni_remapping_replacement_method_index_entry_count = 2, .android_package_name = android_package_name, .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 6504ff88667..d2dacdd9263 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 = 4 | ASSEMBLY_STORE_64BIT_FLAG | ASSEMBLY_STORE_ABI; +static constexpr uint32_t ASSEMBLY_STORE_FORMAT_VERSION = 3 | 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,7 +145,6 @@ 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 @@ -177,7 +176,6 @@ 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 68ee904b23b..085981d7387 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs @@ -855,139 +855,6 @@ public void DeployToDevice ([Values] bool isRelease, [Values (AndroidRuntime.Cor Assert.IsTrue (didLaunch, "Activity should have started."); } - [Test] - public void AssemblyStoreDecompressionCacheMapsPersistedAssemblies ([Values] bool longCachePath) - { - if (IgnoreUnsupportedConfiguration (AndroidRuntime.CoreCLR, release: true)) { - return; - } - - var app = new XamarinAndroidApplicationProject (packageName: PackageUtils.MakePackageName (AndroidRuntime.CoreCLR, longCachePath ? "assemblycachelong" : "assemblycache")) { - IsRelease = true, - }; - app.SetRuntime (AndroidRuntime.CoreCLR); - app.SetRuntimeIdentifiers (new [] { DeviceAbi }); - app.SetProperty ("AndroidEnableAssemblyStoreDecompressionCache", "true"); - app.AndroidManifest = app.AndroidManifest.Replace (" $$""" - package com.test; - - public class CachePathApplication extends android.app.Application { - @Override - public java.io.File getCodeCacheDir () { - java.io.File directory = new java.io.File (super.getCodeCacheDir (), "{{subdirectory}}"); - if (!directory.isDirectory () && !directory.mkdirs ()) { - throw new IllegalStateException ("Unable to create the long code cache directory"); - } - return directory; - } - } - """, - }); - } - string cacheRoot = cacheDirectory + "/decompressed-assembly-cache-v1"; - - using var appBuilder = CreateApkBuilder (); - Assert.IsTrue (appBuilder.Install (app), "Install should have succeeded."); - - ClearAdbLogcat (); - AdbStartActivity ($"{app.PackageName}/{app.JavaPackageName}.MainActivity"); - Assert.IsTrue ( - WaitForActivityToStart ( - app.PackageName, - "MainActivity", - Path.Combine (Root, appBuilder.ProjectDirectory, "assembly-cache-first-launch.log"), - ActivityStartTimeoutInSeconds - ), - "First launch should succeed." - ); - - string [] cacheFiles = []; - for (int attempt = 0; attempt < 40 && cacheFiles.Length < 2; attempt++) { - Thread.Sleep (250); - cacheFiles = RunAdbCommand ( - $"shell run-as {app.PackageName} find {cacheRoot} -type f -name '*.bin'" - ) - .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."); - if (longCachePath) { - Assert.IsTrue (cacheFiles.All (path => path.Length > 1024), "Cache paths should exceed the native stack buffer size."); - } - - RunAdbCommand ($"shell am force-stop --user all {app.PackageName}"); - string staleTempFile = cacheFiles.Last () + ".tmp.stale"; - RunAdbCommand ($"shell run-as {app.PackageName} touch {staleTempFile}"); - StringAssert.Contains (staleTempFile, RunAdbCommand ($"shell run-as {app.PackageName} ls {staleTempFile}"), - "The stale temporary file should exist before restarting."); - 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 ( - WaitForActivityToStart ( - app.PackageName, - "MainActivity", - Path.Combine (Root, appBuilder.ProjectDirectory, "assembly-cache-second-launch.log"), - ActivityStartTimeoutInSeconds - ), - "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."); - Assert.That ( - RunAdbCommand ($"shell run-as {app.PackageName} find {cacheRoot} -type f -name '*.tmp.stale'").Trim (), - Is.Empty, - "The second launch should remove stale temporary files." - ); - - 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."); - var maps = new StringBuilder (); - foreach (string pid in pids) { - maps.Append (RunAdbCommand ($"shell run-as {app.PackageName} cat /proc/{pid}/maps")); - } - StringAssert.Contains ( - "/" + cacheRoot + "/", - maps.ToString (), - "The second launch should map persisted decompressed assemblies." - ); - } - [Test] public void ActivityAliasRuns ([Values] bool isRelease, [Values (AndroidRuntime.CoreCLR, AndroidRuntime.NativeAOT)] AndroidRuntime runtime) {