Skip to content

Commit

Permalink
[One .NET] better logic for deduplication of architecture-specific as…
Browse files Browse the repository at this point in the history 
…semblies (#5276)

Fixes: #5263

In .NET 6, we currently run the `ILLink` MSBuild target for each
`$(RuntimeIdentifier)`, as the Mono runtime packs have some .NET
assemblies that differ per architecture.

To make this happen, the `<ProcessAssemblies/>` task (51fb93e)
works with inputs such as:

  * `android.21-arm\linked\Foo.dll`
  * `android.21-x86\linked\Foo.dll`
  * `android.21-arm\linked\System.Private.CoreLib.dll`
  * `android.21-x86\linked\System.Private.CoreLib.dll`

and "merges" this to what we need in the output of the `.apk`:

  * `assemblies/Foo.dll`
  * `assemblies/armeabi-v7a/System.Private.CoreLib.dll`
  * `assemblies/x86/System.Private.CoreLib.dll`

`Foo.dll` is identical for all architectures, so we only need to use
a single copy of it.

A bug in our implementation was for an assembly that differed between
32-bit and 64-bit platforms, but was identical within those sets:

  * `android.21-arm\linked\System.Collections.Concurrent.dll`
  * `android.21-x86\linked\System.Collections.Concurrent.dll`
  * `android.21-arm64\linked\System.Collections.Concurrent.dll`
  
The `-arm` and `-x86` copies of these assemblies have the same MVID.

The `<ProcessAssemblies/>` task would *skip* processing any assembly
with a previously "seen" MVID.  Consequently, the `.apk` contained:

  * `assemblies/armeabi-v7a/System.Collections.Concurrent.dll`
  * `assemblies/arm64-v8a/System.Collections.Concurrent.dll`

Notably absent was a copy in `assemblies/x86`.

The result was a runtime crash when running on the x86 emulator:

	Unhandled Exception:
	System.TypeLoadException: Could not set up parent class, due to: 
	  Could not load type of field 'Java.Interop.JniRuntime:TrackedInstances' (31) due to:
	    Could not load file or assembly 'System.Collections.Concurrent, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. assembly:Java.Interop.dll type:JniRuntime member:(null)

I reworked the logic to instead do:

  * Group by assembly file name
  * Count the unique MVIDs
  * If we have only 1 MVID, we just need the first assembly.
  * Otherwise, place the assemblies in architecture-specific directories.

The resulting code seems like it is a bit simpler, and likely a
little faster.  I was able to call
`MonoAndroidHelper.HasMonoAndroidReference()` once per file name,
which should help performance.

I updated `XASdkTests.DotNetBuild()` to check this scenario.
  • Loading branch information
@jonathanpeppers
jonathanpeppers authored Nov 11, 2020
1 parent 810f10e commit 55e5c34
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 41 deletions.
94 changes: 55 additions & 39 deletions src/Xamarin.Android.Build.Tasks/Tasks/ProcessAssemblies.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public class ProcessAssemblies : AndroidTask

public override bool RunTask ()
{
var output = new Dictionary<Guid, ITaskItem> ();
var output = new List<ITaskItem> ();
var symbols = new Dictionary<string, ITaskItem> ();

if (ResolvedSymbols != null) {
Expand All @@ -45,53 +45,60 @@ public override bool RunTask ()
}
}

foreach (var assembly in InputAssemblies) {
if (!File.Exists (assembly.ItemSpec)) {
Log.LogDebugMessage ($"Skipping non-existent dependency '{assembly.ItemSpec}'.");
continue;
}
using (var pe = new PEReader (File.OpenRead (assembly.ItemSpec))) {
var reader = pe.GetMetadataReader ();
var module = reader.GetModuleDefinition ();
var mvid = reader.GetGuid (module.Mvid);
if (!output.ContainsKey (mvid)) {
output.Add (mvid, assembly);
// Group by assembly file name
foreach (var group in InputAssemblies.Where (Filter).GroupBy (a => Path.GetFileName (a.ItemSpec))) {
// Get the unique list of MVIDs
var mvids = new HashSet<Guid> ();
bool? frameworkAssembly = null, hasMonoAndroidReference = null;
foreach (var assembly in group) {
using (var pe = new PEReader (File.OpenRead (assembly.ItemSpec))) {
var reader = pe.GetMetadataReader ();
var module = reader.GetModuleDefinition ();
var mvid = reader.GetGuid (module.Mvid);
mvids.Add (mvid);

// Set metadata, such as %(FrameworkAssembly) and %(HasMonoAndroidReference)
string packageId = assembly.GetMetadata ("NuGetPackageId");
bool frameworkAssembly = packageId.StartsWith ("Microsoft.NETCore.App.Runtime.") ||
packageId.StartsWith ("Microsoft.Android.Runtime.");
// Calculate %(FrameworkAssembly) and %(HasMonoAndroidReference) for the first
if (frameworkAssembly == null) {
string packageId = assembly.GetMetadata ("NuGetPackageId") ?? "";
frameworkAssembly = packageId.StartsWith ("Microsoft.NETCore.App.Runtime.") ||
packageId.StartsWith ("Microsoft.Android.Runtime.");
}
if (hasMonoAndroidReference == null) {
hasMonoAndroidReference = MonoAndroidHelper.HasMonoAndroidReference (reader);
}
assembly.SetMetadata ("FrameworkAssembly", frameworkAssembly.ToString ());
assembly.SetMetadata ("HasMonoAndroidReference", MonoAndroidHelper.HasMonoAndroidReference (reader).ToString ());
} else {
symbols.Remove (Path.ChangeExtension (assembly.ItemSpec, ".pdb"));
assembly.SetMetadata ("HasMonoAndroidReference", hasMonoAndroidReference.ToString ());
}
}
}

OutputAssemblies = output.Values.ToArray ();
ResolvedSymbols = symbols.Values.ToArray ();

// Set %(DestinationSubDirectory) and %(DestinationSubPath) for architecture-specific assemblies
var fileNames = new Dictionary<string, ITaskItem> (StringComparer.OrdinalIgnoreCase);
foreach (var assembly in OutputAssemblies) {
var fileName = Path.GetFileName (assembly.ItemSpec);
symbols.TryGetValue (Path.ChangeExtension (assembly.ItemSpec, ".pdb"), out var symbol);
if (fileNames.TryGetValue (fileName, out ITaskItem other)) {
SetDestinationSubDirectory (assembly, fileName, symbol);
if (other != null) {
symbols.TryGetValue (Path.ChangeExtension (other.ItemSpec, ".pdb"), out symbol);
SetDestinationSubDirectory (other, fileName, symbol);
// We don't need to check "other" again
fileNames [fileName] = null;
// If we end up with more than 1 unique mvid, we need *all* assemblies
if (mvids.Count > 1) {
foreach (var assembly in group) {
symbols.TryGetValue (Path.ChangeExtension (assembly.ItemSpec, ".pdb"), out var symbol);
SetDestinationSubDirectory (assembly, group.Key, symbol);
output.Add (assembly);
}
} else {
fileNames.Add (fileName, assembly);
assembly.SetDestinationSubPath ();
symbol?.SetDestinationSubPath ();
// Otherwise only include the first assembly
bool first = true;
foreach (var assembly in group) {
var symbolPath = Path.ChangeExtension (assembly.ItemSpec, ".pdb");
if (first) {
first = false;
if (symbols.TryGetValue (symbolPath, out var symbol)) {
symbol.SetDestinationSubPath ();
}
assembly.SetDestinationSubPath ();
output.Add (assembly);
} else {
symbols.Remove (symbolPath);
}
}
}
}

OutputAssemblies = output.ToArray ();
ResolvedSymbols = symbols.Values.ToArray ();

// Set ShrunkAssemblies for _RemoveRegisterAttribute and <BuildApk/>
if (PublishTrimmed) {
ShrunkAssemblies = OutputAssemblies.Select (a => {
Expand All @@ -106,6 +113,15 @@ public override bool RunTask ()
return !Log.HasLoggedErrors;
}

bool Filter (ITaskItem assembly)
{
if (!File.Exists (assembly.ItemSpec)) {
Log.LogDebugMessage ($"Skipping non-existent dependency '{assembly.ItemSpec}'.");
return false;
}
return true;
}

/// <summary>
/// Sets %(DestinationSubDirectory) and %(DestinationSubPath) based on %(RuntimeIdentifier)
/// </summary>
Expand Down
11 changes: 9 additions & 2 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/XASdkTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,10 @@ public void DotNetBuildBinding ()
/* runtimeIdentifiers */ "android.21-arm;android.21-arm64;android.21-x86;android.21-x64",
/* isRelease */ false,
},
new object [] {
/* runtimeIdentifiers */ "android.21-arm;android.21-arm64;android.21-x86",
/* isRelease */ true,
},
new object [] {
/* runtimeIdentifiers */ "android.21-arm;android.21-arm64;android.21-x86;android.21-x64",
/* isRelease */ true,
Expand Down Expand Up @@ -374,14 +378,17 @@ public void DotNetBuild (string runtimeIdentifiers, bool isRelease)
using (var apk = ZipHelper.OpenZip (apkPath)) {
apk.AssertContainsEntry (apkPath, $"assemblies/{proj.ProjectName}.dll", shouldContainEntry: expectEmbeddedAssembies);
apk.AssertContainsEntry (apkPath, $"assemblies/{proj.ProjectName}.pdb", shouldContainEntry: !CommercialBuildAvailable && !isRelease);
apk.AssertContainsEntry (apkPath, $"assemblies/System.Linq.dll", shouldContainEntry: expectEmbeddedAssembies);
var rids = runtimeIdentifiers.Split (';');
foreach (var abi in rids.Select (MonoAndroidHelper.RuntimeIdentifierToAbi)) {
apk.AssertContainsEntry (apkPath, $"lib/{abi}/libmonodroid.so");
apk.AssertContainsEntry (apkPath, $"lib/{abi}/libmonosgen-2.0.so");
if (rids.Length > 1) {
apk.AssertContainsEntry (apkPath, $"assemblies/{abi}/System.Private.CoreLib.dll", shouldContainEntry: expectEmbeddedAssembies);
apk.AssertContainsEntry (apkPath, $"assemblies/{abi}/System.Private.CoreLib.dll", shouldContainEntry: expectEmbeddedAssembies);
apk.AssertContainsEntry (apkPath, $"assemblies/{abi}/System.Collections.Concurrent.dll", shouldContainEntry: expectEmbeddedAssembies);
} else {
apk.AssertContainsEntry (apkPath, "assemblies/System.Private.CoreLib.dll", shouldContainEntry: expectEmbeddedAssembies);
apk.AssertContainsEntry (apkPath, "assemblies/System.Private.CoreLib.dll", shouldContainEntry: expectEmbeddedAssembies);
apk.AssertContainsEntry (apkPath, "assemblies/System.Collections.Concurrent.dll", shouldContainEntry: expectEmbeddedAssembies);
}
}
}
Expand Down

0 comments on commit 55e5c34

Please sign in to comment.