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

[grompf]

Andreas,

   If you look carefully, I use pos later in the CharCopy, so "saving" 
that variable, would cause two calls to IndexOf.

-kangaroo

On 1-Jun-04, at 5:36 PM, Andreas Nahr wrote:

> If I would have to guess (without testing!) this should be faster than 
> my first (simple) solution and slower that the second one, however the 
> code looks nicer and is more readable ;)
>  
> You could also write:
> if(this.IndexOf(oldChar); == -1)
>     return this;
> to save a variable.
> However the additional function call is going to suck away a good 
> amount of speed.
>  
> Andreas
> ----- Original Message -----
>  From: grompf
> To: Andreas Nahr
> Cc: mono-devel-list at lists.ximian.commono-devel-list@lists.ximian.com
> Sent: Tuesday, June 01, 2004 7:33 PM
> Subject: Re: [Mono-devel-list] String.Replace patch to Managed code.
>
> 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:40bcf677105873176746000!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: text/enriched
Size: 6216 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20040601/4132b3f3/attachment.bin