[Mono-devel-list] Patch for ToUpper/ToLower

[Andreas Nahr]

> I'm not sure if I understand why you want to move the check for the
> invariant culture to managed code.  We would have to re-implement
> g_unichar_toupper()/g_unichar_tolower in managed code. We seem to have
> already done that in Char.ToUpper()/Char.ToLower(), however the glib
> implementation seems to take care of some special cases that we don't.
> (http://cvs.gnome.org/viewcvs/glib/glib/guniprop.c?rev=1.33&view=auto)

I looked at their implementation and now I understand why my Char.ToXXX
functions are much faster ;)
However I could not find any special cases that glib handles that we do not
(for the invariant case). If you find any, please write which!
So it's all about speed...

> Wouldn't it be safer to use the glib version?  As for the string, it
> doesn't seem like we'd be saving anything by doing the invariant culture
> check in managed code.  If we did the Upper/Lower conversion in managed
> code, we'd still have to go to unmanaged code to use the indexer or get
> it as a char[] to do the actual conversion on each character.

No - you do not have to use unmanaged code for that. However you still need
to Allocate a string, which unfortunatelly needs an internalcall.
(Finding a way to not need the internalcall for constructing a string with
InternalAllocateString would really give the string class a speed boost).

However I attached my current version, which is (only a bit, but measurably)
faster that the internalcall for most cases.
Here are some benchmarks (Probably +- 20 ms):
New version:
huge string with much conversation: 14501ms
huge string with little conversation: 14471ms
small string with much conversation: 1062ms
small string with little conversation: 1071ms

Old version (current CVS):
huge string with much conversation: 16193ms
huge string with little conversation: 14771ms
small string with much conversation: 1141ms
small string with little conversation: 1051ms

Andreas

> rob
>
> On Sat, 2004-04-24 at 04:47, Andreas Nahr wrote:
> > We should not make the null check in unmanaged code.
> > Also the shortcut for invariant code should not be taken to c land (as
it is
> > currently) - I'll explain in another posting why.
> >
> > I've made a patch which is the neccessary C# part for it. Perhaps you
could
> > make and test the C part (renaming the function and removing the
invariant
> > shortcut from the Char functions)
> >
> > The same thing is true for String (it's even more important there
because
> > e.g. for String.ToLower() we can guarantee that it never calles with a
null)
> > Already applied the String one as it is trivial
> >
> > A.Nahr
> >
> > ----- Original Message ----- 
> > From: "Robert Shade" <rshade at dvsconsulting.com>
> > To: <mono-devel-list at lists.ximian.com>
> > Sent: Saturday, April 24, 2004 12:07 AM
> > Subject: [Mono-devel-list] Patch for ToUpper/ToLower
> >
> >
> > > According to the documentation, String.ToUpper/ToLower and
> > > Char.ToUpper/ToLower should throw an ArgumentNullException if the
> > > CultureInfo argument is null.  Attached is a patch to do just that.
My
> > > only question is if it would have been better to use
> > > MONO_CHECK_ARG_NULL from object.h.  I wasn't sure if that would abort
> > > the current function or not.  If if MONO_CHECK_ARG_NULL would be
> > > preferred, I can resubmit.  Either way I would need someone to commit
> > > this for me as I don't have CVS access.
> > >
> > > Thanks.
> > >
> > > Rob
> > >
>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: PatchStringTo.txt
Url: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20040424/e2cf6d0b/attachment.txt