[Mono-list] RE: [Mono-devel-list] REGRESSION: StringBuilder

[Paolo Molaro]

On 01/14/04 Iain McCoy wrote:
> I get the same problem, on a freshly checked-out-from-anoncvs-and-built
> mcs and mono. I did some digging (just because I could) and I suspect
> the problem is that StringBuilder uses String.InternalStrcpy to move
> everything to the right of the insert point across. From my reading of
> the code, that would go to the method
> ves_icall_System_String_InternalStrcpy_StrN in
> mono/metadata/string-icalls.c. That function's basically a call to
> memcpy, which can't be used on overlapping memory areas (and breaking
> that rule is what causes the regression). I think there's a few options
> to fix it:
> 1 Add a String.InternalStrmov that uses memmove instead of memcpy
> 2 Change String.InternalStrcpy so that it uses memmove
> 
> Both of these options fix the problem. I suspect Option 2 is detrimental
> to performance because it would mean that overlapping buffers were being
> dealt with even when they weren't actually there.

I committed a change memcpy -> g_memmove, mostly because it's the
smaller change.

> I'm attaching a little patch I wrote that does Option 1. I make no
> claims about this patch, except that it works for me. I didn't look over
> StringBuilder particularly thoroughly, but I think I caught all of the
> cases that needed to be changed. 
> 
> I imagine I have broken 5 rules and 10 guidelines with this patch, but
> oh well - I hope it is useful.

The patch looks good:-)
Thanks for tracking down the issue.
If we get some real data that shows the memmove to be significantly
slower than the memcpy, we'll go for the added complexity of having the
additional icalls.

Thanks!
lupus

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