[Mono-devel-list] Cross application domain call optimization, Take 2.

[Paolo Molaro]

On 10/26/04 Lluis Sanchez wrote:
> +				do {
> +					try {
> +						MemoryStream ms = new MemoryStream ();
> +						_serializationFormatter.Serialize (ms, ex);
> +						return ms.ToArray ();
> +					}
> +					catch (Exception e) {
> +						if (e is ThreadAbortException) {
> +							Thread.ResetAbort ();
> +							retry = 5;
> +							ex = e;
> +						}
> +						else if (retry == 2) {

the catch clause should be in the same line as the '}'. Same for the else
clause here and in other places below.

> --- mono/metadata/marshal.c	20 Oct 2004 01:01:56 -0000	1.210
> +++ mono/metadata/marshal.c	26 Oct 2004 11:09:39 -0000
[...]
> +	if (!module_initialized) {
> +		klass = mono_class_from_name (mono_defaults.corlib, "System.Runtime.Remoting", "RemotingServices");
> +		method_rs_serialize = mono_class_get_method_from_name (klass, "SerializeCallData", -1);
> +		method_rs_deserialize = mono_class_get_method_from_name (klass, "DeserializeCallData", -1);
> +		method_rs_serialize_exc = mono_class_get_method_from_name (klass, "SerializeExceptionData", -1);
> +	
> +		klass = mono_class_from_name (mono_defaults.corlib, "System.Runtime.Remoting.Proxies", "RealProxy");
> +		method_rs_appdomain_target = mono_class_get_method_from_name (klass, "GetAppDomainTarget", -1);
> +	
> +		klass = mono_class_from_name (mono_defaults.corlib, "System", "Exception");
> +		method_exc_fixexc = mono_class_get_method_from_name (klass, "FixRemotingException", -1);
> +	
> +		klass = mono_class_from_name (mono_defaults.corlib, "System.Threading", "Thread");
> +		method_reset_datastore = mono_class_get_method_from_name (klass, "ResetDataStoreStatus", -1);
> +		method_restore_datastore = mono_class_get_method_from_name (klass, "RestoreDataStoreStatus", -1);
> +		method_get_context = mono_class_get_method_from_name (klass, "get_CurrentContext", -1);
> +	
> +		klass = mono_class_from_name (mono_defaults.corlib, "System", "AppDomain");

Exception, Thread and AppDomain are aòlreay in mono_Defaults, no need to look them up again.

> +static inline MonoMethod*
> +mono_marshal_remoting_find_in_cache (MonoMethod *method, int wrapper_type)
> +{
> +	MonoMethod *res = NULL;
> +	MonoRemotingMethods *wrps;
> +
> +	EnterCriticalSection (&marshal_mutex);
> +	wrps = (MonoRemotingMethods*) g_hash_table_lookup (method->klass->image->remoting_invoke_cache, method);

There is no need for the cast, it makes for cleaner code without it, IMHO.

> +	if (*res == NULL) {
> +		/* This does not acquire any locks */
> +		*res = mono_mb_create_method (mb, sig, max_stack);
> +		mono_g_hash_table_insert (wrapper_hash, *res, key);
> +	}
> +	else
> +		/* Somebody created it before us */
> +		;

Please remove these three lines.

> +/* mono_get_xdomain_marshal_type()
> + * Returns the kind of marshalling that a type needs for cross domain calls.
> + */
> +static MonoXDomainMarshalType
> +mono_get_xdomain_marshal_type (MonoType *t)
> +{
> +	switch (t->type) {

Can you ever get a byref type here?

> +/* mono_marshal_emit_load_domain_method ()
> + * Loads into the stack a pointer to the code of the provided method for
> + * the current domain.
> + */
> +static void
> +mono_marshal_emit_load_domain_method (MonoMethodBuilder *mb, MonoMethod *method)
> +{
> +	static MonoMethodSignature *csig = NULL;
> +	if (!csig) {
> +		csig = mono_metadata_signature_alloc (mono_defaults.corlib, 1);
> +		csig->params [0] = &mono_defaults.int_class->byval_arg;
> +		csig->ret = &mono_defaults.int_class->byval_arg;
> +		csig->pinvoke = 1;
> +	}
> +
> +	mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
> +	mono_mb_emit_byte (mb, CEE_MONO_LDPTR);
> +	mono_mb_emit_i4 (mb, mono_mb_add_data (mb, method));
> +	mono_mb_emit_native_call (mb, csig, mono_compile_method);

Why do you need to compile the method? It seems unusual. Maybe you can just use LDFTN?

> +	for (i = 0; i < sig->param_count; i++)
> +		if (marshal_types [i] != MONO_MARSHAL_SERIALIZE)
> +			csig->params [j++] = sig->params [i];

This copies also the attributes of the params: is that the intended behaviour?
Also, if the statement inside the loop is not a one-liner, it's better to use braces:

	for (...) {
		if (cond)
			stmt;
	}

> +	/* Main exception catch */
> +	main_clause->flags = MONO_EXCEPTION_CLAUSE_FILTER;
> +	main_clause->try_len = mb->pos - main_clause->try_offset;
> +	main_clause->token_or_filter = mb->pos;
> +	/* filter code */
> +	mono_mb_emit_byte (mb, CEE_POP);
> +	mono_mb_emit_byte (mb, CEE_LDC_I4_1);
> +	mono_mb_emit_byte (mb, CEE_PREFIX1);
> +	mono_mb_emit_byte (mb, CEE_ENDFILTER);

Why do you need a filter that always catches the exception?

> +void
> +mono_array_full_copy (MonoArray *src, MonoArray *dest)
> +{
> +	int size, i;
> +	MonoClass *klass = src->obj.vtable->klass;
> +
> +	MONO_ARCH_SAVE_REGS;
> +
> +	g_assert (klass == dest->obj.vtable->klass);
> +
> +	if (src->bounds == NULL) {
> +		size = mono_array_length (src);
> +		g_assert (size == mono_array_length (dest));
> +		size *= mono_array_element_size (klass);
> +		memcpy (&dest->vector, &src->vector, size);
> +		return;
> +	}
> +	
> +	size = mono_array_element_size (klass);
> +	for (i = 0; i < klass->rank; ++i) {
> +		g_assert (src->bounds [i].length == dest->bounds [i].length);
> +		size *= src->bounds [i].length;
> +	}
> +	memcpy (&dest->vector, &src->vector, size);

mono_array_length (src)  should already have the total size, so the loop is
not needed and the above code for SZARRAY is fine.

Thanks!

lupus

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