From 7c3a3b100edd9cad29e12904d9b35a0f843f1298 Mon Sep 17 00:00:00 2001 From: Bernhard Urban Date: Fri, 29 Jun 2018 20:04:13 +0200 Subject: [PATCH] [mini] respect additional flags when caching runtime invoke wrappers depending on `virtual_` and `need_direct_wrapper` we generate slightly different IL. The interpreter always set `need_direct_wrapper`, but the JIT doesn't, so this gets problemantic with mixed mode. We need to be careful when we cache those results. --- mono/metadata/image.c | 6 +- mono/metadata/marshal.c | 113 +++++++++++++++++++++-------- mono/metadata/metadata-internals.h | 6 +- 3 files changed, 87 insertions(+), 38 deletions(-) diff --git a/mono/metadata/image.c b/mono/metadata/image.c index ecebf452b63e..a9d8d1ed2651 100644 --- a/mono/metadata/image.c +++ b/mono/metadata/image.c @@ -1958,12 +1958,11 @@ mono_wrapper_caches_free (MonoWrapperCaches *cache) free_hash (cache->delegate_invoke_cache); free_hash (cache->delegate_begin_invoke_cache); free_hash (cache->delegate_end_invoke_cache); - free_hash (cache->runtime_invoke_cache); - free_hash (cache->runtime_invoke_vtype_cache); + free_hash (cache->runtime_invoke_signature_cache); free_hash (cache->delegate_abstract_invoke_cache); - free_hash (cache->runtime_invoke_direct_cache); + free_hash (cache->runtime_invoke_method_cache); free_hash (cache->managed_wrapper_cache); free_hash (cache->native_wrapper_cache); @@ -2133,7 +2132,6 @@ mono_image_close_except_pools (MonoImage *image) } free_hash (image->delegate_bound_static_invoke_cache); - free_hash (image->runtime_invoke_vcall_cache); free_hash (image->ldfld_wrapper_cache); free_hash (image->ldflda_wrapper_cache); free_hash (image->stfld_wrapper_cache); diff --git a/mono/metadata/marshal.c b/mono/metadata/marshal.c index d0684baac7fd..7c622cfac125 100644 --- a/mono/metadata/marshal.c +++ b/mono/metadata/marshal.c @@ -2506,6 +2506,55 @@ runtime_invoke_signature_equal (MonoMethodSignature *sig1, MonoMethodSignature * return mono_metadata_signature_equal (sig1, sig2); } +struct _MonoWrapperMethodCacheKey { + MonoMethod *method; + gboolean virtual_; + gboolean need_direct_wrapper; +}; + +struct _MonoWrapperSignatureCacheKey { + MonoMethodSignature *signature; + gboolean virtual_; + gboolean need_direct_wrapper; +}; + +typedef struct _MonoWrapperMethodCacheKey MonoWrapperMethodCacheKey; +typedef struct _MonoWrapperSignatureCacheKey MonoWrapperSignatureCacheKey; + +static guint +wrapper_cache_method_key_hash (MonoWrapperMethodCacheKey *key) +{ + return mono_aligned_addr_hash (key->method) ^ ((!!key->virtual_) << 18) ^ ((!!key->need_direct_wrapper) << 19); +} + +static guint +wrapper_cache_signature_key_hash (MonoWrapperSignatureCacheKey *key) +{ + return mono_signature_hash (key->signature) ^ ((!!key->virtual_) << 18) ^ ((!!key->need_direct_wrapper) << 19); +} + +static gboolean +wrapper_cache_method_key_equal (MonoWrapperMethodCacheKey *key1, MonoWrapperMethodCacheKey *key2) +{ + if (key1->virtual_ != key2->virtual_ || key1->need_direct_wrapper != key2->need_direct_wrapper) + return FALSE; + return key1->method == key2->method; +} + +static gboolean +wrapper_cache_signature_key_equal (MonoWrapperSignatureCacheKey *key1, MonoWrapperSignatureCacheKey *key2) +{ + if (key1->virtual_ != key2->virtual_ || key1->need_direct_wrapper != key2->need_direct_wrapper) + return FALSE; + return runtime_invoke_signature_equal (key1->signature, key2->signature); +} + +static gboolean +wrapper_cache_method_matches_data (gpointer key, gpointer value, gpointer user_data) +{ + MonoWrapperMethodCacheKey *mkey = (MonoWrapperMethodCacheKey *) key; + return mkey->method == (MonoMethod *) user_data; +} /** * mono_marshal_get_runtime_invoke: @@ -2524,7 +2573,8 @@ mono_marshal_get_runtime_invoke_full (MonoMethod *method, gboolean virtual_, gbo { MonoMethodSignature *sig, *csig, *callsig; MonoMethodBuilder *mb; - GHashTable *cache = NULL; + GHashTable *method_cache = NULL, *sig_cache; + GHashTable **cache_table = NULL; MonoClass *target_klass; MonoMethod *res = NULL; static MonoMethodSignature *cctor_signature = NULL; @@ -2532,6 +2582,9 @@ mono_marshal_get_runtime_invoke_full (MonoMethod *method, gboolean virtual_, gbo char *name; const char *param_names [16]; WrapperInfo *info; + MonoWrapperMethodCacheKey *method_key; + MonoWrapperMethodCacheKey method_key_lookup_only = { .method = method, .virtual_ = virtual_, .need_direct_wrapper = need_direct_wrapper }; + method_key = &method_key_lookup_only; g_assert (method); @@ -2545,16 +2598,10 @@ mono_marshal_get_runtime_invoke_full (MonoMethod *method, gboolean virtual_, gbo finalize_signature->hasthis = 1; } - /* - * Use a separate cache indexed by methods to speed things up and to avoid the - * boundless mempool growth caused by the signature_dup stuff below. - */ - if (virtual_) - cache = get_cache (&get_method_image (method)->runtime_invoke_vcall_cache, mono_aligned_addr_hash, NULL); - else - cache = get_cache (&mono_method_get_wrapper_cache (method)->runtime_invoke_direct_cache, mono_aligned_addr_hash, NULL); + cache_table = &mono_method_get_wrapper_cache (method)->runtime_invoke_method_cache; + method_cache = get_cache (cache_table, (GHashFunc) wrapper_cache_method_key_hash, (GCompareFunc) wrapper_cache_method_key_equal); - res = mono_marshal_find_in_cache (cache, method); + res = mono_marshal_find_in_cache (method_cache, method_key); if (res) return res; @@ -2588,19 +2635,14 @@ mono_marshal_get_runtime_invoke_full (MonoMethod *method, gboolean virtual_, gbo MonoMethodSignature *tmp_sig; callsig = mono_marshal_get_runtime_invoke_sig (callsig); - GHashTable **cache_table = NULL; + MonoWrapperSignatureCacheKey sig_key = { .signature = callsig, .virtual_ = virtual_, .need_direct_wrapper = need_direct_wrapper }; - if (m_class_is_valuetype (method->klass) && mono_method_signature (method)->hasthis) - cache_table = &mono_method_get_wrapper_cache (method)->runtime_invoke_vtype_cache; - else - cache_table = &mono_method_get_wrapper_cache (method)->runtime_invoke_cache; - - cache = get_cache (cache_table, (GHashFunc)mono_signature_hash, - (GCompareFunc)runtime_invoke_signature_equal); + cache_table = &mono_method_get_wrapper_cache (method)->runtime_invoke_signature_cache; + sig_cache = get_cache (cache_table, (GHashFunc) wrapper_cache_signature_key_hash, (GCompareFunc) wrapper_cache_signature_key_equal); /* from mono_marshal_find_in_cache */ mono_marshal_lock (); - res = (MonoMethod *)g_hash_table_lookup (cache, callsig); + res = (MonoMethod *)g_hash_table_lookup (sig_cache, &sig_key); mono_marshal_unlock (); if (res) { @@ -2644,15 +2686,25 @@ mono_marshal_get_runtime_invoke_full (MonoMethod *method, gboolean virtual_, gbo get_marshal_cb ()->emit_runtime_invoke_body (mb, param_names, m_class_get_image (target_klass), method, sig, callsig, virtual_, need_direct_wrapper); + method_key = g_new (MonoWrapperMethodCacheKey, 1); + method_key->method = method; + method_key->virtual_ = virtual_; + method_key->need_direct_wrapper = need_direct_wrapper; + if (need_direct_wrapper) { get_marshal_cb ()->mb_skip_visibility (mb); info = mono_wrapper_info_create (mb, virtual_ ? WRAPPER_SUBTYPE_RUNTIME_INVOKE_VIRTUAL : WRAPPER_SUBTYPE_RUNTIME_INVOKE_DIRECT); info->d.runtime_invoke.method = method; - res = mono_mb_create_and_cache_full (cache, method, mb, csig, sig->param_count + 16, info, NULL); + res = mono_mb_create_and_cache_full (method_cache, method_key, mb, csig, sig->param_count + 16, info, NULL); } else { + MonoWrapperSignatureCacheKey *sig_key = g_new0 (MonoWrapperSignatureCacheKey, 1); + sig_key->signature = callsig; + sig_key->virtual_ = virtual_; + sig_key->need_direct_wrapper = need_direct_wrapper; + /* taken from mono_mb_create_and_cache */ mono_marshal_lock (); - res = (MonoMethod *)g_hash_table_lookup (cache, callsig); + res = (MonoMethod *)g_hash_table_lookup (sig_cache, sig_key); mono_marshal_unlock (); info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_RUNTIME_INVOKE_NORMAL); @@ -2664,19 +2716,20 @@ mono_marshal_get_runtime_invoke_full (MonoMethod *method, gboolean virtual_, gbo newm = mono_mb_create (mb, csig, sig->param_count + 16, info); mono_marshal_lock (); - res = (MonoMethod *)g_hash_table_lookup (cache, callsig); + res = (MonoMethod *)g_hash_table_lookup (sig_cache, sig_key); if (!res) { - GHashTable *direct_cache; res = newm; - g_hash_table_insert (cache, callsig, res); - /* Can't insert it into wrapper_hash since the key is a signature */ - direct_cache = mono_method_get_wrapper_cache (method)->runtime_invoke_direct_cache; - - g_hash_table_insert (direct_cache, method, res); + g_hash_table_insert (sig_cache, sig_key, res); + g_hash_table_insert (method_cache, method_key, res); } else { mono_free_method (newm); + g_free (sig_key); + g_free (method_key); } mono_marshal_unlock (); + } else { + g_free (sig_key); + g_free (method_key); } /* end mono_mb_create_and_cache */ @@ -6189,8 +6242,8 @@ mono_marshal_free_dynamic_wrappers (MonoMethod *method) * FIXME: We currently leak the wrappers. Freeing them would be tricky as * they could be shared with other methods ? */ - if (image->wrapper_caches.runtime_invoke_direct_cache) - g_hash_table_remove (image->wrapper_caches.runtime_invoke_direct_cache, method); + if (image->wrapper_caches.runtime_invoke_method_cache) + g_hash_table_foreach_remove (image->wrapper_caches.runtime_invoke_method_cache, wrapper_cache_method_matches_data, method); if (image->wrapper_caches.delegate_abstract_invoke_cache) g_hash_table_foreach_remove (image->wrapper_caches.delegate_abstract_invoke_cache, signature_pointer_pair_matches_pointer, method); // FIXME: Need to clear the caches in other images as well diff --git a/mono/metadata/metadata-internals.h b/mono/metadata/metadata-internals.h index 433d632b5a09..325c9939122b 100644 --- a/mono/metadata/metadata-internals.h +++ b/mono/metadata/metadata-internals.h @@ -152,8 +152,7 @@ typedef struct { GHashTable *delegate_invoke_cache; GHashTable *delegate_begin_invoke_cache; GHashTable *delegate_end_invoke_cache; - GHashTable *runtime_invoke_cache; - GHashTable *runtime_invoke_vtype_cache; + GHashTable *runtime_invoke_signature_cache; GHashTable *runtime_invoke_sig_cache; /* @@ -165,7 +164,7 @@ typedef struct { * indexed by MonoMethod pointers * Protected by the marshal lock */ - GHashTable *runtime_invoke_direct_cache; + GHashTable *runtime_invoke_method_cache; GHashTable *managed_wrapper_cache; GHashTable *native_wrapper_cache; @@ -377,7 +376,6 @@ struct _MonoImage { /* * indexed by MonoMethod pointers */ - GHashTable *runtime_invoke_vcall_cache; GHashTable *wrapper_param_names; GHashTable *array_accessor_cache;