Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Marshal methods, Part 1 of ? #7004

Merged
merged 14 commits into from
Jun 6, 2022
Merged

Marshal methods, Part 1 of ? #7004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1c9a211
First steps towards marshal methods generation
grendello Mar 29, 2022
dcf5ebc
[WIP] MonoImage loader class
grendello May 5, 2022
83515ef
Don't silently cast `MonoError*` to `bool`, doh
grendello May 6, 2022
e16eb40
Marshal methods POC works
grendello May 12, 2022
2cd2e70
Baby steps
grendello May 12, 2022
41c186d
More baby steps
grendello May 13, 2022
a8bca69
Prepare for PR
grendello May 16, 2022
e3ce6a8
Do NOT enable marshal methods by default
grendello May 16, 2022
ef70c2a
Disable one more bit of code by default
grendello May 16, 2022
ccda3f3
Update apkdesc files
grendello May 17, 2022
df365df
Update for the recent main changes
grendello May 18, 2022
42f4a69
Rearrange some code
grendello May 20, 2022
799a72b
Use the same JI as main
grendello May 20, 2022
2d27ded
Update apkdesc files
grendello May 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
<RollForward>Major</RollForward>
</PropertyGroup>

<PropertyGroup Condition=" '$(MSBuildRuntimeType)' == 'Core' ">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is building with dotnet build a requirement for this one? When you build inside Visual Studio, it will be Full using .NET Framework MSBuild.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, marshal methods will be NET7+ only

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, but you can build .NET 7 projects in Visual Studio with .NET Framework MSBuild.

Does this feature require our build to be running under .NET 7?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can maybe shell out to dotnet to make this work, if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Native portion of the feature uses APIs that aren't available in Mono classic, so I wanted to make sure it's not enabled for classic by accident. However, this properly is here just until I finish all the parts of the feature, it will be
removed in the last PR. It's just easier this way for me to keep the feature disabled in main while I can enable it globally in the branch I'm working in on the next PRs.

<_EnableMarshalMethods>NoThanks</_EnableMarshalMethods> <!-- set to YesPlease to enable -->
</PropertyGroup>

<PropertyGroup>
<ProductVersion>12.3.99</ProductVersion>
<!-- NuGet package version numbers. See Documentation/guides/OneDotNet.md.
Expand Down
11 changes: 11 additions & 0 deletions src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ namespace Xamarin.Android.Tasks

public class GenerateJavaStubs : AndroidTask
{
public const string MarshalMethodsRegisterTaskKey = ".:!MarshalMethods!:.";

public override string TaskPrefix => "GJS";

[Required]
Expand Down Expand Up @@ -338,6 +340,9 @@ bool CreateJavaSources (IEnumerable<TypeDefinition> javaTypes, TypeDefinitionCac
string monoInit = GetMonoInitSource (AndroidSdkPlatform);
bool hasExportReference = ResolvedAssemblies.Any (assembly => Path.GetFileName (assembly.ItemSpec) == "Mono.Android.Export.dll");
bool generateOnCreateOverrides = int.Parse (AndroidSdkPlatform) <= 10;
#if ENABLE_MARSHAL_METHODS
var overriddenMethodDescriptors = new List<OverriddenMethodDescriptor> ();
#endif

bool ok = true;
foreach (var t in javaTypes) {
Expand All @@ -355,6 +360,9 @@ bool CreateJavaSources (IEnumerable<TypeDefinition> javaTypes, TypeDefinitionCac
};

jti.Generate (writer);
#if ENABLE_MARSHAL_METHODS
overriddenMethodDescriptors.AddRange (jti.OverriddenMethodDescriptors);
#endif
writer.Flush ();

var path = jti.GetDestinationPath (outputPath);
Expand Down Expand Up @@ -388,6 +396,9 @@ bool CreateJavaSources (IEnumerable<TypeDefinition> javaTypes, TypeDefinitionCac
}
}
}
#if ENABLE_MARSHAL_METHODS
BuildEngine4.RegisterTaskObjectAssemblyLocal (MarshalMethodsRegisterTaskKey, overriddenMethodDescriptors, RegisteredTaskObjectLifetime.Build);
#endif
return ok;
}

