[Mono-devel-list] Re: [Mono-patches] r36936 - trunk/mono/mono/metadata

[Ben Maurer]

> +        nginst = g_new0 (MonoGenericInst, 1);
> +        nginst->type_argc = ginst->type_argc;
> +        nginst->type_argv = g_new0 (MonoType*, nginst->type_argc);

Is there any reason not to make MonoGenericInst a variable lengh
structure. This would avoid making two mallocs.

> +static guint
> +mono_generic_inst_hash (gconstpointer data)
> +{
> +	const MonoGenericInst *ginst = (const MonoGenericInst *) data;
> +	return ginst->type_argc | (ginst->is_open << 8);
> +}

This hash function looks like it will have tons of conflicts. It does not
take into account this part of equals:

+        for (i = 0; i < a->type_argc; ++i) {
+                if (!do_mono_metadata_type_equal (a->type_argv [i],
b->type_argv [i], FALSE))
+                        return FALSE;
+        }
> +MonoGenericInst *
> +mono_metadata_lookup_generic_inst (MonoGenericInst *ginst)
> +{
> +	MonoGenericInst *cached;
> +	int i;
> +
> +	cached = g_hash_table_lookup (generic_inst_cache, ginst);
> +	if (cached) {
> +		for (i = 0; i < ginst->type_argc; i++)
> +			mono_metadata_free_type (ginst->type_argv [i]);
> +		g_free (ginst->type_argv);
> +		g_free (ginst);
> +		return cached;
> +	}
> +
> +	ginst->id = ++next_generic_inst_id;
> +	g_hash_table_insert (generic_inst_cache, ginst, ginst);
> +
> +	return ginst;
> +}

Who locks this?

-- Ben