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.

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

[Experimental] Add alternative way of resolving UCO function pointers for Marshal Methods #9805

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Mono.Android/Android.Runtime/JNIEnvInit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ internal struct JnienvInitializeArgs {
public bool marshalMethodsEnabled;
public IntPtr grefGCUserPeerable;
public uint runtimeType;
public bool managedMarshalMethodsLookupEnabled;
}
#pragma warning restore 0649

Expand Down Expand Up @@ -122,9 +123,17 @@ internal static unsafe void Initialize (JnienvInitializeArgs* args)
}
}

if (args->managedMarshalMethodsLookupEnabled) {
delegate* unmanaged <int, int, int, IntPtr*, void> getFunctionPointer = &ManagedMarshalMethodsLookupTable.GetFunctionPointer;
xamarin_app_init (args->env, getFunctionPointer);
}

SetSynchronizationContext ();
}

[DllImport ("xamarin-app")]
static extern unsafe void xamarin_app_init (IntPtr env, delegate* unmanaged <int, int, int, IntPtr*, void> get_function_pointer);

// NOTE: prevents Android.App.Application static ctor from running
[MethodImpl (MethodImplOptions.NoInlining)]
static void SetSynchronizationContext () =>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
using System;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

using Android.Runtime;

namespace Java.Interop;

internal class ManagedMarshalMethodsLookupTable
{
[UnmanagedCallersOnly]
internal static unsafe void GetFunctionPointer (int assemblyIndex, int classIndex, int methodIndex, IntPtr* target)
{
try {
IntPtr result = GetFunctionPointer (assemblyIndex, classIndex, methodIndex);
if (result == IntPtr.Zero || result == (IntPtr)(-1)) {
throw new InvalidOperationException ($"Failed to get function pointer for ({assemblyIndex}, {classIndex}, {methodIndex})");
}

*target = result;
} catch (Exception ex) {
AndroidEnvironment.UnhandledException (ex);
AndroidEnvironment.FailFast ("GetFunctionPointer failed: should not be reached");
}
}

static IntPtr GetFunctionPointer (int assemblyIndex, int classIndex, int methodIndex)
{
// ManagedMarshalMethodsLookupGenerator generates the body of this method is generated at app build time
throw new NotImplementedException ();
}
}
1 change: 1 addition & 0 deletions src/Mono.Android/Mono.Android.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@
<Compile Include="Java.Interop\JavaLocationException.cs" />
<Compile Include="Java.Interop\JavaObjectExtensions.cs" />
<Compile Include="Java.Interop\JavaTypeParametersAttribute.cs" />
<Compile Include="Java.Interop\ManagedMarshalMethodsLookupTable.cs" />
<Compile Include="Java.Interop\Runtime.cs" />
<Compile Include="Java.Interop\TypeManager.cs" />
<Compile Include="Java.Lang\Boolean.cs" />
Expand Down
15 changes: 12 additions & 3 deletions src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public class GenerateJavaStubs : AndroidTask
public bool LinkingEnabled { get; set; }
public bool HaveMultipleRIDs { get; set; }
public bool EnableMarshalMethods { get; set; }
public bool EnableManagedMarshalMethodsLookup { get; set; }

public bool Debug { get; set; }

Expand Down Expand Up @@ -210,8 +211,16 @@ void Run (bool useMarshalMethods)

