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

[Andreas Nahr]

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:40bbba5f248971207617767!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20040601/03ac3d08/attachment.html