Ok, here's an updated patch which caches the method for reuse as long
as both the list type and listitemtype are the same as the previous
invocation.
This gives a modest 13% increase in performance for the testcase i was
using. Let me know if this is good to commit as-is.
Just as a point of style, should i put the variable declarations at
the top of the .cs file with a comment specifying how they are used?
Or should i leave them as they are.
Alan.
On Jan 10, 2008 3:26 AM, Alan McGovern <[EMAIL PROTECTED]> wrote:
> (and to the list again... doh)
>
>
> On Jan 10, 2008 3:26 AM, Alan McGovern <[EMAIL PROTECTED]> wrote:
> > Hi,
> >
> > Yeah, by reducing the method searchs performance increases by 15% or
> > so. I'll work that patch up tomorrow at some stage.
> >
> > Thanks,
> > Alan.
> >
> >
> > On Jan 10, 2008 1:07 AM, Robert Jordan <[EMAIL PROTECTED]> wrote:
> > > Hi,
> > >
> > > I've filed it:
> > >
> > > https://bugzilla.novell.com/show_bug.cgi?id=352805
> > >
> > > BTW, the "previousObject" check in your patch is still useful,
> > > as it cuts down the type.GetMethod ("Add") calls from
> > > collectionLength to 1, if I understand the patch correctly.
> > >
> > > Just remove the CreateDelegate stuff and extend the optimization
> > > to cover NET 1.1 as well.
> > >
> > > Robert
> > >
> > >
> > >
> > > Alan McGovern wrote:
> > > > (and also sending to the list...)
> > > >
> > > > On Jan 10, 2008 12:41 AM, Alan McGovern <[EMAIL PROTECTED]> wrote:
> > > >> Hi,
> > > >>
> > > >> I was wondering about that alright. It did seem a bit weird that it
> > > >> would work, i would've expected the delegate parameter to be at least
> > > >> as restrictive as the method i was calling. Bang goes that idea then.
> > > >>
> > > >> Alan.
> > > >>
> > > >>
> > > >> On Jan 10, 2008 12:36 AM, Robert Jordan <[EMAIL PROTECTED]> wrote:
> > > >>> Robert Jordan wrote:
> > > >>>> Alan McGovern wrote:
> > > >>>>> There was a thread a week or two ago called 'Speed difference
> > > >>>>> Windows
> > > >>>>> - Linux' which noted there was a big difference in performance
> > > >>>>> between
> > > >>>>> .NET and mono. I did a brief bit of profiling and came up with this
> > > >>>>> patch which improves performance ~30% for the testcase which was
> > > >>>>> attached in the email. This reduces runtime memory usage by about
> > > >>>>> 10MB
> > > >>>>> (80MB -> 70MB) and decreases processing time by 30% (3.4s -> 2.6s).
> > > >>>>>
> > > >>>>> Anyone have any ideas on how to tidy this up to make it neater?
> > > >>>>> Also,
> > > >>>>> would this optimisation be too specific, or can it be generalised
> > > >>>>> somewhere higher up in the stack.
> > > >>>> I don't think the patch is correct. It is assuming that every
> > > >>>> "Add" method of a collection/list is compatible with
> > > >>>> AddDelegate(object).
> > > >>>>
> > > >>>> If the test cases are still working, it could be that
> > > >>>> CreateDelegate is buggy: MSDN states:
> > > >>>>
> > > >>>> "A parameter of a delegate is compatible with the corresponding
> > > >>>> parameter of a method if the type of the delegate parameter is more
> > > >>>> restrictive than the type of the method parameter, because this
> > > >>>> guarantees that an argument passed to the delegate can be passed
> > > >>>> safely
> > > >>>> to the method."
> > > >>> It's indeed a bug in Mono's CreateDelegate. The following test case
> > > >>> must fail but it doesn't:
> > > >>>
> > > >>> using System;
> > > >>>
> > > >>> delegate void Method(object o);
> > > >>>
> > > >>> class T
> > > >>> {
> > > >>> static void Main ()
> > > >>> {
> > > >>> T t = new T ();
> > > >>> Method m = (Method) Delegate.CreateDelegate
> > > >>> (typeof(Method), t,
> > > >>> t.GetType ().GetMethod ("Test"));
> > > >>> m (new Uri ("http://mono-project.com";));
> > > >>> }
> > > >>>
> > > >>> public void Test (Uri uri)
> > > >>> {
> > > >>> Console.WriteLine (uri);
> > > >>>
> > > >>> }
> > > >>> }
> > > >>>
> > > >>> Robert
> > > >>>
> > > >>> _______________________________________________
> > > >>> Mono-list maillist - [email protected]
> > > >>> http://lists.ximian.com/mailman/listinfo/mono-list
> > > >>>
> > > > _______________________________________________
> > > > Mono-list maillist - [email protected]
> > > > http://lists.ximian.com/mailman/listinfo/mono-list
> > > >
> > >
> > >
> >
>
Index: class/System.XML/System.Xml.Serialization/XmlSerializationReaderInterpreter.cs
===================================================================
--- class/System.XML/System.Xml.Serialization/XmlSerializationReaderInterpreter.cs (revision 90185)
+++ class/System.XML/System.Xml.Serialization/XmlSerializationReaderInterpreter.cs (working copy)
@@ -723,6 +723,9 @@
return list;
}
+
+ private MethodInfo addMethod;
+ private TypeData previousType;
void AddListValue (TypeData listType, ref object list, int index, object value, bool canCreateInstance)
{
Type type = listType.Type;
@@ -737,12 +740,18 @@
if (canCreateInstance) list = Activator.CreateInstance (type, true);
else throw CreateReadOnlyCollectionException (type.FullName);
}
+
+ // If the current TypeData does not match the one that was sent previously, recreate the methodinfo
+ if (addMethod == null || previousType.Type != listType.Type || previousType.ListItemType != listType.ListItemType)
+ {
+ previousType = listType;
+ addMethod = type.GetMethod ("Add", new Type[] {listType.ListItemType} );
+ }
- MethodInfo mi = type.GetMethod ("Add", new Type[] {listType.ListItemType} );
- mi.Invoke (list, new object[] { value });
+ addMethod.Invoke(list, new object[] { value });
}
}
-
+
object CreateInstance (Type type)
{
return Activator.CreateInstance (type, empty_array);
_______________________________________________
Mono-list maillist - [email protected]
http://lists.ximian.com/mailman/listinfo/mono-list