Expand Down
39 changes: 32 additions & 7 deletions src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,16 @@ void AddEnvironment ()

int assemblyCount = 0;
HashSet<string> archAssemblyNames = null;

#if ENABLE_MARSHAL_METHODS
var uniqueAssemblyNames = new HashSet<string> (StringComparer.OrdinalIgnoreCase);
#endif
Action<ITaskItem> updateAssemblyCount = (ITaskItem assembly) => {
string assemblyName = Path.GetFileName (assembly.ItemSpec);
#if ENABLE_MARSHAL_METHODS
if (!uniqueAssemblyNames.Contains (assemblyName)) {
uniqueAssemblyNames.Add (assemblyName);
}
#endif
if (!UseAssemblyStore) {
assemblyCount++;
return;
Expand All @@ -316,7 +324,6 @@ void AddEnvironment ()
} else {
archAssemblyNames ??= new HashSet<string> (StringComparer.OrdinalIgnoreCase);

string assemblyName = Path.GetFileName (assembly.ItemSpec);
if (!archAssemblyNames.Contains (assemblyName)) {
assemblyCount++;
archAssemblyNames.Add (assemblyName);
Expand Down Expand Up @@ -431,16 +438,34 @@ void AddEnvironment ()
JNIEnvRegisterJniNativesToken = jnienv_registerjninatives_method_token,
};
appConfigAsmGen.Init ();

#if ENABLE_MARSHAL_METHODS
var marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator () {
NumberOfAssembliesInApk = assemblyCount,
UniqueAssemblyNames = uniqueAssemblyNames,
OverriddenMethodDescriptors = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal<List<Java.Interop.Tools.JavaCallableWrappers.OverriddenMethodDescriptor>> (GenerateJavaStubs.MarshalMethodsRegisterTaskKey, RegisteredTaskObjectLifetime.Build)
};
marshalMethodsAsmGen.Init ();
#endif
foreach (string abi in SupportedAbis) {
string baseAsmFilePath = Path.Combine (EnvironmentOutputDirectory, $"environment.{abi.ToLowerInvariant ()}");
string llFilePath = $"{baseAsmFilePath}.ll";
string targetAbi = abi.ToLowerInvariant ();
string environmentBaseAsmFilePath = Path.Combine (EnvironmentOutputDirectory, $"environment.{targetAbi}");
string marshalMethodsBaseAsmFilePath = Path.Combine (EnvironmentOutputDirectory, $"marshal_methods.{targetAbi}");
string environmentLlFilePath = $"{environmentBaseAsmFilePath}.ll";
string marshalMethodsLlFilePath = $"{marshalMethodsBaseAsmFilePath}.ll";

AndroidTargetArch targetArch = GetAndroidTargetArchForAbi (abi);
using (var sw = MemoryStreamPool.Shared.CreateStreamWriter ()) {
appConfigAsmGen.Write (targetArch, sw, environmentLlFilePath);
sw.Flush ();
Files.CopyIfStreamChanged (sw.BaseStream, environmentLlFilePath);
}
#if ENABLE_MARSHAL_METHODS
using (var sw = MemoryStreamPool.Shared.CreateStreamWriter ()) {
appConfigAsmGen.Write (GetAndroidTargetArchForAbi (abi), sw, llFilePath);
marshalMethodsAsmGen.Write (targetArch, sw, marshalMethodsLlFilePath);
sw.Flush ();
Files.CopyIfStreamChanged (sw.BaseStream, llFilePath);
Files.CopyIfStreamChanged (sw.BaseStream, marshalMethodsLlFilePath);
}
#endif
}

void AddEnvironmentVariable (string name, string value)
Expand Down
8 changes: 7 additions & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/PrepareAbiItems.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ public class PrepareAbiItems : AndroidTask
const string TypeMapBase = "typemaps";
const string EnvBase = "environment";
const string CompressedAssembliesBase = "compressed_assemblies";

#if ENABLE_MARSHAL_METHODS
const string MarshalMethodsBase = "marshal_methods";
#endif
public override string TaskPrefix => "PAI";

[Required]
Expand Down Expand Up @@ -50,6 +52,10 @@ public override bool RunTask ()
baseName = EnvBase;
} else if (String.Compare ("compressed", Mode, StringComparison.OrdinalIgnoreCase) == 0) {
baseName = CompressedAssembliesBase;
#if ENABLE_MARSHAL_METHODS
} else if (String.Compare ("marshal_methods", Mode, StringComparison.OrdinalIgnoreCase) == 0) {
baseName = MarshalMethodsBase;
#endif
} else {
Log.LogError ($"Unknown mode: {Mode}");
return false;
Expand Down
12 changes: 6 additions & 6 deletions ...ild.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"Size": 60736
},
"assemblies/Mono.Android.dll": {
"Size": 152435
"Size": 152344
},
"assemblies/rc.bin": {
"Size": 1083
Expand Down Expand Up @@ -59,13 +59,13 @@
"Size": 9591
},
"assemblies/UnnamedProject.dll": {
"Size": 3560
"Size": 3555
},
"classes.dex": {
"Size": 348440
},
"lib/arm64-v8a/libmonodroid.so": {
"Size": 484512
"Size": 510920
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems odd that libmonodroid.so is ~26KB larger, yet lots of the new C++ code is not even compiled. Weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot more code is inlined now. It's a bit surprising it generates that much of a difference, tbh.

},
"lib/arm64-v8a/libmonosgen-2.0.so": {
"Size": 4667768
Expand All @@ -80,7 +80,7 @@
"Size": 146816
},
"lib/arm64-v8a/libxamarin-app.so": {
"Size": 15944
"Size": 15904
},
"META-INF/BNDLTOOL.RSA": {
"Size": 1213
Expand All @@ -92,7 +92,7 @@
"Size": 3646
},
"res/drawable-hdpi-v4/icon.png": {
"Size": 4762
"Size": 4791
},
"res/drawable-mdpi-v4/icon.png": {
"Size": 2200
Expand All @@ -116,5 +116,5 @@
"Size": 1904
}
},
"PackageSize": 3492724
"PackageSize": 3500916
}
10 changes: 5 additions & 5 deletions ...ild.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleLegacy.apkdesc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
"Size": 2604
},
"assemblies/Java.Interop.dll": {
"Size": 67948
"Size": 67950
},
"assemblies/Mono.Android.dll": {
"Size": 257194
"Size": 257187
},
"assemblies/mscorlib.dll": {
"Size": 769016
Expand All @@ -23,7 +23,7 @@
"Size": 2879
},
"classes.dex": {
"Size": 349820
"Size": 349736
},
"lib/arm64-v8a/libmono-btls-shared.so": {
"Size": 1613872
Expand All @@ -32,7 +32,7 @@
"Size": 750976
},
"lib/arm64-v8a/libmonodroid.so": {
"Size": 392576
"Size": 421872
},
"lib/arm64-v8a/libmonosgen-2.0.so": {
"Size": 4030448
Expand Down Expand Up @@ -74,5 +74,5 @@
"Size": 1724
}
},
"PackageSize": 4044500
"PackageSize": 4052692
}
12 changes: 6 additions & 6 deletions ...ild.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.apkdesc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"Size": 66806
},
"assemblies/Mono.Android.dll": {
"Size": 447633
"Size": 447543
},
"assemblies/mscorlib.dll": {
"Size": 3888
Expand Down Expand Up @@ -119,7 +119,7 @@
"Size": 1906
},
"assemblies/UnnamedProject.dll": {
"Size": 117250
"Size": 117244
},
"assemblies/Xamarin.AndroidX.Activity.dll": {
"Size": 5941
Expand Down Expand Up @@ -191,7 +191,7 @@
"Size": 3460820
},
"lib/arm64-v8a/libmonodroid.so": {
"Size": 484512
"Size": 510920
},
"lib/arm64-v8a/libmonosgen-2.0.so": {
"Size": 4667768
Expand All @@ -206,7 +206,7 @@
"Size": 146816
},
"lib/arm64-v8a/libxamarin-app.so": {
"Size": 98480
"Size": 98440
},
"META-INF/android.support.design_material.version": {
"Size": 12
Expand Down Expand Up @@ -773,7 +773,7 @@
"Size": 470
},
"res/drawable-hdpi-v4/icon.png": {
"Size": 4762
"Size": 4791
},
"res/drawable-hdpi-v4/notification_bg_low_normal.9.png": {
"Size": 212
Expand Down Expand Up @@ -1961,5 +1961,5 @@
"Size": 341228
}
},
"PackageSize": 8347981
"PackageSize": 8356173
}
20 changes: 10 additions & 10 deletions ...ild.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsLegacy.apkdesc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
"Size": 7215
},
"assemblies/Java.Interop.dll": {
"Size": 68913
"Size": 68916
},
"assemblies/Mono.Android.dll": {
"Size": 567718
"Size": 567711
},
"assemblies/Mono.Security.dll": {
"Size": 68432
Expand Down Expand Up @@ -65,22 +65,22 @@
"Size": 131930
},
"assemblies/Xamarin.AndroidX.DrawerLayout.dll": {
"Size": 15426
"Size": 15425
},
"assemblies/Xamarin.AndroidX.Fragment.dll": {
"Size": 43135
"Size": 43134
},
"assemblies/Xamarin.AndroidX.Legacy.Support.Core.UI.dll": {
"Size": 6715
"Size": 6714
},
"assemblies/Xamarin.AndroidX.Lifecycle.Common.dll": {
"Size": 7062
},
"assemblies/Xamarin.AndroidX.Lifecycle.LiveData.Core.dll": {
"Size": 7193
"Size": 7194
},
"assemblies/Xamarin.AndroidX.Lifecycle.ViewModel.dll": {
"Size": 4872
"Size": 4873
},
"assemblies/Xamarin.AndroidX.Loader.dll": {
"Size": 13585
Expand Down Expand Up @@ -113,7 +113,7 @@
"Size": 43497
},
"classes.dex": {
"Size": 3462252
"Size": 3462080
},
"lib/arm64-v8a/libmono-btls-shared.so": {
"Size": 1613872
Expand All @@ -122,7 +122,7 @@
"Size": 750976
},
"lib/arm64-v8a/libmonodroid.so": {
"Size": 392576
"Size": 421872
},
"lib/arm64-v8a/libmonosgen-2.0.so": {
"Size": 4030448
Expand Down Expand Up @@ -1883,5 +1883,5 @@
"Size": 341040
}
},
"PackageSize": 9562270
"PackageSize": 9570462
}
14 changes: 1 addition & 13 deletions src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Text;

using Java.Interop.Tools.TypeNameMappings;
using K4os.Hash.xxHash;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;

Expand Down Expand Up @@ -342,21 +340,11 @@ void WriteDSOCache (LlvmIrGenerator generator)

// We need to hash here, because the hash is architecture-specific
foreach (StructureInstance<DSOCacheEntry> entry in dsoCache) {
entry.Obj.hash = HashName (entry.Obj.HashedName);
entry.Obj.hash = HashName (entry.Obj.HashedName, is64Bit);
}
dsoCache.Sort ((StructureInstance<DSOCacheEntry> a, StructureInstance<DSOCacheEntry> b) => a.Obj.hash.CompareTo (b.Obj.hash));

generator.WriteStructureArray (dsoCacheEntryStructureInfo, dsoCache, "dso_cache");

ulong HashName (string name)
{
byte[] nameBytes = Encoding.UTF8.GetBytes (name);
if (is64Bit) {
return XXH64.DigestOf (nameBytes, 0, nameBytes.Length);
}

return (ulong)XXH32.DigestOf (nameBytes, 0, nameBytes.Length);
}
}
}
}