Skip to content

Commit

Permalink
[mini] respect additional flags when caching runtime invoke wrappers
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lewurm authored and marek-safar committed Jul 9, 2018
1 parent 2115f27 commit 7c3a3b1
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 38 deletions.
6 changes: 2 additions & 4 deletions mono/metadata/image.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
113 changes: 83 additions & 30 deletions mono/metadata/marshal.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -2524,14 +2573,18 @@ 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;
static MonoMethodSignature *finalize_signature = NULL;
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);

Expand All @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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 */
Expand Down Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions mono/metadata/metadata-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/*
Expand All @@ -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;
Expand Down Expand Up @@ -377,7 +376,6 @@ struct _MonoImage {
/*
* indexed by MonoMethod pointers
*/
GHashTable *runtime_invoke_vcall_cache;
GHashTable *wrapper_param_names;
GHashTable *array_accessor_cache;

Expand Down

0 comments on commit 7c3a3b1

Please sign in to comment.