Skip to content

Commit aac5912

Browse files
authored
[CoreCLR] Ignore some assemblies when asked to load them (#10249)
Ignore assemblies whose names end in `.ni` and `.ni.dll`, when a load request is made via the CoreCLR host contract callback. For each CoreCLR will ask to first load `Assembly.ni.dll`, NI standing for Native Image. This is something that was used on Windows long ago and it never was, nor will it be, supported on Unix. This is mostly a cosmetic change in that it prevents a false warning about a missing assembly from being logged in logcat. This, in itself, is a startup time improvement since those warnings would always be logged during every application launch, with logcat logging being very expensive (and unpredictable). The message about a missing `.ni.dll` is still logged, but with the Debug severity, which will cause it to show up only if the `assembly` logging category is enabled (it is disabled by default)
1 parent a6a5195 commit aac5912

16 files changed

Lines changed: 130 additions & 52 deletions

src/Xamarin.Android.Build.Tasks/Tasks/CreateAssemblyStore.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,20 @@ public class CreateAssemblyStore : AndroidTask
2828
[Required]
2929
public string [] SupportedAbis { get; set; } = [];
3030

31+
[Required]
32+
public string TargetRuntime { get; set; } = "";
33+
3134
public bool UseAssemblyStore { get; set; }
3235

3336
[Output]
3437
public ITaskItem [] AssembliesToAddToArchive { get; set; } = [];
3538

39+
AndroidRuntime targetRuntime;
40+
3641
public override bool RunTask ()
3742
{
43+
targetRuntime = MonoAndroidHelper.ParseAndroidRuntime (TargetRuntime);
44+
3845
// Get all the user and framework assemblies we may need to package
3946
var assemblies = ResolvedFrameworkAssemblies.Concat (ResolvedUserAssemblies).Where (asm => !(ShouldSkipAssembly (asm))).ToArray ();
4047

@@ -43,7 +50,7 @@ public override bool RunTask ()
4350
return !Log.HasLoggedErrors;
4451
}
4552

46-
var store_builder = new AssemblyStoreBuilder (Log);
53+
var store_builder = new AssemblyStoreBuilder (Log, targetRuntime);
4754
var per_arch_assemblies = MonoAndroidHelper.GetPerArchAssemblies (assemblies, SupportedAbis, true);
4855

4956
foreach (var kvp in per_arch_assemblies) {

src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreAssemblyInfo.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,17 @@ class AssemblyStoreAssemblyInfo
1616
public byte[] AssemblyNameNoExtBytes { get; }
1717
public FileInfo? SymbolsFile { get; set; }
1818
public FileInfo? ConfigFile { get; set; }
19+
public bool Ignored { get; }
1920

20-
public AssemblyStoreAssemblyInfo (string sourceFilePath, ITaskItem assembly)
21+
public AssemblyStoreAssemblyInfo (string sourceFilePath, ITaskItem assembly, bool assemblyIsIgnored = false)
2122
{
2223
Arch = MonoAndroidHelper.GetTargetArch (assembly);
2324
if (Arch == AndroidTargetArch.None) {
2425
throw new InvalidOperationException ($"Internal error: assembly item '{assembly}' lacks ABI information metadata");
2526
}
2627

2728
SourceFile = new FileInfo (sourceFilePath);
29+
Ignored = assemblyIsIgnored;
2830

2931
string? name = Path.GetFileName (SourceFile.Name);
3032
if (name == null) {

src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreBuilder.cs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@ class AssemblyStoreBuilder
1313
{
1414
readonly TaskLoggingHelper log;
1515
readonly AssemblyStoreGenerator storeGenerator;
16+
readonly AndroidRuntime targetRuntime;
1617

17-
public AssemblyStoreBuilder (TaskLoggingHelper log)
18+
public AssemblyStoreBuilder (TaskLoggingHelper log, AndroidRuntime targetRuntime)
1819
{
1920
this.log = log;
21+
this.targetRuntime = targetRuntime;
2022
storeGenerator = new (log);
2123
}
2224

@@ -39,6 +41,24 @@ public void AddAssembly (string assemblySourcePath, ITaskItem assemblyItem, bool
3941
}
4042

4143
storeGenerator.Add (storeAssemblyInfo);
44+
45+
ClrAddIgnoredNativeImageAssembly (assemblyItem);
46+
}
47+
48+
// When CoreCLR tries to load an assembly (say `AssemblyName.dll`) it will always first try to load
49+
// a "native image" assembly from `AssemblyName.ni.dll` which will **never** exist. The native image
50+
// assemblies were once supported only on Windows and were never (nor will ever be) supported on
51+
// Unix. In order to speed up load times, we add an empty entry for each `*.ni.dll` to the assembly
52+
// store index.
53+
void ClrAddIgnoredNativeImageAssembly (ITaskItem assemblyItem)
54+
{
55+
if (targetRuntime != AndroidRuntime.CoreCLR) {
56+
return;
57+
}
58+
59+
string ignoredName = Path.GetFileName (Path.ChangeExtension (assemblyItem.ItemSpec, ".ni.dll"));
60+
var storeAssemblyInfo = new AssemblyStoreAssemblyInfo (ignoredName, assemblyItem, assemblyIsIgnored: true);
61+
storeGenerator.Add (storeAssemblyInfo);
4262
}
4363

4464
public Dictionary<AndroidTargetArch, string> Generate (string outputDirectoryPath) => storeGenerator.Generate (outputDirectoryPath);

src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.Classes.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,21 @@ public AssemblyStoreHeader (uint magic, uint version, uint entry_count, uint ind
3232

3333
sealed class AssemblyStoreIndexEntry
3434
{
35-
public const uint NativeSize32 = 2 * sizeof (uint);
36-
public const uint NativeSize64 = sizeof (ulong) + sizeof (uint);
35+
// We treat `bool` as `byte` here, since that's what gets written to the binary
36+
public const uint NativeSize32 = 2 * sizeof (uint) + sizeof (byte);
37+
public const uint NativeSize64 = sizeof (ulong) + sizeof (uint) + sizeof (byte);
3738

3839
public readonly string name;
3940
public readonly ulong name_hash;
4041
public readonly uint descriptor_index;
42+
public readonly bool ignore;
4143

42-
public AssemblyStoreIndexEntry (string name, ulong name_hash, uint descriptor_index)
44+
public AssemblyStoreIndexEntry (string name, ulong name_hash, uint descriptor_index, bool ignore)
4345
{
4446
this.name = name;
4547
this.name_hash = name_hash;
4648
this.descriptor_index = descriptor_index;
49+
this.ignore = ignore;
4750
}
4851
}
4952

src/Xamarin.Android.Build.Tasks/Utilities/AssemblyStoreGenerator.cs

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ namespace Xamarin.Android.Tasks;
1616
// [HEADER]
1717
// [INDEX]
1818
// [ASSEMBLY_DESCRIPTORS]
19+
// [ASSEMBLY_NAMES]
1920
// [ASSEMBLY DATA]
2021
//
2122
// Formats of the sections above are as follows:
@@ -30,6 +31,7 @@ namespace Xamarin.Android.Tasks;
3031
// INDEX (variable size, HEADER.ENTRY_COUNT*2 entries, for assembly names with and without the extension)
3132
// [NAME_HASH] uint on 32-bit platforms, ulong on 64-bit platforms; xxhash of the assembly name
3233
// [DESCRIPTOR_INDEX] uint; index into in-store assembly descriptor array
34+
// [IGNORE] byte; if set to anything other than 0, the assembly is to be ignored when loading
3335
//
3436
// ASSEMBLY_DESCRIPTORS (variable size, HEADER.ENTRY_COUNT entries), each entry formatted as follows:
3537
// [MAPPING_INDEX] uint; index into a runtime array where assembly data pointers are stored
@@ -50,8 +52,8 @@ partial class AssemblyStoreGenerator
5052
const uint ASSEMBLY_STORE_MAGIC = 0x41424158; // 'XABA', little-endian, must match the BUNDLED_ASSEMBLIES_BLOB_MAGIC native constant
5153

5254
// Bit 31 is set for 64-bit platforms, cleared for the 32-bit ones
53-
const uint ASSEMBLY_STORE_FORMAT_VERSION_64BIT = 0x80000002; // Must match the ASSEMBLY_STORE_FORMAT_VERSION native constant
54-
const uint ASSEMBLY_STORE_FORMAT_VERSION_32BIT = 0x00000002;
55+
const uint ASSEMBLY_STORE_FORMAT_VERSION_64BIT = 0x80000003; // Must match the ASSEMBLY_STORE_FORMAT_VERSION native constant
56+
const uint ASSEMBLY_STORE_FORMAT_VERSION_32BIT = 0x00000003;
5557

5658
const uint ASSEMBLY_STORE_ABI_AARCH64 = 0x00010000;
5759
const uint ASSEMBLY_STORE_ABI_ARM = 0x00020000;
@@ -122,19 +124,29 @@ string Generate (string baseOutputDirectory, AndroidTargetArch arch, List<Assemb
122124
using var fs = File.Open (storePath, FileMode.Create, FileAccess.Write, FileShare.Read);
123125
fs.Seek ((long)curPos, SeekOrigin.Begin);
124126

127+
uint mappingIndex = 0;
125128
foreach (AssemblyStoreAssemblyInfo info in infos) {
126129
(AssemblyStoreEntryDescriptor desc, curPos) = MakeDescriptor (info, curPos);
127-
desc.mapping_index = (uint)descriptors.Count;
130+
if (info.Ignored) {
131+
desc.mapping_index = 0;
132+
} else {
133+
desc.mapping_index = mappingIndex++;
134+
}
135+
uint entryIndex = (uint)descriptors.Count;
128136
descriptors.Add (desc);
129137

130-
if ((uint)fs.Position != desc.data_offset) {
138+
if (!info.Ignored && (uint)fs.Position != desc.data_offset) {
131139
throw new InvalidOperationException ($"Internal error: corrupted store '{storePath}' stream");
132140
}
133141

134142
ulong name_with_ext_hash = MonoAndroidHelper.GetXxHash (info.AssemblyNameBytes, is64Bit);
135143
ulong name_no_ext_hash = MonoAndroidHelper.GetXxHash (info.AssemblyNameNoExtBytes, is64Bit);
136-
index.Add (new AssemblyStoreIndexEntry (info.AssemblyName, name_with_ext_hash, desc.mapping_index));
137-
index.Add (new AssemblyStoreIndexEntry (info.AssemblyNameNoExt, name_no_ext_hash, desc.mapping_index));
144+
index.Add (new AssemblyStoreIndexEntry (info.AssemblyName, name_with_ext_hash, entryIndex, info.Ignored));
145+
index.Add (new AssemblyStoreIndexEntry (info.AssemblyNameNoExt, name_no_ext_hash, entryIndex, info.Ignored));
146+
147+
if (info.Ignored) {
148+
continue;
149+
}
138150

139151
CopyData (info.SourceFile, fs, storePath);
140152
CopyData (info.SymbolsFile, fs, storePath);
@@ -184,8 +196,8 @@ void CopyData (FileInfo? src, Stream dest, string storePath)
184196
static (AssemblyStoreEntryDescriptor desc, ulong newPos) MakeDescriptor (AssemblyStoreAssemblyInfo info, ulong curPos)
185197
{
186198
var ret = new AssemblyStoreEntryDescriptor {
187-
data_offset = (uint)curPos,
188-
data_size = GetDataLength (info.SourceFile),
199+
data_offset = info.Ignored ? 0 : (uint)curPos,
200+
data_size = info.Ignored ? 0 : GetDataLength (info.SourceFile),
189201
};
190202
if (info.SymbolsFile != null) {
191203
ret.debug_data_offset = ret.data_offset + ret.data_size;
@@ -197,9 +209,11 @@ void CopyData (FileInfo? src, Stream dest, string storePath)
197209
ret.config_data_size = GetDataLength (info.ConfigFile);
198210
}
199211

200-
curPos += ret.data_size + ret.debug_data_size + ret.config_data_size;
201-
if (curPos > UInt32.MaxValue) {
202-
throw new NotSupportedException ("Assembly store size exceeds the maximum supported value");
212+
if (!info.Ignored) {
213+
curPos += ret.data_size + ret.debug_data_size + ret.config_data_size;
214+
if (curPos > UInt32.MaxValue) {
215+
throw new NotSupportedException ("Assembly store size exceeds the maximum supported value");
216+
}
203217
}
204218

205219
return (ret, curPos);
@@ -252,8 +266,9 @@ void WriteIndex (BinaryWriter writer, StreamWriter manifestWriter, List<Assembly
252266
manifestWriter.Write ($"0x{(uint)entry.name_hash:x}");
253267
}
254268
writer.Write (entry.descriptor_index);
255-
manifestWriter.Write ($" di:{entry.descriptor_index}");
269+
writer.Write ((byte)(entry.ignore ? 1 : 0));
256270

271+
manifestWriter.Write ($" di:{entry.descriptor_index}");
257272
AssemblyStoreEntryDescriptor desc = descriptors[(int)entry.descriptor_index];
258273
manifestWriter.Write ($" mi:{desc.mapping_index}");
259274
manifestWriter.Write ($" do:{desc.data_offset}");
@@ -262,7 +277,11 @@ void WriteIndex (BinaryWriter writer, StreamWriter manifestWriter, List<Assembly
262277
manifestWriter.Write ($" dds:{desc.debug_data_size}");
263278
manifestWriter.Write ($" cdo:{desc.config_data_offset}");
264279
manifestWriter.Write ($" cds:{desc.config_data_size}");
265-
manifestWriter.WriteLine ($" {entry.name}");
280+
manifestWriter.Write ($" {entry.name}");
281+
if (entry.ignore) {
282+
manifestWriter.Write (" (ignored)");
283+
}
284+
manifestWriter.WriteLine ();
266285
}
267286
}
268287

@@ -285,7 +304,9 @@ List<AssemblyStoreIndexEntry> ReadIndex (BinaryReader reader, AssemblyStoreHeade
285304
}
286305

287306
uint descriptor_index = reader.ReadUInt32 ();
288-
index.Add (new AssemblyStoreIndexEntry (String.Empty, name_hash, descriptor_index));
307+
bool ignored = reader.ReadByte () != 0;
308+
309+
index.Add (new AssemblyStoreIndexEntry (String.Empty, name_hash, descriptor_index, ignored));
289310
}
290311

291312
return index;

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2288,6 +2288,7 @@ because xbuild doesn't support framework reference assemblies.
22882288
-->
22892289
<CreateAssemblyStore
22902290
Condition=" '$(_AndroidRuntime)' != 'NativeAOT' "
2291+
TargetRuntime="$(_AndroidRuntime)"
22912292
AppSharedLibrariesDir="$(_AndroidApplicationSharedLibraryPath)"
22922293
IncludeDebugSymbols="$(AndroidIncludeDebugSymbols)"
22932294
ResolvedFrameworkAssemblies="@(_BuildApkResolvedFrameworkAssemblies)"

src/native/clr/host/assembly-store.cc

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,8 @@ auto AssemblyStore::find_assembly_store_entry (hash_t hash, const AssemblyStoreI
195195
auto AssemblyStore::open_assembly (std::string_view const& name, int64_t &size) noexcept -> void*
196196
{
197197
hash_t name_hash = xxhash::hash (name.data (), name.length ());
198-
log_debug (LOG_ASSEMBLY, "AssemblyStore::open_assembly: looking for bundled name: '{}' (hash {:x})"sv, optional_string (name.data ()), name_hash);
199198

200199
if constexpr (Constants::is_debug_build) {
201-
// TODO: implement filesystem lookup here
202-
203200
// In fastdev mode we might not have any assembly store.
204201
if (assembly_store_hashes == nullptr) {
205202
log_warn (LOG_ASSEMBLY, "Assembly store not registered. Unable to look up assembly '{}'"sv, name);
@@ -208,12 +205,15 @@ auto AssemblyStore::open_assembly (std::string_view const& name, int64_t &size)
208205
}
209206

210207
const AssemblyStoreIndexEntry *hash_entry = find_assembly_store_entry (name_hash, assembly_store_hashes, assembly_store.index_entry_count);
211-
if (hash_entry == nullptr) {
212-
// This message should really be `log_warn`, but since CoreCLR attempts to load `AssemblyName.ni.dll` for each
213-
// `AssemblyName.dll`, it creates a lot of non-actionable noise.
214-
// TODO (in separate PR): generate hashes for the .ni.dll names and ignore them at the top of the function. Then restore
215-
// `log_warn` here.
216-
log_debug (LOG_ASSEMBLY, "Assembly '{}' (hash 0x{:x}) not found"sv, optional_string (name.data ()), name_hash);
208+
if (hash_entry == nullptr) [[unlikely]] {
209+
size = 0;
210+
log_warn (LOG_ASSEMBLY, "Assembly '{}' (hash 0x{:x}) not found"sv, name, name_hash);
211+
return nullptr;
212+
}
213+
214+
if (hash_entry->ignore != 0) {
215+
size = 0;
216+
log_debug (LOG_ASSEMBLY, "Assembly '{}' ignored"sv, name);
217217
return nullptr;
218218
}
219219

src/native/clr/include/constants.hh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <fcntl.h>
44
#include <sys/system_properties.h>
55

6+
#include <limits>
67
#include <string_view>
78

89
#include <shared/cpp-util.hh>

src/native/clr/include/host/assembly-store.hh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#pragma once
22

33
#include <cstdint>
4+
#include <limits>
45
#include <mutex>
56
#include <string_view>
67
#include <tuple>

src/native/clr/include/xamarin-app.hh

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ static constexpr uint32_t ASSEMBLY_STORE_ABI = 0x00040000;
3131
#endif
3232

3333
// Increase whenever an incompatible change is made to the assembly store format
34-
static constexpr uint32_t ASSEMBLY_STORE_FORMAT_VERSION = 2 | ASSEMBLY_STORE_64BIT_FLAG | ASSEMBLY_STORE_ABI;
34+
static constexpr uint32_t ASSEMBLY_STORE_FORMAT_VERSION = 3 | ASSEMBLY_STORE_64BIT_FLAG | ASSEMBLY_STORE_ABI;
3535

3636
static constexpr uint32_t MODULE_MAGIC_NAMES = 0x53544158; // 'XATS', little-endian
3737
static constexpr uint32_t MODULE_INDEX_MAGIC = 0x49544158; // 'XATI', little-endian
@@ -153,6 +153,7 @@ struct XamarinAndroidBundledAssembly
153153
// [HEADER]
154154
// [INDEX]
155155
// [ASSEMBLY_DESCRIPTORS]
156+
// [ASSEMBLY_NAMES]
156157
// [ASSEMBLY DATA]
157158
//
158159
// Formats of the sections above are as follows:
@@ -167,6 +168,7 @@ struct XamarinAndroidBundledAssembly
167168
// INDEX (variable size, HEADER.ENTRY_COUNT*2 entries, for assembly names with and without the extension)
168169
// [NAME_HASH] uint on 32-bit platforms, ulong on 64-bit platforms; xxhash of the assembly name
169170
// [DESCRIPTOR_INDEX] uint; index into in-store assembly descriptor array
171+
// [IGNORE] byte; if set to anything other than 0, the assembly is to be ignored when loading
170172
//
171173
// ASSEMBLY_DESCRIPTORS (variable size, HEADER.ENTRY_COUNT entries), each entry formatted as follows:
172174
// [MAPPING_INDEX] uint; index into a runtime array where assembly data pointers are stored
@@ -199,6 +201,7 @@ struct [[gnu::packed]] AssemblyStoreIndexEntry final
199201
{
200202
xamarin::android::hash_t name_hash;
201203
uint32_t descriptor_index;
204+
uint8_t ignore; // Assembly should be ignored when loading, its data isn't actually there
202205
};
203206

204207
struct [[gnu::packed]] AssemblyStoreEntryDescriptor final

0 commit comments

Comments
 (0)