foreach (var kvp in nativeCodeGenStates) {
NativeCodeGenState state = kvp.Value;
RewriteMarshalMethods (state, brokenExceptionTransitionsEnabled);
state.Classifier.AddSpecialCaseMethods ();
if (!EnableManagedMarshalMethodsLookup) {
RewriteMarshalMethods (state, brokenExceptionTransitionsEnabled);
state.Classifier.AddSpecialCaseMethods ();
} else {
// We need to run `AddSpecialCaseMethods` before `RewriteMarshalMethods` so that we can see the special case
// methods (such as TypeManager.n_Activate_mm) when generating the managed lookup tables.
state.Classifier.AddSpecialCaseMethods ();
state.ManagedMarshalMethodsLookupInfo = new ManagedMarshalMethodsLookupInfo (Log);
RewriteMarshalMethods (state, brokenExceptionTransitionsEnabled);
}
Comment on lines +214 to +223
Copy link
Member Author

Choose a reason for hiding this comment

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

I did not observe any functional changes when swapping the order of AddSpecialCaseMethods and RewriteMarshalMethods. I also don't see a reason that the code in AddSpecialCaseMethods could not run before RewriteMarshalMethods. Nevertheless, I thought it might not be a good idea to change the existing behavior and only change the order when this (experimental) feature is enabled.

Copy link
Member

Choose a reason for hiding this comment

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

That's what CI is for: a safety net. Try it and see! ;-)

We can certainly leave this for now.


Log.LogDebugMessage ($"[{state.TargetArch}] Number of generated marshal methods: {state.Classifier.MarshalMethods.Count}");
if (state.Classifier.RejectedMethodCount > 0) {
Expand Down Expand Up @@ -307,7 +316,7 @@ void RewriteMarshalMethods (NativeCodeGenState state, bool brokenExceptionTransi
return;
}

var rewriter = new MarshalMethodsAssemblyRewriter (Log, state.TargetArch, state.Classifier, state.Resolver);
var rewriter = new MarshalMethodsAssemblyRewriter (Log, state.TargetArch, state.Classifier, state.Resolver, state.ManagedMarshalMethodsLookupInfo);
rewriter.Rewrite (brokenExceptionTransitionsEnabled);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public class GeneratePackageManagerJava : AndroidTask
public bool EnablePreloadAssembliesDefault { get; set; }

public bool EnableMarshalMethods { get; set; }
public bool EnableManagedMarshalMethodsLookup { get; set; }
public string RuntimeConfigBinFilePath { get; set; }
public string BoundExceptionType { get; set; }

Expand Down Expand Up @@ -342,6 +343,7 @@ void AddEnvironment ()
JniRemappingReplacementTypeCount = jniRemappingNativeCodeInfo == null ? 0 : jniRemappingNativeCodeInfo.ReplacementTypeCount,
JniRemappingReplacementMethodIndexEntryCount = jniRemappingNativeCodeInfo == null ? 0 : jniRemappingNativeCodeInfo.ReplacementMethodIndexEntryCount,
MarshalMethodsEnabled = EnableMarshalMethods,
ManagedMarshalMethodsLookupEnabled = EnableManagedMarshalMethodsLookup,
IgnoreSplitConfigs = ShouldIgnoreSplitConfigs (),
};
LLVMIR.LlvmIrModule appConfigModule = appConfigAsmGen.Construct ();
Expand Down Expand Up @@ -370,7 +372,8 @@ void AddEnvironment ()
Log,
assemblyCount,
uniqueAssemblyNames,
EnsureCodeGenState (targetArch)
EnsureCodeGenState (targetArch),
EnableManagedMarshalMethodsLookup
);
} else {
marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ public void XA1037PropertyDeprecatedWarning (string property, string value, bool
XamarinAndroidProject proj = isBindingProject ? new XamarinAndroidBindingProject () : new XamarinAndroidApplicationProject ();
proj.IsRelease = isRelease;
proj.SetProperty (property, value);

using (ProjectBuilder b = isBindingProject ? CreateDllBuilder (Path.Combine ("temp", TestName)) : CreateApkBuilder (Path.Combine ("temp", TestName))) {
Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
Assert.IsTrue (StringAssertEx.ContainsText (b.LastBuildOutput, $"The '{property}' MSBuild property is deprecated and will be removed"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,10 @@ public sealed class ApplicationConfig
public uint jni_remapping_replacement_method_index_entry_count;
public uint mono_components_mask;
public string android_package_name = String.Empty;
public bool managed_marshal_methods_lookup_enabled;
}

const uint ApplicationConfigFieldCount = 26;
const uint ApplicationConfigFieldCount = 27;

const string ApplicationConfigSymbolName = "application_config";
const string AppEnvironmentVariablesSymbolName = "app_environment_variables";
Expand Down Expand Up @@ -335,6 +336,11 @@ static ApplicationConfig ReadApplicationConfig (EnvironmentFile envFile)
Assert.IsTrue (expectedPointerTypes.Contains (field [0]), $"Unexpected pointer field type in '{envFile.Path}:{item.LineNumber}': {field [0]}");
pointers.Add (field [1].Trim ());
break;

case 26: // managed_marshal_methods_lookup_enabled: bool / .byte
AssertFieldType (envFile.Path, parser.SourceFilePath, ".byte", field [0], item.LineNumber);
ret.managed_marshal_methods_lookup_enabled = ConvertFieldToBool ("managed_marshal_methods_lookup_enabled", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
break;
}
fieldCount++;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,6 @@ sealed class ApplicationConfig
[NativeAssembler (NumberFormat = LLVMIR.LlvmIrVariableNumberFormat.Hexadecimal)]
public uint mono_components_mask;
public string android_package_name = String.Empty;
public bool managed_marshal_methods_lookup_enabled;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ sealed class XamarinAndroidBundledAssembly
public PackageNamingPolicy PackageNamingPolicy { get; set; }
public List<ITaskItem> NativeLibraries { get; set; }
public bool MarshalMethodsEnabled { get; set; }
public bool ManagedMarshalMethodsLookupEnabled { get; set; }
public bool IgnoreSplitConfigs { get; set; }

public ApplicationConfigNativeAssemblyGenerator (IDictionary<string, string> environmentVariables, IDictionary<string, string> systemProperties, TaskLoggingHelper log)
Expand Down Expand Up @@ -237,6 +238,7 @@ protected override void Construct (LlvmIrModule module)
have_runtime_config_blob = HaveRuntimeConfigBlob,
have_assemblies_blob = HaveAssemblyStore,
marshal_methods_enabled = MarshalMethodsEnabled,
managed_marshal_methods_lookup_enabled = ManagedMarshalMethodsLookupEnabled,
ignore_split_configs = IgnoreSplitConfigs,
bound_stream_io_exception_type = (byte)BoundExceptionType,
package_naming_policy = (uint)PackageNamingPolicy,
Expand Down
Loading
Loading