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
--
-----------------------------------------------------------------
[EMAIL PROTECTED] debian/rules
[EMAIL PROTECTED] Monkeys do it better
_______________________________________________
Mono-devel-list mailing list
[email protected]
http://lists.ximian.com/mailman/listinfo/mono-devel-list
