[mono] Futher cleanups to delegate invocation (#83705)

* [mono] Cache delegate->method_ptr in MonoDelegateTrampInfo instead of in delegate->method_code.

Repurpose the method_code field to store the MonoDelegateTrampInfo pointer
for future optimizations.

* Remove now unused MONO_PATCH_INFO_METHOD_CODE_SLOT.

* Remove unused mono_runtime_create_delegate_trampoline ().

* Reduce handle usage in mini_init_delegate ().

* Small cleanups to mono_delegate_trampoline ().

* Use the MonoDelegateTrampInfo from the delegate if possible.

Author: vargaz

src/mono/mono/metadata/class-internals.h

@@ -755,9 +755,6 @@ mono_lookup_dynamic_token (MonoImage *image, guint32 token, MonoGenericContext *
 gpointer
 mono_lookup_dynamic_token_class (MonoImage *image, guint32 token, gboolean check_token, MonoClass **handle_class, MonoGenericContext *context, MonoError *error);
 
-gpointer
-mono_runtime_create_delegate_trampoline (MonoClass *klass);
-
 void
 mono_install_get_cached_class_info (MonoGetCachedClassInfo func);
 

src/mono/mono/metadata/object-internals.h

@@ -687,7 +687,6 @@ typedef struct {
 	gpointer (*create_jit_trampoline) (MonoMethod *method, MonoError *error);
 	/* used to free a dynamic method */
 	void     (*free_method) (MonoMethod *method);
-	gpointer (*create_delegate_trampoline) (MonoClass *klass);
 	GHashTable *(*get_weak_field_indexes) (MonoImage *image);
 	gboolean (*is_interpreter_enabled) (void);
 	void (*init_mem_manager)(MonoMemoryManager*);
@@ -836,11 +835,8 @@ struct _MonoDelegate {
 	gpointer delegate_trampoline;
 	/* Extra argument passed to the target method in llvmonly mode */
 	gpointer extra_arg;
-	/*
-	 * If non-NULL, this points to a memory location which stores the address of
-	 * the compiled code of the method, or NULL if it is not yet compiled.
-	 */
-	guint8 **method_code;
+	/* MonoDelegateTrampInfo */
+	gpointer invoke_info;
 	gpointer interp_method;
 	/* Interp method that is executed when invoking the delegate */
 	gpointer interp_invoke_impl;

src/mono/mono/metadata/object-offsets.h

@@ -85,7 +85,7 @@ DECL_OFFSET(MonoDelegate, target)
 DECL_OFFSET(MonoDelegate, method_ptr)
 DECL_OFFSET(MonoDelegate, invoke_impl)
 DECL_OFFSET(MonoDelegate, method)
-DECL_OFFSET(MonoDelegate, method_code)
+DECL_OFFSET(MonoDelegate, invoke_info)
 DECL_OFFSET(MonoDelegate, method_is_virtual)
 DECL_OFFSET(MonoDelegate, bound)
 DECL_OFFSET(MonoDelegate, extra_arg)

src/mono/mono/metadata/object.c

@@ -775,15 +775,6 @@ mono_compile_method_checked (MonoMethod *method, MonoError *error)
 	return res;
 }
 
-gpointer
-mono_runtime_create_delegate_trampoline (MonoClass *klass)
-{
-	MONO_REQ_GC_NEUTRAL_MODE
-
-	g_assert (callbacks.create_delegate_trampoline);
-	return callbacks.create_delegate_trampoline (klass);
-}
-
 /**
  * mono_runtime_free_method:
  * \param method method to release

src/mono/mono/mini/aot-compiler.c

@@ -6967,7 +6967,6 @@ encode_patch (MonoAotCompile *acfg, MonoJumpInfo *patch_info, guint8 *buf, guint
 	case MONO_PATCH_INFO_ICALL_ADDR:
 	case MONO_PATCH_INFO_ICALL_ADDR_CALL:
 	case MONO_PATCH_INFO_METHOD_RGCTX:
-	case MONO_PATCH_INFO_METHOD_CODE_SLOT:
 	case MONO_PATCH_INFO_METHOD_PINVOKE_ADDR_CACHE:
 		encode_method_ref (acfg, patch_info->data.method, p, &p);
 		break;
@@ -8936,7 +8935,6 @@ can_encode_patch (MonoAotCompile *acfg, MonoJumpInfo *patch_info)
 	case MONO_PATCH_INFO_METHOD:
 	case MONO_PATCH_INFO_METHOD_FTNDESC:
 	case MONO_PATCH_INFO_METHODCONST:
-	case MONO_PATCH_INFO_METHOD_CODE_SLOT:
 	case MONO_PATCH_INFO_METHOD_PINVOKE_ADDR_CACHE:
 	case MONO_PATCH_INFO_LLVMONLY_INTERP_ENTRY: {
 		MonoMethod *method = patch_info->data.method;

src/mono/mono/mini/aot-runtime.c

@@ -3778,7 +3778,6 @@ decode_patch (MonoAotModule *aot_module, MonoMemPool *mp, MonoJumpInfo *ji, guin
 	case MONO_PATCH_INFO_ICALL_ADDR:
 	case MONO_PATCH_INFO_ICALL_ADDR_CALL:
 	case MONO_PATCH_INFO_METHOD_RGCTX:
-	case MONO_PATCH_INFO_METHOD_CODE_SLOT:
 	case MONO_PATCH_INFO_METHOD_PINVOKE_ADDR_CACHE:
 	case MONO_PATCH_INFO_LLVMONLY_INTERP_ENTRY: {
 		MethodRef ref;

src/mono/mono/mini/method-to-ir.c

@@ -3692,6 +3692,9 @@ handle_delegate_ctor (MonoCompile *cfg, MonoClass *klass, MonoInst *target, Mono
 		return obj;
 	}
 
+	/* Set invoke_info field */
+	MONO_EMIT_NEW_STORE_MEMBASE (cfg, OP_STORE_MEMBASE_REG, obj->dreg, MONO_STRUCT_OFFSET (MonoDelegate, invoke_info), info_ins->dreg);
+
 	/* Set method field */
 	if (target_method_context_used || invoke_context_used) {
 		// We copy from the delegate trampoline info as it's faster than a rgctx fetch
@@ -3704,21 +3707,6 @@ handle_delegate_ctor (MonoCompile *cfg, MonoClass *klass, MonoInst *target, Mono
 		MONO_EMIT_NEW_STORE_MEMBASE (cfg, OP_STORE_MEMBASE_REG, obj->dreg, MONO_STRUCT_OFFSET (MonoDelegate, method), method_ins->dreg);
 	}
 
-	/*
-	 * To avoid looking up the compiled code belonging to the target method
-	 * in mono_delegate_trampoline (), we allocate a per-domain memory slot to
-	 * store it, and we fill it after the method has been compiled.
-	 */
-	if (!method->dynamic) {
-		MonoInst *code_slot_ins;
-
-		if (target_method_context_used)
-			code_slot_ins = emit_get_rgctx_method (cfg, target_method_context_used, method, MONO_RGCTX_INFO_METHOD_DELEGATE_CODE);
-		else
-			code_slot_ins = mini_emit_runtime_constant (cfg, MONO_PATCH_INFO_METHOD_CODE_SLOT, method);
-		MONO_EMIT_NEW_STORE_MEMBASE (cfg, OP_STORE_MEMBASE_REG, obj->dreg, MONO_STRUCT_OFFSET (MonoDelegate, method_code), code_slot_ins->dreg);
-	}
-
 	/* Set invoke_impl field */
 	dreg = alloc_preg (cfg);
 	MONO_EMIT_NEW_LOAD_MEMBASE (cfg, dreg, info_ins->dreg, MONO_STRUCT_OFFSET (MonoDelegateTrampInfo, invoke_impl));

src/mono/mono/mini/mini-runtime.c

@@ -1235,7 +1235,6 @@ mono_patch_info_hash (gconstpointer data)
 	case MONO_PATCH_INFO_SEQ_POINT_INFO:
 	case MONO_PATCH_INFO_METHOD_RGCTX:
 	case MONO_PATCH_INFO_SIGNATURE:
-	case MONO_PATCH_INFO_METHOD_CODE_SLOT:
 	case MONO_PATCH_INFO_AOT_JIT_INFO:
 	case MONO_PATCH_INFO_METHOD_PINVOKE_ADDR_CACHE:
 	case MONO_PATCH_INFO_GSHARED_METHOD_INFO:
@@ -1493,22 +1492,6 @@ mono_resolve_patch_target_ext (MonoMemoryManager *mem_manager, MonoMethod *metho
 		mono_error_assert_ok (error);
 		break;
 	}
-	case MONO_PATCH_INFO_METHOD_CODE_SLOT: {
-		gpointer code_slot;
-
-		MonoJitMemoryManager *jit_mm = jit_mm_for_method (patch_info->data.method);
-		jit_mm_lock (jit_mm);
-		if (!jit_mm->method_code_hash)
-			jit_mm->method_code_hash = g_hash_table_new (NULL, NULL);
-		code_slot = g_hash_table_lookup (jit_mm->method_code_hash, patch_info->data.method);
-		if (!code_slot) {
-			code_slot = mono_mem_manager_alloc0 (jit_mm->mem_manager, sizeof (gpointer));
-			g_hash_table_insert (jit_mm->method_code_hash, patch_info->data.method, code_slot);
-		}
-		jit_mm_unlock (jit_mm);
-		target = code_slot;
-		break;
-	}
 	case MONO_PATCH_INFO_METHOD_PINVOKE_ADDR_CACHE: {
 		target = mono_mem_manager_alloc0 (mem_manager, sizeof (gpointer));
 		break;
@@ -3900,7 +3883,7 @@ mini_init_delegate (MonoDelegateHandle delegate, MonoObjectHandle target, gpoint
 	if (!method && !addr) {
 		// Multicast delegate init
 		if (!mono_llvm_only) {
-			MONO_HANDLE_SETVAL (delegate, invoke_impl, gpointer, mono_create_delegate_trampoline (mono_handle_class (delegate)));
+			del->invoke_impl = mono_create_delegate_trampoline (mono_handle_class (delegate));
 		} else {
 			mini_llvmonly_init_delegate (del, NULL);
 		}
@@ -3925,13 +3908,11 @@ mini_init_delegate (MonoDelegateHandle delegate, MonoObjectHandle target, gpoint
 	}
 
 	if (method)
-		MONO_HANDLE_SETVAL (delegate, method, MonoMethod*, method);
-
+		del->method = method;
 	if (addr)
-		MONO_HANDLE_SETVAL (delegate, method_ptr, gpointer, addr);
-
-	MONO_HANDLE_SET (delegate, target, target);
-	MONO_HANDLE_SETVAL (delegate, invoke_impl, gpointer, mono_create_delegate_trampoline (mono_handle_class (delegate)));
+		del->method_ptr = addr;
+	MONO_OBJECT_SETREF_INTERNAL (del, target, MONO_HANDLE_RAW (target));
+	del->invoke_impl = mono_create_delegate_trampoline (mono_handle_class (delegate));
 
 	MonoDelegateTrampInfo *info = NULL;
 
@@ -4302,7 +4283,6 @@ free_jit_mem_manager (MonoMemoryManager *mem_manager)
 		g_hash_table_foreach (info->dynamic_code_hash, dynamic_method_info_free, NULL);
 		g_hash_table_destroy (info->dynamic_code_hash);
 	}
-	g_hash_table_destroy (info->method_code_hash);
 	g_hash_table_destroy (info->jump_trampoline_hash);
 	g_hash_table_destroy (info->jit_trampoline_hash);
 	g_hash_table_destroy (info->delegate_info_hash);
@@ -4562,7 +4542,6 @@ mini_init (const char *filename)
 #ifdef JIT_TRAMPOLINES_WORK
 	callbacks.compile_method = mono_jit_compile_method;
 	callbacks.create_jit_trampoline = mono_create_jit_trampoline;
-	callbacks.create_delegate_trampoline = mono_create_delegate_trampoline;
 	callbacks.free_method = mono_jit_free_method;
 	callbacks.get_ftnptr = get_ftnptr_for_method;
 #endif

src/mono/mono/mini/mini-runtime.h

@@ -32,7 +32,6 @@ typedef struct {
 	GHashTable *static_rgctx_trampoline_hash;
 	/* maps MonoMethod -> MonoJitDynamicMethodInfo */
 	GHashTable *dynamic_code_hash;
-	GHashTable *method_code_hash;
 	/* Maps methods to a RuntimeInvokeInfo structure, protected by the associated MonoDomain lock */
 	MonoConcurrentHashTable *runtime_invoke_hash;
 	/* Maps MonoMethod to a GPtrArray containing sequence point locations */

src/mono/mono/mini/mini-trampolines.c

@@ -969,7 +969,8 @@ mono_delegate_trampoline (host_mgreg_t *regs, guint8 *code, gpointer *arg, guint
 	MonoMethod *m;
 	MonoMethod *method = NULL;
 	ERROR_DECL (error);
-	gboolean multicast, callvirt = FALSE, closed_over_null = FALSE;
+	gboolean multicast, callvirt = FALSE;
+	gboolean closed_static = FALSE, closed_over_null = FALSE;
 	gboolean need_rgctx_tramp = FALSE;
 	gboolean need_unbox_tramp = FALSE;
 	gboolean enable_caching = TRUE;
@@ -978,14 +979,23 @@ mono_delegate_trampoline (host_mgreg_t *regs, guint8 *code, gpointer *arg, guint
 	guint8 *impl_this = (guint8 *)tramp_info->impl_this;
 	guint8 *impl_nothis = (guint8 *)tramp_info->impl_nothis;
 	MonoMethodSignature *sig;
-	gpointer addr, compiled_method;
+	gpointer addr, invoke_impl, compiled_method;
 
 	UnlockedIncrement (&trampoline_calls);
 
 	/* Obtain the delegate object according to the calling convention */
 	delegate = (MonoDelegate *)mono_arch_get_this_arg_from_call (regs, code);
 	g_assert (mono_class_has_parent (mono_object_class (delegate), mono_defaults.multicastdelegate_class));
 
+	/*
+	 * The 'arg' argument might point to a MonoDelegateTrampInfo which
+	 * has no method set, if the trampoline was created by
+	 * mono_create_delegate_trampoline (). Use the more precise info
+	 * from the delegate itself.
+	 */
+	if (delegate->invoke_info)
+		tramp_info = (MonoDelegateTrampInfo*)delegate->invoke_info;
+
 	if (delegate->method) {
 		method = delegate->method;
 
@@ -1040,6 +1050,8 @@ mono_delegate_trampoline (host_mgreg_t *regs, guint8 *code, gpointer *arg, guint
 		callvirt = !delegate->target && sig->hasthis;
 		if (callvirt)
 			closed_over_null = tramp_info->invoke_sig->param_count == sig->param_count;
+		if (m_method_is_static (method) && sig->param_count == tramp_info->invoke_sig->param_count + 1)
+			closed_static = TRUE;
 
 		if (callvirt && !closed_over_null) {
 			/*
@@ -1051,7 +1063,7 @@ mono_delegate_trampoline (host_mgreg_t *regs, guint8 *code, gpointer *arg, guint
 			 * If the call doesn't return a valuetype, then the vcall uses the same calling
 			 * convention as a normal call.
 			 */
-			if ((mono_class_is_sealed (method->klass) || !(method->flags & METHOD_ATTRIBUTE_VIRTUAL)) && !MONO_TYPE_ISSTRUCT (sig->ret)) {
+			if ((mono_class_is_sealed (method->klass) || !m_method_is_virtual (method)) && !MONO_TYPE_ISSTRUCT (sig->ret)) {
 				callvirt = FALSE;
 				enable_caching = FALSE;
 			}
@@ -1066,7 +1078,7 @@ mono_delegate_trampoline (host_mgreg_t *regs, guint8 *code, gpointer *arg, guint
 			enable_caching = FALSE;
 		}
 
-		if (method->iflags & METHOD_IMPL_ATTRIBUTE_SYNCHRONIZED)
+		if (m_method_is_synchronized (method))
 			method = mono_marshal_get_synchronized_wrapper (method);
 
 		if (method == tramp_info->method)
@@ -1081,8 +1093,8 @@ mono_delegate_trampoline (host_mgreg_t *regs, guint8 *code, gpointer *arg, guint
 	 */
 	if (method && !callvirt) {
 		/* Avoid the overhead of looking up an already compiled method if possible */
-		if (enable_caching && delegate->method_code && *delegate->method_code) {
-			delegate->method_ptr = *delegate->method_code;
+		if (enable_caching && method == tramp_info->method && tramp_info->method_ptr) {
+			delegate->method_ptr = tramp_info->method_ptr;
 		} else {
 			compiled_method = addr = mono_jit_compile_method (method, error);
 			if (!is_ok (error)) {
@@ -1091,46 +1103,42 @@ mono_delegate_trampoline (host_mgreg_t *regs, guint8 *code, gpointer *arg, guint
 			}
 			addr = mini_add_method_trampoline (method, compiled_method, need_rgctx_tramp, need_unbox_tramp);
 			delegate->method_ptr = addr;
-			if (enable_caching && delegate->method_code)
-				*delegate->method_code = (guint8 *)delegate->method_ptr;
 		}
 	} else {
 		if (need_rgctx_tramp)
 			delegate->method_ptr = mono_create_static_rgctx_trampoline (method, delegate->method_ptr);
 	}
 
-	/* Necessary for !code condition to fallback to slow path */
-	code = NULL;
+	invoke_impl = NULL;
 
 	multicast = ((MonoMulticastDelegate*)delegate)->delegates != NULL;
 	if (!multicast && !callvirt) {
-		if (method && (method->flags & METHOD_ATTRIBUTE_STATIC) && mono_method_signature_internal (method)->param_count == mono_method_signature_internal (invoke)->param_count + 1)
-			/* Closed static delegate */
-			code = impl_this;
+		if (closed_static)
+			invoke_impl = impl_this;
 		else
-			code = delegate->target ? impl_this : impl_nothis;
+			invoke_impl = delegate->target ? impl_this : impl_nothis;
 	}
 
-	if (!code) {
+	if (!invoke_impl) {
 		/* The general, unoptimized case */
 		m = mono_marshal_get_delegate_invoke (invoke, delegate);
-		code = (guint8 *)mono_jit_compile_method (m, error);
+		invoke_impl = (guint8 *)mono_jit_compile_method (m, error);
 		if (!is_ok (error)) {
 			mono_error_set_pending_exception (error);
 			return NULL;
 		}
-		code = (guint8 *)mini_add_method_trampoline (m, code, mono_method_needs_static_rgctx_invoke (m, FALSE), FALSE);
+		invoke_impl = (guint8 *)mini_add_method_trampoline (m, invoke_impl, mono_method_needs_static_rgctx_invoke (m, FALSE), FALSE);
 	}
 
 	mono_memory_barrier ();
 
-	delegate->invoke_impl = mono_get_addr_from_ftnptr (code);
+	delegate->invoke_impl = mono_get_addr_from_ftnptr (invoke_impl);
 	if (enable_caching && !callvirt && tramp_info->method) {
 		tramp_info->method_ptr = delegate->method_ptr;
 		tramp_info->invoke_impl = delegate->invoke_impl;
 	}
 
-	return code;
+	return invoke_impl;
 }
 
 /*

src/mono/mono/mini/patch-info.h

@@ -44,7 +44,6 @@ PATCH_INFO(SIGNATURE, "signature")
 PATCH_INFO(GSHAREDVT_CALL, "gsharedvt_call")
 PATCH_INFO(GSHAREDVT_METHOD, "gsharedvt_method")
 PATCH_INFO(OBJC_SELECTOR_REF, "objc_selector_ref")
-PATCH_INFO(METHOD_CODE_SLOT, "method_code_slot")
 PATCH_INFO(LDSTR_LIT, "ldstr_lit")
 PATCH_INFO(GC_NURSERY_START, "gc_nursery_start")
 PATCH_INFO(VIRT_METHOD, "virt_method")