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

[grompf]

Err.. sorry that should be

  /* This method is culture insensitive */
                 public unsafe String Replace (char oldChar, char 
newChar)
                 {
                         // Change Replace(char, char) to managed code, 
because ICU doesn't deal with replacing \0
                         int pos = this.IndexOf(oldChar);
                         if(pos == -1)
                                 return this;
                         string tmp = InternalAllocateStr(length);
                         fixed(char* s = &start_char, d = tmp) {
                                 CharCopy(s, d, pos);
                                 for(int i = pos; i < length ; i++)
                                 {
                                         if(s[i] == oldChar)
                                                 d[i] = newChar;
                                         else
                                                 d[i] = s[i];
                                 }
                         }
                         return tmp;
                 }

-kangaroo

On 1-Jun-04, at 1:26 PM, grompf wrote:

> 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: 8500 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20040601/ac57b814/attachment.bin