MSR: Remove use of reflection in UCOs of generic types #18421

simonrozsival · 2023-06-08T12:35:37Z

In the static UnmanagedCallersOnly methods we don't know the generic parameters of the type we're working with and we need to use this trick to be able to call methods on the generic type without using reflection. When we call a non-generic interface method implemented on a generic class, the .NET runtime will resolve the generic parameters for us. In the implementation of the interface method, we can simply use the generic parameters and generate the same code we usually generate in the UnmanagedCallersOnly callback method.

This is an example of the code we generate in addition to user code:

internal interface __IRegistrarGenericTypeProxy__CustomNSObject_1__
{
	void __IRegistrarGenericTypeProxy__CustomNSObject_1____SomeMethod (IntPtr p0);
}

public class CustomNSObject<T> : NSObject, __IRegistrarGenericTypeProxy__CustomNSObject_1__
	where T : NSObject
{
	[Export ("someMethod:")]
	public void SomeMethod (T someInput)
	{
		// ...
	}

	// generated implementation of the proxy interface:
	public void __IRegistrarGenericTypeProxy__CustomNSObject_1____SomeMethod (IntPtr sel, IntPtr p0, IntPtr* exception_gchandle)
	{
		try {
			var obj0 = (T) Runtime.GetNSObject<T> (p0);
			SomeMethod (obj0);
		} catch (Exception ex) {
			*exception_gchandle = Runtime.AllocGCHandle (ex);
		}
	}

	// generated registrar callbacks:
	private static class __Registrar_Callbacks__
	{
		[UnmanagedCallersOnly (EntryPoint = "_callback_1_CustomNSObject_1_SomeMethod")]
		public unsafe static void callback_1_CustomNSObject_1_SomeMethod (IntPtr pobj, IntPtr sel, IntPtr p0, IntPtr* exception_gchandle)
		{
			var proxy = (__IRegistrarGenericTypeProxy__CustomNSObject_1__)Runtime.GetNSObject (pobj);
			proxy.__IRegistrarGenericTypeProxy__CustomNSObject_1____SomeMethod (sel, p0, exception_gchandle);
		}
	}
}

// regular non-generic class for comparison:
public class CustomNSObject : NSObject
	where T : NSObject
{
	[Export ("someMethod:")]
	public void SomeMethod (NSSet someInput)
	{
		// ...
	}
	
	private static class __Registrar_Callbacks__
	{
		[UnmanagedCallersOnly (EntryPoint = "_callback_1_CustomNSObject_1_SomeMethod")]
		public unsafe static void callback_1_CustomNSObject_1_SomeMethod (IntPtr pobj, IntPtr sel, IntPtr p0, IntPtr* exception_gchandle)
		{
			try {
				NSSet obj0 = Runtime.GetNSObject<NSSet> (p0);
				SomeMethod (obj0);
			} catch (Exception ex) {
				*exception_gchandle = Runtime.AllocGCHandle (ex);
			}
		}
	}
}

github-actions · 2023-06-08T12:40:37Z

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

rolfbjarne · 2023-06-09T05:41:47Z

/azp run

azure-pipelines · 2023-06-09T05:42:10Z

Azure Pipelines successfully started running 1 pipeline(s).

rolfbjarne

There's one failing test: it's failing because an exception isn't thrown when calling a method taking a generic parameter T instantiated with NSSet is passed NSObject:

xamarin-macios/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs

Line 1351 in e308887

    
           ThrowsICEIfDebug (() => Messaging.void_objc_msgSend_IntPtr (obj.Handle, Selector.GetHandle ("m1:"), nsobj.Handle), "m1: ICE");

tools/dotnet-linker/Steps/ManagedRegistrarStep.cs

rolfbjarne · 2023-06-13T12:37:20Z

/azp run

azure-pipelines · 2023-06-13T12:37:41Z

Azure Pipelines successfully started running 1 pipeline(s).

github-actions · 2023-06-13T12:38:54Z

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

rolfbjarne · 2023-06-13T12:59:03Z

/azp run

azure-pipelines · 2023-06-13T12:59:23Z

Azure Pipelines successfully started running 1 pipeline(s).

simonrozsival · 2023-06-20T20:33:43Z

@rolfbjarne I fixed the bug that caused the GenericClassWithUnrelatedGenericDelegate. Please re-run the tests one more time.

rolfbjarne · 2023-06-20T20:41:10Z

/azp run

azure-pipelines · 2023-06-20T20:41:30Z

Azure Pipelines successfully started running 1 pipeline(s).

dalexsoto · 2023-06-28T04:50:06Z

/azp run

azure-pipelines · 2023-06-28T04:50:18Z

Azure Pipelines successfully started running 1 pipeline(s).

vs-mobiletools-engineering-service2 · 2023-06-28T06:33:59Z

💻 [CI Build] Windows Integration Tests passed 💻

✅ All Windows Integration Tests passed.

Pipeline on Agent
Hash: 9fb4a7622dcdaa0e4aacf009c61b318b9dbbc67b [PR build]

