[Mono-devel-list] String.Replace patch to Managed code.

[grompf]

Andreas,

   Admittedly and fully understood that what I initially presented 
wasn't optimized.  I was working on tackling the bug first (with "safe" 
managed code), before moving it to the optimized unsafe version (which 
you posted for Replace(char, char) and looks pretty good.

   Now, that being said, considering your latest patch with IndexOf 
improvments.  Your second "optimized" patch, wouldn't it be faster to 
do:

int pos = this.IndexOf(oldChar);
if(pos == -1)
	return this;

fixed (char *source = &start_char) {
   string tmp = InternalAllocateStr(length);
   CharCopy(source, dest, pos);
   for(int i = 0; i < length; i++) {
     if(source[x] == oldChar)
	dest[x] = newChar;
     else
	dest[x] = source[x];
   }
   return tmp;
}


??

-kangaroo

On 31-May-04, at 7:06 PM, Andreas Nahr wrote:

> Hi,
>  
> I'm working on string for quite some time now, but only when I find 
> spare time to do so, so things are progressing relatively slow in that 
> field (especially as things are very performance sensitive in there 
> and need a *lot* of testing)
>  
> And I think that also a slight problem with your patch. Just from 
> looking at it (without too much testing): The performance of it would 
> be really bad.
> For the invariant version a relatively fast managed implementation is 
> relatively easy (unfortunatelly it is still a little bit slower than 
> native, but a lot faster than your solution):
>  
>    string tmp = InternalAllocateStr (length);
>    fixed (char* s = &start_char, d = tmp) {
>     char* source = s, dest = d;
>     for (int x = 0; x < length; x++) {
>      if (*source == oldChar)
>       *dest = newChar;
>      else
>       *dest = *source;
>      source++;
>      dest++;
>     }
>    }
>    return tmp;
>  
>  
>  
> If you like to test things a little bit you could also look at this 
> optimized version (needs the CharCopy patch I submitted to this list):
>  
>    fixed (char* source = &start_char) {
>     for (int x = 0; x < length; x++) {
>      if (source[x] == oldChar) {
>       string tmp = InternalAllocateStr (length);
>       fixed (char* dest = tmp) {
>        CharCopy (source, dest, x);
>        source[x] = newChar;
>        for (; x < length; x++) {
>         if (source[x] == oldChar)
>          dest[x] = newChar;
>         else
>          dest[x] = source[x];
>        }
>       }
>       return tmp;
>      }
>     }
>    }
>    return this;
>  
>  
>  
> Andreas
>  
> ----- Original Message -----
>  From: grompf
> To: mono-devel-list at lists.ximian.com
> Sent: Monday, May 31, 2004 9:03 PM
> Subject: [Mono-devel-list] String.Replace patch to Managed code.
>
> In my effort to address bug #59274, I tracked the problem down to 
> icu/glib in locales.c not replacing \0.
>
> Attached is a patch for String.cs to replace the internal methods with 
> managed code.
>
> There is probably a more efficient way of Replace(String, String), but 
> both of these methods have been tested and working for bug #59274 as 
> well as other Replace testings. However, I'm a little unsure how to 
> test the culture dependancy of Replace(String, String). Considering 
> the IndexOf call should determine the culture as well, it _should_ be 
> ok as far I understand the culture dependancy stuff.
>
> If this looks good, let me know and I'll move on to some other methods 
> (like IndexOf).
>
> -kangaroo
>
>
>  
>  !DSPAM:40bbba5f248971207617767!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: text/enriched
Size: 7361 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20040601/fa8a9815/attachment.bin