[Mono-dev] Patch: Fast virtual generic method calls

[Paolo Molaro]

On 10/01/08 Mark Probst wrote:
> Here's the patch again, updated with code to keep track of how many
> times a generic virtual method is invoked and to insert it in the thunk
> only if a threshold (currently 100) is reached.

100 seems high, especially since the lookup can be a O(n) list walk.
Maybe 10 is more appropriate.

> Index: metadata/object.c
> ===================================================================
> --- metadata/object.c	(revision 114206)
> +++ metadata/object.c	(working copy)
> +static void
> +init_thunk_free_lists (MonoDomain *domain)
> +{
> +	if (domain->thunk_free_lists)
> +		return;
> +	domain->thunk_free_lists = mono_mempool_alloc0 (domain->mp, sizeof (gpointer) * NUM_FREE_LISTS);

mono_domain_alloc0 () should be used, so that all the domain allocations
are properly tracked.

> +/*
> + * LOCKING: The domain lock must be held.
> + */
> +static void
> +invalidate_generic_virtual_thunk (MonoDomain *domain, gpointer code)
> +{
> +	guint32 *p = code;
> +	MonoThunkFreeList *l = (MonoThunkFreeList*)(p - 1);
> +
> +	init_thunk_free_lists (domain);
> +
> +	while (domain->thunk_free_lists [0] && domain->thunk_free_lists [0]->length >= MAX_WAIT_LENGTH) {
> +		MonoThunkFreeList *item = domain->thunk_free_lists [0];
> +		int length = item->length;
> +		int i;
> +
> +		/* unlink the first item from the wait list */
> +		domain->thunk_free_lists [0] = item->next;
> +		domain->thunk_free_lists [0]->length = length - 1;
> +
> +		i = list_index_for_size (item->size);
> +
> +		g_print ("putting thunk of size %d in list %d\n", item->size, i);

Leftover.

> +/**
> + * mono_method_add_generic_virtual_case:

Maybe invocation is better than case? Any other suggestion?

> +void
> +mono_method_add_generic_virtual_case (MonoDomain *domain, gpointer *vtable_slot,
> +	MonoGenericInst *method_inst, gpointer code)
> +{
> +	static gboolean inited = FALSE;
> +	static int num_added = 0;
> +
> +	GenericVirtualCase *gvc, *list;
> +	MonoImtBuilderEntry *entries;
> +	int i;
> +	GPtrArray *sorted;
> +
> +	mono_domain_lock (domain);
> +	if (!domain->generic_virtual_cases)
> +		domain->generic_virtual_cases = g_hash_table_new (mono_aligned_addr_hash, NULL);
> +
> +	/* Check whether the case was already added */
> +	gvc = g_hash_table_lookup (domain->generic_virtual_cases, vtable_slot);
> +	while (gvc) {
> +		if (gvc->inst == method_inst)
> +			break;
> +		gvc = gvc->next;
> +	}

This is the O(n) loop I mentioned. It shouldn't be a big deal, though.

> +
> +	/* If not found, make a new one */
> +	if (!gvc) {
> +		gvc = mono_mempool_alloc (domain->mp, sizeof (GenericVirtualCase));

mono_domain_alloc () here.

> Index: metadata/domain-internals.h
> ===================================================================
> --- metadata/domain-internals.h	(revision 114206)
> +++ metadata/domain-internals.h	(working copy)
> @@ -141,6 +141,12 @@
>  	MONO_APPDOMAIN_UNLOADED
>  } MonoAppDomainState;
>  
> +typedef struct _MonoThunkFreeList {
> +	guint32 size;
> +	struct _MonoThunkFreeList *next;
> +	int length;		/* only valid for the wait list */

Put the pointer first, so sie and length fit inside the other 8 bytes on
64 bit systems and the total size will be 16 instead of 24.

> --- mini/tramp-x86.c	(revision 114206)
> +++ mini/tramp-x86.c	(working copy)
> @@ -94,7 +94,7 @@
>  	} else {
>  		printf ("Invalid trampoline sequence: %x %x %x %x %x %x %x\n", code [0], code [1], code [2], code [3],
>  				code [4], code [5], code [6]);
> -		g_assert_not_reached ();
> +		//g_assert_not_reached ();

Leftover.

The rest of the code looks fine to me.
It would be nice to also see some speedup numbers from this change:)
Thanks!

lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better