vs-mobiletools-engineering-service2 · 2023-06-28T06:36:10Z

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2023-06-28T06:37:33Z

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2023-06-28T06:42:49Z

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13.0) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2023-06-28T06:49:53Z

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)

~~iOS~~ (no change detected)
~~tvOS~~ (no change detected)
~~watchOS~~ (no change detected)
~~macOS~~ (no change detected)

NET (empty diffs)

~~iOS~~: (empty diff detected)
~~tvOS~~: (empty diff detected)
~~MacCatalyst~~: (empty diff detected)
~~macOS~~: (empty diff detected)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
watchOS: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)
Microsoft.iOS vs Microsoft.MacCatalyst: vsdrops gist

Legacy Xamarin (stable) vs .NET

iOS: vsdrops gist
tvOS: vsdrops gist
macOS: vsdrops gist

✅ Generator diff

Generator diff is empty

Pipeline on Agent
Hash: 9fb4a7622dcdaa0e4aacf009c61b318b9dbbc67b [PR build]

vs-mobiletools-engineering-service2 · 2023-06-28T09:57:15Z

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 235 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 12 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 35 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 9fb4a7622dcdaa0e4aacf009c61b318b9dbbc67b [PR build]

rolfbjarne · 2023-06-28T10:25:11Z

Great work!

simonrozsival and others added 2 commits June 8, 2023 14:35

MSR: Remove reflection for generic types

cfa1467

Auto-format source code

03958dc

This comment has been minimized.

Sign in to view

rolfbjarne reviewed Jun 12, 2023

View reviewed changes

tools/dotnet-linker/Steps/ManagedRegistrarStep.cs Outdated Show resolved Hide resolved

tools/dotnet-linker/Steps/ManagedRegistrarStep.cs Outdated Show resolved Hide resolved

tools/dotnet-linker/Steps/ManagedRegistrarStep.cs Outdated Show resolved Hide resolved

Fix certain scenarios (ref, out, arrays)

818e967

Auto-format source code

14bf8b7

This comment has been minimized.

Sign in to view

simonrozsival added 2 commits June 14, 2023 19:20

Make the generated interface internal

97b78c5

Add a comment with a sample of generated code

9fe1bd3

This comment has been minimized.

Sign in to view

Fix passing parameters to the proxy method

47dd125

simonrozsival marked this pull request as ready for review June 20, 2023 20:33

This comment has been minimized.

Sign in to view

simonrozsival and others added 2 commits June 21, 2023 13:54

Merge branch 'main' into msr-improve-generic-type-calls

a3ba74b

Merge branch 'main' into msr-improve-generic-type-calls

9fb4a76

rolfbjarne approved these changes Jun 28, 2023

View reviewed changes

rolfbjarne merged commit 6010c13 into xamarin:main Jun 28, 2023
50 of 52 checks passed

filipnavara mentioned this pull request Jul 1, 2023

NativeAOT: Asking for Array<T> MethodTable dotnet/runtime#88287

Closed

filipnavara mentioned this pull request Aug 1, 2023

Use Type.GetMemberWithSameMetadataDefinitionAs to speed up finding closed generic members #18355

Open

MSR: Remove use of reflection in UCOs of generic types #18421

MSR: Remove use of reflection in UCOs of generic types #18421

Conversation

simonrozsival commented Jun 8, 2023 • edited Loading

github-actions bot commented Jun 8, 2023

⚠️ Your code has been reformatted. ⚠️

rolfbjarne commented Jun 9, 2023

azure-pipelines bot commented Jun 9, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rolfbjarne left a comment

Choose a reason for hiding this comment

rolfbjarne commented Jun 13, 2023

azure-pipelines bot commented Jun 13, 2023

github-actions bot commented Jun 13, 2023

⚠️ Your code has been reformatted. ⚠️

rolfbjarne commented Jun 13, 2023

azure-pipelines bot commented Jun 13, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

simonrozsival commented Jun 20, 2023

rolfbjarne commented Jun 20, 2023

azure-pipelines bot commented Jun 20, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

dalexsoto commented Jun 28, 2023

azure-pipelines bot commented Jun 28, 2023

vs-mobiletools-engineering-service2 commented Jun 28, 2023

💻 [CI Build] Windows Integration Tests passed 💻

vs-mobiletools-engineering-service2 commented Jun 28, 2023

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

vs-mobiletools-engineering-service2 commented Jun 28, 2023

📚 [PR Build] Artifacts 📚

Packages generated

vs-mobiletools-engineering-service2 commented Jun 28, 2023

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

vs-mobiletools-engineering-service2 commented Jun 28, 2023

✅ API diff for current PR / commit

✅ API diff vs stable

✅ Generator diff

vs-mobiletools-engineering-service2 commented Jun 28, 2023

🚀 [CI Build] Test results 🚀

Test results

Tests counts

rolfbjarne commented Jun 28, 2023

simonrozsival commented Jun 8, 2023 •

edited

Loading