Skip to content

Commit

Permalink
[mono] Migrate more ghashtables to simdhash (#102476)
Browse files Browse the repository at this point in the history
Remove multi-domain support from mono_de_set_breakpoint
Migrate various ghashtables to simdhash
  • Loading branch information
kg authored Jun 8, 2024
1 parent 4cb0c50 commit 2da65a9
Show file tree
Hide file tree
Showing 14 changed files with 106 additions and 70 deletions.
49 changes: 27 additions & 22 deletions src/mono/mono/component/debugger-engine.c
Original file line number Diff line number Diff line change
Expand Up @@ -362,31 +362,39 @@ set_bp_in_method (MonoDomain *domain, MonoMethod *method, MonoSeqPointInfo *seq_
typedef struct {
MonoBreakpoint *bp;
GPtrArray *methods;
GPtrArray *method_domains;
GPtrArray *method_seq_points;

MonoDomain *domain;
} CollectDomainData;

static void
collect_domain_bp (gpointer key, gpointer value, gpointer user_data)
collect_domain_bp_inner (gpointer key, gpointer value, gpointer user_data)
{
GHashTableIter iter;
MonoSeqPointInfo *seq_points;
MonoDomain *domain = (MonoDomain*)key;
MonoMethod *m = (MonoMethod *)key;
MonoSeqPointInfo *seq_points = (MonoSeqPointInfo *)value;
CollectDomainData *ud = (CollectDomainData*)user_data;
MonoMethod *m;

if (!bp_matches_method (ud->bp, m))
return;

/* Save the info locally to simplify the code inside the domain lock */
g_ptr_array_add (ud->methods, m);
g_ptr_array_add (ud->method_seq_points, seq_points);
}

// HACK: Address some sort of linker problem on arm64
void
mono_jit_memory_manager_foreach_seq_point (dn_simdhash_ght_t *seq_points, dn_simdhash_ght_foreach_func func, gpointer user_data);

static void
collect_domain_bp (CollectDomainData *ud)
{
// FIXME:
MonoJitMemoryManager *jit_mm = get_default_jit_mm ();
jit_mm_lock (jit_mm);
g_hash_table_iter_init (&iter, jit_mm->seq_points);
while (g_hash_table_iter_next (&iter, (void**)&m, (void**)&seq_points)) {
if (bp_matches_method (ud->bp, m)) {
/* Save the info locally to simplify the code inside the domain lock */
g_ptr_array_add (ud->methods, m);
g_ptr_array_add (ud->method_domains, domain);
g_ptr_array_add (ud->method_seq_points, seq_points);
}
}
// FIXME: This previously used an iterator instead of foreach, so introducing
// an iterator API to simdhash might make it faster.
mono_jit_memory_manager_foreach_seq_point (jit_mm->seq_points, collect_domain_bp_inner, ud);
jit_mm_unlock (jit_mm);
}

Expand All @@ -408,7 +416,6 @@ mono_de_set_breakpoint (MonoMethod *method, long il_offset, EventRequest *req, M
MonoMethod *m;
MonoSeqPointInfo *seq_points;
GPtrArray *methods;
GPtrArray *method_domains;
GPtrArray *method_seq_points;

if (error)
Expand All @@ -429,7 +436,6 @@ mono_de_set_breakpoint (MonoMethod *method, long il_offset, EventRequest *req, M
PRINT_DEBUG_MSG (1, "[dbg] Setting %sbreakpoint at %s:0x%x.\n", (req->event_kind == EVENT_KIND_STEP) ? "single step " : "", method ? mono_method_full_name (method, TRUE) : "<all>", (int)il_offset);

methods = g_ptr_array_new ();
method_domains = g_ptr_array_new ();
method_seq_points = g_ptr_array_new ();

mono_loader_lock ();
Expand All @@ -438,20 +444,20 @@ mono_de_set_breakpoint (MonoMethod *method, long il_offset, EventRequest *req, M
memset (&user_data, 0, sizeof (user_data));
user_data.bp = bp;
user_data.methods = methods;
user_data.method_domains = method_domains;
user_data.method_seq_points = method_seq_points;
mono_de_foreach_domain (collect_domain_bp, &user_data);
user_data.domain = mono_get_root_domain ();
collect_domain_bp (&user_data);

for (guint i = 0; i < methods->len; ++i) {
m = (MonoMethod *)g_ptr_array_index (methods, i);
domain = (MonoDomain *)g_ptr_array_index (method_domains, i);
domain = user_data.domain;
seq_points = (MonoSeqPointInfo *)g_ptr_array_index (method_seq_points, i);
set_bp_in_method (domain, m, seq_points, bp, error);
}

// trying to get the seqpoints directly from the jit info of the method
// the seqpoints in get_default_jit_mm may not be found for AOTed methods in arm64
if (methods->len == 0)
if (methods->len == 0)
{
MonoJitInfo *ji;
(void)mono_jit_search_all_backends_for_jit_info (method, &ji);
Expand All @@ -464,7 +470,6 @@ mono_de_set_breakpoint (MonoMethod *method, long il_offset, EventRequest *req, M
mono_loader_unlock ();

g_ptr_array_free (methods, TRUE);
g_ptr_array_free (method_domains, TRUE);
g_ptr_array_free (method_seq_points, TRUE);

if (error && !is_ok (error)) {
Expand Down
2 changes: 2 additions & 0 deletions src/mono/mono/metadata/class.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
#include <mono/utils/unlocked.h>
#include <mono/utils/bsearch.h>
#include <mono/utils/checked-build.h>
// for dn_simdhash_ght_t
#include "../native/containers/dn-simdhash-specializations.h"

MonoStats mono_stats;

Expand Down
3 changes: 1 addition & 2 deletions src/mono/mono/metadata/loader-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,7 @@ struct _MonoMemoryManager {
MonoAssemblyLoadContext **alcs;

// Generic-specific caches
GHashTable *gsignature_cache;
dn_simdhash_ght_t *ginst_cache, *gmethod_cache;
dn_simdhash_ght_t *ginst_cache, *gmethod_cache, *gsignature_cache;
MonoConcurrentHashTable *gclass_cache;

/* mirror caches of ones already on MonoImage. These ones contain generics */
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/memory-manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ memory_manager_delete (MonoMemoryManager *memory_manager, gboolean debug_unload)
mono_conc_hashtable_destroy (mm->gclass_cache);
free_simdhash (&mm->ginst_cache);
free_simdhash (&mm->gmethod_cache);
free_hash (&mm->gsignature_cache);
free_simdhash (&mm->gsignature_cache);
free_hash (&mm->szarray_cache);
free_hash (&mm->array_cache);
free_hash (&mm->ptr_cache);
Expand Down
17 changes: 10 additions & 7 deletions src/mono/mono/metadata/metadata.c
Original file line number Diff line number Diff line change
Expand Up @@ -2452,7 +2452,7 @@ mono_metadata_signature_dup_internal (MonoImage *image, MonoMemPool *mp, MonoMem

/**
* Allocates memory for a MonoMethodSignature based on the provided parameters.
*
*
* @param image MonoImage for allocation.
* @param mp MonoMemPool for allocation.
* @param mem_manager MonoMemoryManager for allocation.
Expand Down Expand Up @@ -2568,10 +2568,10 @@ mono_metadata_signature_dup_delegate_invoke_to_target (MonoMethodSignature *sig)
* @param sig The original method signature.
* @param num_params The number parameters in the new signature.
* @param new_params An array of MonoType pointers representing the new parameters.
*
*
* Duplicate an existing \c MonoMethodSignature but with a new set of parameters.
* This is a Mono runtime internal function.
*
*
* @return the new \c MonoMethodSignature structure.
*/
MonoMethodSignature*
Expand Down Expand Up @@ -3339,7 +3339,7 @@ MonoMethodSignature *
mono_metadata_get_inflated_signature (MonoMethodSignature *sig, MonoGenericContext *context)
{
MonoInflatedMethodSignature helper;
MonoInflatedMethodSignature *res;
MonoInflatedMethodSignature *res = NULL;
CollectData data;

helper.sig = sig;
Expand All @@ -3354,16 +3354,19 @@ mono_metadata_get_inflated_signature (MonoMethodSignature *sig, MonoGenericConte
mono_mem_manager_lock (mm);

if (!mm->gsignature_cache)
mm->gsignature_cache = g_hash_table_new_full (inflated_signature_hash, inflated_signature_equal, NULL, (GDestroyNotify)free_inflated_signature);
// FIXME: Pick a better pre-reserved size
mm->gsignature_cache = dn_simdhash_ght_new_full (inflated_signature_hash, inflated_signature_equal, NULL, (GDestroyNotify)free_inflated_signature, 256, NULL);

// FIXME: The lookup is done on the newly allocated sig so it always fails
res = (MonoInflatedMethodSignature *)g_hash_table_lookup (mm->gsignature_cache, &helper);
dn_simdhash_ght_try_get_value (mm->gsignature_cache, &helper, (gpointer *)&res);
if (!res) {
res = mono_mem_manager_alloc0 (mm, sizeof (MonoInflatedMethodSignature));
// FIXME: sig is an inflated signature not owned by the mem manager
res->sig = sig;
res->context.class_inst = context->class_inst;
res->context.method_inst = context->method_inst;
g_hash_table_insert (mm->gsignature_cache, res, res);
// FIXME: We're wasting memory and cpu by storing key and value redundantly for this table
dn_simdhash_ght_insert (mm->gsignature_cache, res, res);
}

mono_mem_manager_unlock (mm);
Expand Down
4 changes: 2 additions & 2 deletions src/mono/mono/mini/aot-runtime.c
Original file line number Diff line number Diff line change
Expand Up @@ -3417,8 +3417,8 @@ decode_exception_debug_info (MonoAotModule *amodule,
jit_mm_lock (jit_mm);
/* This could be set already since this function can be called more than once for the same method */
MonoSeqPointInfo *existing_seq_points = NULL;
if (!g_hash_table_lookup_extended (jit_mm->seq_points, method, NULL, (gpointer *)&existing_seq_points)) {
g_hash_table_insert (jit_mm->seq_points, method, seq_points);
if (!dn_simdhash_ght_try_get_value (jit_mm->seq_points, method, (void **)&existing_seq_points)) {
dn_simdhash_ght_insert (jit_mm->seq_points, method, seq_points);
} else {
mono_seq_point_info_free (seq_points);
seq_points = existing_seq_points;
Expand Down
25 changes: 16 additions & 9 deletions src/mono/mono/mini/interp/tiering.c
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
#include "tiering.h"

static mono_mutex_t tiering_mutex;
static GHashTable *patch_sites_table;
// FIXME: The add/remove traffic on this table may require dn_simdhash to implement cascade flag cleanup
// and compaction
static dn_simdhash_ptr_ptr_t *patch_sites_table;
static gboolean enable_tiering;

void
mono_interp_tiering_init (void)
{
mono_os_mutex_init_recursive (&tiering_mutex);
patch_sites_table = g_hash_table_new (NULL, NULL);
patch_sites_table = dn_simdhash_ptr_ptr_new (0, NULL);
enable_tiering = TRUE;
}

Expand Down Expand Up @@ -61,11 +63,12 @@ patch_imethod_site (gpointer data, gpointer user_data)
static void
patch_interp_data_items (InterpMethod *old_imethod, InterpMethod *new_imethod)
{
GSList *sites = g_hash_table_lookup (patch_sites_table, old_imethod);
g_slist_foreach (sites, patch_imethod_site, new_imethod);

g_hash_table_remove (patch_sites_table, old_imethod);
g_slist_free (sites);
GSList *sites = NULL;
if (dn_simdhash_ptr_ptr_try_get_value (patch_sites_table, old_imethod, (void **)&sites)) {
g_slist_foreach (sites, patch_imethod_site, new_imethod);
dn_simdhash_ptr_ptr_try_remove (patch_sites_table, old_imethod);
g_slist_free (sites);
}
}

static InterpMethod*
Expand Down Expand Up @@ -100,9 +103,13 @@ static void
register_imethod_patch_site (InterpMethod *imethod, gpointer *ptr)
{
g_assert (!imethod->optimized);
GSList *sites = g_hash_table_lookup (patch_sites_table, imethod);
GSList *sites = NULL;
guint8 found = dn_simdhash_ptr_ptr_try_get_value (patch_sites_table, imethod, (void **)&sites);
sites = g_slist_prepend (sites, ptr);
g_hash_table_insert_replace (patch_sites_table, imethod, sites, TRUE);
if (found)
dn_simdhash_ptr_ptr_try_replace_value (patch_sites_table, imethod, sites);
else
dn_simdhash_ptr_ptr_try_add (patch_sites_table, imethod, sites);
}

static void
Expand Down
31 changes: 19 additions & 12 deletions src/mono/mono/mini/interp/transform.c
Original file line number Diff line number Diff line change
Expand Up @@ -1150,7 +1150,8 @@ init_last_ins_call (TransformData *td)
static guint32
get_data_item_wide_index (TransformData *td, void *ptr, gboolean *new_slot)
{
gpointer p = g_hash_table_lookup (td->data_hash, ptr);
gpointer p = NULL;
guint8 found = dn_simdhash_ptr_ptr_try_get_value (td->data_hash, ptr, (void **)&p);
guint32 index;
if (p != NULL) {
if (new_slot)
Expand All @@ -1164,7 +1165,10 @@ get_data_item_wide_index (TransformData *td, void *ptr, gboolean *new_slot)
index = td->n_data_items;
td->data_items [index] = ptr;
++td->n_data_items;
g_hash_table_insert (td->data_hash, ptr, GUINT_TO_POINTER (index + 1));
if (found)
dn_simdhash_ptr_ptr_try_replace_value (td->data_hash, ptr, GUINT_TO_POINTER (index + 1));
else
dn_simdhash_ptr_ptr_try_add (td->data_hash, ptr, GUINT_TO_POINTER (index + 1));
if (new_slot)
*new_slot = TRUE;
return index;
Expand Down Expand Up @@ -3002,7 +3006,7 @@ interp_inline_method (TransformData *td, MonoMethod *target_method, MonoMethodHe

/* Remove any newly added items */
for (i = prev_n_data_items; i < td->n_data_items; i++) {
g_hash_table_remove (td->data_hash, td->data_items [i]);
dn_simdhash_ptr_ptr_try_remove (td->data_hash, td->data_items [i]);
}
td->n_data_items = prev_n_data_items;
/* Also remove any added indexes from the imethod list */
Expand Down Expand Up @@ -3973,7 +3977,8 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
if (MINT_IS_PATCHABLE_CALL (td->last_ins->opcode)) {
g_assert (!calli && !is_virtual);
td->last_ins->flags |= INTERP_INST_FLAG_RECORD_CALL_PATCH;
g_hash_table_insert (td->patchsite_hash, td->last_ins, target_method);
if (!dn_simdhash_ptr_ptr_try_add (td->patchsite_hash, td->last_ins, target_method))
dn_simdhash_ptr_ptr_try_replace (td->patchsite_hash, td->last_ins, target_method);
}
#endif
}
Expand Down Expand Up @@ -8992,9 +8997,10 @@ emit_compacted_instruction (TransformData *td, guint16* start_ip, InterpInst *in
g_assert (MINT_IS_PATCHABLE_CALL (opcode));

/* TODO: could `ins` be removed by any interp optimization? */
MonoMethod *target_method = (MonoMethod *) g_hash_table_lookup (td->patchsite_hash, ins);
MonoMethod *target_method = NULL;
dn_simdhash_ptr_ptr_try_get_value (td->patchsite_hash, ins, (void **)&target_method);
g_assert (target_method);
g_hash_table_remove (td->patchsite_hash, ins);
dn_simdhash_ptr_ptr_try_remove (td->patchsite_hash, ins);

mini_tiered_record_callsite (start_ip, target_method, TIERED_PATCH_KIND_INTERP);

Expand Down Expand Up @@ -9284,9 +9290,9 @@ generate (MonoMethod *method, MonoMethodHeader *header, InterpMethod *rtm, MonoG
td->dummy_var = -1;
td->ref_handle_var = -1;
td->data_items = NULL;
td->data_hash = g_hash_table_new (NULL, NULL);
td->data_hash = dn_simdhash_ptr_ptr_new (0, NULL);
#ifdef ENABLE_EXPERIMENT_TIERED
td->patchsite_hash = g_hash_table_new (NULL, NULL);
td->patchsite_hash = dn_simdhash_ptr_ptr_new (0, NULL);
#endif
td->gen_seq_points = !mini_debug_options.no_seq_points_compact_data || mini_debug_options.gen_sdb_seq_points;
td->gen_sdb_seq_points = mini_debug_options.gen_sdb_seq_points;
Expand Down Expand Up @@ -9507,9 +9513,9 @@ generate (MonoMethod *method, MonoMethodHeader *header, InterpMethod *rtm, MonoG
g_free (td->renamable_vars);
g_free (td->renamed_fixed_vars);
g_free (td->local_ref_count);
g_hash_table_destroy (td->data_hash);
dn_simdhash_free (td->data_hash);
#ifdef ENABLE_EXPERIMENT_TIERED
g_hash_table_destroy (td->patchsite_hash);
dn_simdhash_free (td->patchsite_hash);
#endif
g_ptr_array_free (td->seq_points, TRUE);
if (td->line_numbers)
Expand Down Expand Up @@ -9690,9 +9696,10 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon

// FIXME Publishing of seq points seems to be racy with tiereing. We can have both tiered and untiered method
// running at the same time. We could therefore get the optimized imethod seq points for the unoptimized method.
gpointer seq_points = g_hash_table_lookup (jit_mm->seq_points, imethod->method);
gpointer seq_points = NULL;
dn_simdhash_ght_try_get_value (jit_mm->seq_points, imethod->method, (void **)&seq_points);
if (!seq_points || seq_points != imethod->jinfo->seq_points)
g_hash_table_replace (jit_mm->seq_points, imethod->method, imethod->jinfo->seq_points);
dn_simdhash_ght_replace (jit_mm->seq_points, imethod->method, imethod->jinfo->seq_points);
}
jit_mm_unlock (jit_mm);

Expand Down
8 changes: 5 additions & 3 deletions src/mono/mono/mini/interp/transform.h
Original file line number Diff line number Diff line change
Expand Up @@ -311,10 +311,12 @@ typedef struct
int n_data_items;
int max_data_items;
void **data_items;
GHashTable *data_hash;
// FIXME: ptr_u32
dn_simdhash_ptr_ptr_t *data_hash;
GSList *imethod_items;
#ifdef ENABLE_EXPERIMENT_TIERED
GHashTable *patchsite_hash;
// FIXME: ptr_u32
dn_simdhash_ptr_ptr_t *patchsite_hash;
#endif
int *clause_indexes;
int *clause_vars;
Expand Down Expand Up @@ -504,7 +506,7 @@ interp_dump_ins (InterpInst *ins, gpointer *data_items);
InterpInst*
interp_get_ldc_i4_from_const (TransformData *td, InterpInst *ins, gint32 ct, int dreg);

gint32
gint32
interp_get_const_from_ldc_i4 (InterpInst *ins);

int
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/mini/interp/whitebox.c
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ transform_method (MonoDomain *domain, MonoImage *image, TestItem *ti)
td->rtm = rtm;
td->clause_indexes = (int*)g_malloc (header->code_size * sizeof (int));
td->data_items = NULL;
td->data_hash = g_hash_table_new (NULL, NULL);
td->data_hash = dn_simdhash_ptr_ptr_new (0, NULL);
/* TODO: init more fields of `td` */

mono_test_interp_method_compute_offsets (td, rtm, signature, header);
Expand Down
Loading

0 comments on commit 2da65a9

Please sign in to comment.