Skip to content

Commit

Permalink
[mono] miscellaneous AOT optimizations (#102039)
Browse files Browse the repository at this point in the history
* Lazily evaluate full name of methods when creating jiterpreter entry trampolines
* Optimize out an unnecessary memset in do_jit_call
* Migrate some hot ghashtables to simdhash
  • Loading branch information
kg committed May 10, 2024
1 parent 00a8973 commit 0235164
Show file tree
Hide file tree
Showing 14 changed files with 106 additions and 67 deletions.
81 changes: 52 additions & 29 deletions src/mono/browser/runtime/jiterpreter-interp-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,45 +91,66 @@ class TrampolineInfo {
hasThisReference: boolean;
unbox: boolean;
hasReturnValue: boolean;
name: string;
traceName: string;
private name?: string;
private traceName?: string;

defaultImplementation: number;
result: number;
hitCount: number;

constructor (
imethod: number, method: MonoMethod, argumentCount: number, pParamTypes: NativePointer,
unbox: boolean, hasThisReference: boolean, hasReturnValue: boolean, name: string,
defaultImplementation: number
unbox: boolean, hasThisReference: boolean, hasReturnValue: boolean, defaultImplementation: number
) {
this.imethod = imethod;
this.method = method;
this.argumentCount = argumentCount;
this.unbox = unbox;
this.hasThisReference = hasThisReference;
this.hasReturnValue = hasReturnValue;
this.name = name;
this.paramTypes = new Array(argumentCount);
for (let i = 0; i < argumentCount; i++)
this.paramTypes[i] = <any>getU32_unaligned(<any>pParamTypes + (i * 4));
this.defaultImplementation = defaultImplementation;
this.result = 0;
let subName = name;
if (!subName) {
subName = `${this.imethod.toString(16)}_${this.hasThisReference ? "i" : "s"}${this.hasReturnValue ? "_r" : ""}_${this.argumentCount}`;
} else {
// truncate the real method name so that it doesn't make the module too big. this isn't a big deal for module-per-function,
// but since we jit in groups now we need to keep the sizes reasonable. we keep the tail end of the name
// since it is likely to contain the method name and/or signature instead of type and noise
const maxLength = 24;
if (subName.length > maxLength)
subName = subName.substring(subName.length - maxLength, subName.length);
subName = `${this.imethod.toString(16)}_${subName}`;
}
this.traceName = subName;
this.hitCount = 0;
}

generateName () {
const namePtr = cwraps.mono_wasm_method_get_full_name(this.method);
try {
const name = utf8ToString(namePtr);
this.name = name;
let subName = name;
if (!subName) {
subName = `${this.imethod.toString(16)}_${this.hasThisReference ? "i" : "s"}${this.hasReturnValue ? "_r" : ""}_${this.argumentCount}`;
} else {
// truncate the real method name so that it doesn't make the module too big. this isn't a big deal for module-per-function,
// but since we jit in groups now we need to keep the sizes reasonable. we keep the tail end of the name
// since it is likely to contain the method name and/or signature instead of type and noise
const maxLength = 24;
if (subName.length > maxLength)
subName = subName.substring(subName.length - maxLength, subName.length);
subName = `${this.imethod.toString(16)}_${subName}`;
}
this.traceName = subName;
} finally {
if (namePtr)
Module._free(<any>namePtr);
}
}

getTraceName () {
if (!this.traceName)
this.generateName();
return this.traceName || "unknown";
}

getName () {
if (!this.name)
this.generateName();
return this.name || "unknown";
}
}

let mostRecentOptions: JiterpreterOptions | undefined = undefined;
Expand Down Expand Up @@ -170,17 +191,15 @@ export function mono_interp_record_interp_entry (imethod: number) {
// returns function pointer
export function mono_interp_jit_wasm_entry_trampoline (
imethod: number, method: MonoMethod, argumentCount: number, pParamTypes: NativePointer,
unbox: boolean, hasThisReference: boolean, hasReturnValue: boolean, name: NativePointer,
defaultImplementation: number
unbox: boolean, hasThisReference: boolean, hasReturnValue: boolean, defaultImplementation: number
): number {
// HACK
if (argumentCount > maxInlineArgs)
return 0;

const info = new TrampolineInfo(
imethod, method, argumentCount, pParamTypes,
unbox, hasThisReference, hasReturnValue, utf8ToString(<any>name),
defaultImplementation
unbox, hasThisReference, hasReturnValue, defaultImplementation
);
if (!fnTable)
fnTable = getWasmFunctionTable();
Expand Down Expand Up @@ -311,7 +330,7 @@ function flush_wasm_entry_trampoline_jit_queue () {

// Function type for compiled traces
builder.defineType(
info.traceName, sig, WasmValtype.void, false
info.getTraceName(), sig, WasmValtype.void, false
);
}

Expand All @@ -338,17 +357,19 @@ function flush_wasm_entry_trampoline_jit_queue () {
builder.appendULeb(jitQueue.length);
for (let i = 0; i < jitQueue.length; i++) {
const info = jitQueue[i];
const traceName = info.getTraceName();
// Function type for our compiled trace
mono_assert(builder.functionTypes[info.traceName], "func type missing");
builder.appendULeb(builder.functionTypes[info.traceName][0]);
mono_assert(builder.functionTypes[traceName], "func type missing");
builder.appendULeb(builder.functionTypes[traceName][0]);
}

// Export section
builder.beginSection(7);
builder.appendULeb(jitQueue.length);
for (let i = 0; i < jitQueue.length; i++) {
const info = jitQueue[i];
builder.appendName(info.traceName);
const traceName = info.getTraceName();
builder.appendName(traceName);
builder.appendU8(0);
// Imports get added to the function index space, so we need to add
// the count of imported functions to get the index of our compiled trace
Expand All @@ -360,15 +381,16 @@ function flush_wasm_entry_trampoline_jit_queue () {
builder.appendULeb(jitQueue.length);
for (let i = 0; i < jitQueue.length; i++) {
const info = jitQueue[i];
builder.beginFunction(info.traceName, {
const traceName = info.getTraceName();
builder.beginFunction(traceName, {
"sp_args": WasmValtype.i32,
"need_unbox": WasmValtype.i32,
"scratchBuffer": WasmValtype.i32,
});

const ok = generate_wasm_body(builder, info);
if (!ok)
throw new Error(`Failed to generate ${info.traceName}`);
throw new Error(`Failed to generate ${traceName}`);

builder.appendU8(WasmOpcode.end);
builder.endFunction(true);
Expand All @@ -390,9 +412,10 @@ function flush_wasm_entry_trampoline_jit_queue () {
// to point to the new jitted trampolines instead of the default implementations
for (let i = 0; i < jitQueue.length; i++) {
const info = jitQueue[i];
const traceName = info.getTraceName();

// Get the exported trampoline
const fn = traceInstance.exports[info.traceName];
const fn = traceInstance.exports[traceName];
// Patch the function pointer for this function to use the trampoline now
fnTable.set(info.result, fn);

Expand Down
11 changes: 6 additions & 5 deletions src/mono/mono/metadata/class.c
Original file line number Diff line number Diff line change
Expand Up @@ -1168,7 +1168,7 @@ MonoMethod*
mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *klass_hint, MonoGenericContext *context, MonoError *error)
{
MonoMethod *result;
MonoMethodInflated *iresult, *cached;
MonoMethodInflated *iresult, *cached = NULL;
MonoMethodSignature *sig;
MonoGenericContext tmp_context;

Expand Down Expand Up @@ -1223,8 +1223,8 @@ mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *k
// check cache
mono_mem_manager_lock (mm);
if (!mm->gmethod_cache)
mm->gmethod_cache = g_hash_table_new_full (inflated_method_hash, inflated_method_equal, NULL, (GDestroyNotify)free_inflated_method);
cached = (MonoMethodInflated *)g_hash_table_lookup (mm->gmethod_cache, iresult);
mm->gmethod_cache = dn_simdhash_ght_new_full (inflated_method_hash, inflated_method_equal, NULL, (GDestroyNotify)free_inflated_method, 0, NULL);
dn_simdhash_ght_try_get_value (mm->gmethod_cache, iresult, (void **)&cached);
mono_mem_manager_unlock (mm);

if (cached) {
Expand Down Expand Up @@ -1319,9 +1319,10 @@ mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *k

// check cache
mono_mem_manager_lock (mm);
cached = (MonoMethodInflated *)g_hash_table_lookup (mm->gmethod_cache, iresult);
cached = NULL;
dn_simdhash_ght_try_get_value (mm->gmethod_cache, iresult, (void **)&cached);
if (!cached) {
g_hash_table_insert (mm->gmethod_cache, iresult, iresult);
dn_simdhash_ght_insert (mm->gmethod_cache, iresult, iresult);
iresult->owner = mm;
cached = iresult;
}
Expand Down
4 changes: 2 additions & 2 deletions src/mono/mono/metadata/image.c
Original file line number Diff line number Diff line change
Expand Up @@ -2066,9 +2066,9 @@ mono_image_close_except_pools (MonoImage *image)
}

if (image->method_cache)
g_hash_table_destroy (image->method_cache);
dn_simdhash_free (image->method_cache);
if (image->methodref_cache)
g_hash_table_destroy (image->methodref_cache);
dn_simdhash_free (image->methodref_cache);
mono_internal_hash_table_destroy (&image->class_cache);
mono_conc_hashtable_destroy (image->field_cache);
if (image->array_cache) {
Expand Down
4 changes: 3 additions & 1 deletion src/mono/mono/metadata/loader-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <mono/utils/mono-error.h>
#include <mono/utils/mono-forward.h>
#include <mono/utils/mono-conc-hashtable.h>
#include "../native/containers/dn-simdhash-specializations.h"

#if defined(TARGET_OSX)
#define MONO_LOADER_LIBRARY_NAME "libcoreclr.dylib"
Expand Down Expand Up @@ -174,7 +175,8 @@ struct _MonoMemoryManager {
MonoAssemblyLoadContext **alcs;

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

/* mirror caches of ones already on MonoImage. These ones contain generics */
Expand Down
18 changes: 9 additions & 9 deletions src/mono/mono/metadata/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -1182,12 +1182,12 @@ mono_get_method_checked (MonoImage *image, guint32 token, MonoClass *klass, Mono

if (mono_metadata_token_table (token) == MONO_TABLE_METHOD) {
if (!image->method_cache)
image->method_cache = g_hash_table_new (NULL, NULL);
result = (MonoMethod *)g_hash_table_lookup (image->method_cache, GINT_TO_POINTER (mono_metadata_token_index (token)));
image->method_cache = dn_simdhash_u32_ptr_new (0, NULL);
dn_simdhash_u32_ptr_try_get_value (image->method_cache, mono_metadata_token_index (token), (void **)&result);
} else if (!image_is_dynamic (image)) {
if (!image->methodref_cache)
image->methodref_cache = g_hash_table_new (NULL, NULL);
result = (MonoMethod *)g_hash_table_lookup (image->methodref_cache, GINT_TO_POINTER (token));
image->methodref_cache = dn_simdhash_u32_ptr_new (0, NULL);
dn_simdhash_u32_ptr_try_get_value (image->methodref_cache, token, (void **)&result);
}
mono_image_unlock (image);

Expand All @@ -1204,19 +1204,19 @@ mono_get_method_checked (MonoImage *image, guint32 token, MonoClass *klass, Mono
MonoMethod *result2 = NULL;

if (mono_metadata_token_table (token) == MONO_TABLE_METHOD)
result2 = (MonoMethod *)g_hash_table_lookup (image->method_cache, GINT_TO_POINTER (mono_metadata_token_index (token)));
dn_simdhash_u32_ptr_try_get_value (image->method_cache, mono_metadata_token_index (token), (void **)&result2);
else if (!image_is_dynamic (image))
result2 = (MonoMethod *)g_hash_table_lookup (image->methodref_cache, GINT_TO_POINTER (token));
dn_simdhash_u32_ptr_try_get_value (image->methodref_cache, token, (void **)&result2);

if (result2) {
mono_image_unlock (image);
return result2;
}

if (mono_metadata_token_table (token) == MONO_TABLE_METHOD)
g_hash_table_insert (image->method_cache, GINT_TO_POINTER (mono_metadata_token_index (token)), result);
dn_simdhash_u32_ptr_try_add (image->method_cache, mono_metadata_token_index (token), result);
else if (!image_is_dynamic (image))
g_hash_table_insert (image->methodref_cache, GINT_TO_POINTER (token), result);
dn_simdhash_u32_ptr_try_add (image->methodref_cache, token, result);
}

mono_image_unlock (image);
Expand Down Expand Up @@ -1514,7 +1514,7 @@ mono_method_get_param_token (MonoMethod *method, int index)
idx = mono_method_get_index (method);
if (idx > 0) {
guint param_index = mono_metadata_get_method_params (klass_image, idx, NULL);

if (index == -1)
/* Return value */
return mono_metadata_make_token (MONO_TABLE_PARAM, 0);
Expand Down
12 changes: 10 additions & 2 deletions src/mono/mono/metadata/memory-manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,14 @@ free_hash (GHashTable **hash)
*hash = NULL;
}

static void
free_simdhash (dn_simdhash_t **hash)
{
if (*hash)
dn_simdhash_free (*hash);
*hash = NULL;
}

// Full deletion
static void
memory_manager_delete (MonoMemoryManager *memory_manager, gboolean debug_unload)
Expand All @@ -230,8 +238,8 @@ memory_manager_delete (MonoMemoryManager *memory_manager, gboolean debug_unload)
MonoMemoryManager *mm = memory_manager;
if (mm->gclass_cache)
mono_conc_hashtable_destroy (mm->gclass_cache);
free_hash (&mm->ginst_cache);
free_hash (&mm->gmethod_cache);
free_simdhash (&mm->ginst_cache);
free_simdhash (&mm->gmethod_cache);
free_hash (&mm->gsignature_cache);
free_hash (&mm->szarray_cache);
free_hash (&mm->array_cache);
Expand Down
4 changes: 2 additions & 2 deletions src/mono/mono/metadata/metadata-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -418,11 +418,11 @@ struct _MonoImage {
/*
* Indexed by method tokens and typedef tokens.
*/
GHashTable *method_cache; /*protected by the image lock*/
dn_simdhash_u32_ptr_t *method_cache; /*protected by the image lock*/
MonoInternalHashTable class_cache;

/* Indexed by memberref + methodspec tokens */
GHashTable *methodref_cache; /*protected by the image lock*/
dn_simdhash_u32_ptr_t *methodref_cache; /*protected by the image lock*/

/*
* Indexed by fielddef and memberref tokens
Expand Down
7 changes: 4 additions & 3 deletions src/mono/mono/metadata/metadata.c
Original file line number Diff line number Diff line change
Expand Up @@ -3449,9 +3449,10 @@ mono_metadata_get_canonical_generic_inst (MonoGenericInst *candidate)
mono_loader_lock ();

if (!mm->ginst_cache)
mm->ginst_cache = g_hash_table_new_full (mono_metadata_generic_inst_hash, mono_metadata_generic_inst_equal, NULL, (GDestroyNotify)free_generic_inst);
mm->ginst_cache = dn_simdhash_ght_new_full (mono_metadata_generic_inst_hash, mono_metadata_generic_inst_equal, NULL, (GDestroyNotify)free_generic_inst, 0, NULL);

MonoGenericInst *ginst = (MonoGenericInst *)g_hash_table_lookup (mm->ginst_cache, candidate);
MonoGenericInst *ginst = NULL;
dn_simdhash_ght_try_get_value (mm->ginst_cache, candidate, (void **)&ginst);
if (!ginst) {
int size = MONO_SIZEOF_GENERIC_INST + type_argc * sizeof (MonoType *);
ginst = (MonoGenericInst *)mono_mem_manager_alloc0 (mm, size);
Expand All @@ -3465,7 +3466,7 @@ mono_metadata_get_canonical_generic_inst (MonoGenericInst *candidate)
for (int i = 0; i < type_argc; ++i)
ginst->type_argv [i] = mono_metadata_type_dup (NULL, candidate->type_argv [i]);

g_hash_table_insert (mm->ginst_cache, ginst, ginst);
dn_simdhash_ght_insert (mm->ginst_cache, ginst, ginst);
}

mono_loader_unlock ();
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/mini/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ set(imported_native_sources
../../../native/containers/dn-simdhash-ptr-ptr.c
../../../native/containers/dn-simdhash-string-ptr.c
../../../native/containers/dn-simdhash-u32-ptr.c
../../../native/containers/dn-simdhash-ght-compatible.c
../../../native/containers/dn-simdhash-ptrpair-ptr.c)

set(mini_common_sources
Expand Down
Loading

0 comments on commit 0235164

Please sign in to comment.