StringReplace extremely slow with big strings lazarus / 64-FPC development ---------------------------------------------------------------------- jarto · 2014-10-14 19:24 I noticed this programming with Delphi 2007, but FPC does suffer from the same problem: StringReplace is awful with long strings. I ran into this in real world while wondering how Indy's headers could be so slow with big POST buffers. It converted '+' to ' ' with StringReplace. When I looked at the code, the reason was obvious:  StringReplace does a lot of string manipulation, copy and concatenation. It gets really ugly fast, when there's a lot to replace and the string is big. I wrote a new version, which is a lot more optimized. It can certainly be optimized more, but the general idea is to calculate the length of the result string, set the length once and only manipulate the contents of it. Here are some test results: 58058 bytes Single char, replace all 203 x faster 58058 bytes Same length, replace all 61 x faster 58058 bytes Single char, replace all, ignore case 68 x faster 58058 bytes Same length, replace all, ignore case 34 x faster 58058 bytes New longer, replace all 32 x faster 58058 bytes New shorter, replace all 32 x faster 58058 bytes New longer, replace all, ignore case 22 x faster 58058 bytes New shorter, replace all, ignore case 22 x faster 58058 bytes New empty, replace all 35 x faster 58058 bytes New empty, replace all, ignore case 23 x faster 116058 bytes Single char, replace all 260 x faster 116058 bytes Same length, replace all 148 x faster 116058 bytes Single char, replace all, ignore case 153 x faster 116058 bytes Same length, replace all, ignore case 75 x faster 116058 bytes New longer, replace all 62 x faster 116058 bytes New shorter, replace all 61 x faster 116058 bytes New longer, replace all, ignore case 45 x faster 116058 bytes New shorter, replace all, ignore case 44 x faster 116058 bytes New empty, replace all 68 x faster 116058 bytes New empty, replace all, ignore case 50 x faster 174058 bytes Single char, replace all 518 x faster 174058 bytes Same length, replace all 228 x faster 174058 bytes Single char, replace all, ignore case 219 x faster 174058 bytes Same length, replace all, ignore case 113 x faster 174058 bytes New longer, replace all 100 x faster 174058 bytes New shorter, replace all 104 x faster 174058 bytes New longer, replace all, ignore case 70 x faster 174058 bytes New shorter, replace all, ignore case 72 x faster 174058 bytes New empty, replace all 107 x faster 174058 bytes New empty, replace all, ignore case 80 x faster 232058 bytes Single char, replace all 634 x faster 232058 bytes Same length, replace all 291 x faster 232058 bytes Single char, replace all, ignore case 305 x faster 232058 bytes Same length, replace all, ignore case 152 x faster 232058 bytes New longer, replace all 141 x faster 232058 bytes New shorter, replace all 140 x faster 232058 bytes New longer, replace all, ignore case 98 x faster 232058 bytes New shorter, replace all, ignore case 98 x faster 232058 bytes New empty, replace all 153 x faster 232058 bytes New empty, replace all, ignore case 107 x faster 290058 bytes Single char, replace all 1043 x faster 290058 bytes Same length, replace all 487 x faster 290058 bytes Single char, replace all, ignore case 542 x faster 290058 bytes Same length, replace all, ignore case 311 x faster 290058 bytes New longer, replace all 254 x faster 290058 bytes New shorter, replace all 249 x faster 290058 bytes New longer, replace all, ignore case 183 x faster 290058 bytes New shorter, replace all, ignore case 179 x faster 290058 bytes New empty, replace all 277 x faster 290058 bytes New empty, replace all, ignore case 197 x faster And the new code is: (Update: This original code has a bug in it, look below for a fixed and improved version) Code: [Select]Function NewStringReplace(const S, OldPattern, NewPattern: string;  Flags: TReplaceFlags): string; var   OldPat,Srch: string; // Srch and Oldp can contain uppercase versions of S,OldPattern   PatLength,NewPatLength,P,i,PatCount,PrevP: Integer;   c,d: pchar; begin   PatLength:=Length(OldPattern);   if PatLength=0 then begin     Result:=S;     exit;   end;   if rfIgnoreCase in Flags then begin     Srch:=AnsiUpperCase(S);     OldPat:=AnsiUpperCase(OldPattern);   end else begin     Srch:=S;     OldPat:=OldPattern;   end;   PatLength:=Length(OldPat);   if Length(NewPattern)=PatLength then begin     //Result length will not change     Result:=S;     P:=1;     repeat       P:=PosEx(OldPat,Srch,P);       if P>0 then begin         for i:=1 to PatLength do           Result[P+i-1]:=NewPattern[i];         if not (rfReplaceAll in Flags) then exit;         inc(P,PatLength);       end;     until p=0;   end else begin     //Different pattern length -> Result length will change     //To avoid creating a lot of temporary strings, we count how many     //replacements we're going to make.     P:=1; PatCount:=0;     repeat       P:=PosEx(OldPat,Srch,P);       if P>0 then begin         inc(P,PatLength);         inc(PatCount);         if not (rfReplaceAll in Flags) then break;       end;     until p=0;     if PatCount=0 then begin       Result:=S;       exit;     end;     NewPatLength:=Length(NewPattern);     SetLength(Result,Length(S)+PatCount*(NewPatLength-PatLength));     P:=1; PrevP:=0;     c:=pchar(Result); d:=pchar(S);     repeat       P:=PosEx(OldPat,Srch,P);       if P>0 then begin         for i:=PrevP+1 to P-1 do begin           c^:=d^;           inc(c); inc(d);         end;         for i:=1 to NewPatLength do begin           c^:=NewPattern[i];           inc(c);         end;         if not (rfReplaceAll in Flags) then exit;         inc(P,PatLength);         inc(d,PatLength);         PrevP:=P-1;       end else begin         for i:=PrevP+1 to Length(S) do begin           c^:=d^;           inc(c); inc(d);         end;       end;     until p=0;   end; end; Comments are welcome. It'd be nice to get this one or an even more improved one to the FPC source tree. ---------------------------------------------------------------------- Leledumbo · 2014-10-14 21:15 QuoteIt'd be nice to get this one or an even more improved one to the FPC source tree.Post it as a patch to the bugtracker. I'm not sure whether we have unit testing for rtl units, someone else might be able to confirm this and you can run your code against the unit tests. If it generates the same result as the original one, your code is likely to be accepted. ---------------------------------------------------------------------- circular · 2014-10-14 22:14 Hi, I would propose to change this: Code: [Select]        for i:=1 to PatLength do           Result[P+i-1]:=NewPattern[i];Into Code: [Select]        move(NewPattern[1], Result[P], PatLength); And Code: [Select]        for i:=1 to NewPatLength do begin           c^:=NewPattern[i];           inc(c);         end;Into Code: [Select]        Move(NewPattern[1],c^,NewPatLength);         inc(c, NewPatLength); Also I think that Code: [Select]if not (rfReplaceAll in Flags) then exit;Will not copy the end of the string Also the copying of the end of the string can also be done with Move ---------------------------------------------------------------------- jarto · 2014-10-15 02:56 circular, thank you for your suggestions and for noticing how the "replace one" -case didn't copy until the end of the string. Here's the updated code. Before submitting to the bugtracker, I wonder if it's ok that the function uses posex? Code: [Select]Function NewStringReplace(const S, OldPattern, NewPattern: string;  Flags: TReplaceFlags): string; var   OldPat,Srch: string; // Srch and Oldp can contain uppercase versions of S,OldPattern   PatLength,NewPatLength,P,Cnt,PatCount,PrevP: Integer;   c,d: pchar; begin   PatLength:=Length(OldPattern);   if PatLength=0 then begin     Result:=S;     exit;   end;   if rfIgnoreCase in Flags then begin     Srch:=AnsiUpperCase(S);     OldPat:=AnsiUpperCase(OldPattern);   end else begin     Srch:=S;     OldPat:=OldPattern;   end;   PatLength:=Length(OldPat);   if Length(NewPattern)=PatLength then begin     //Result length will not change     Result:=S;     P:=1;     repeat       P:=PosEx(OldPat,Srch,P);       if P>0 then begin         move(NewPattern[1],Result[P],PatLength);         if not (rfReplaceAll in Flags) then exit;         inc(P,PatLength);       end;     until p=0;   end else begin     //Different pattern length -> Result length will change     //To avoid creating a lot of temporary strings, we count how many     //replacements we're going to make.     P:=1; PatCount:=0;     repeat       P:=PosEx(OldPat,Srch,P);       if P>0 then begin         inc(P,PatLength);         inc(PatCount);         if not (rfReplaceAll in Flags) then break;       end;     until p=0;     if PatCount=0 then begin       Result:=S;       exit;     end;     NewPatLength:=Length(NewPattern);     SetLength(Result,Length(S)+PatCount*(NewPatLength-PatLength));     P:=1; PrevP:=0;     c:=pchar(Result); d:=pchar(S);     repeat       P:=PosEx(OldPat,Srch,P);       if P>0 then begin         Cnt:=P-PrevP-1;         if Cnt>0 then begin           Move(d^,c^,Cnt);           inc(c,Cnt);           inc(d,Cnt);         end;         if NewPatLength>0 then begin           Move(NewPattern[1],c^,NewPatLength);           inc(c,NewPatLength);         end;         inc(P,PatLength);         inc(d,PatLength);         PrevP:=P-1;         if not (rfReplaceAll in Flags) then break;       end;     until p=0;     Cnt:=Length(S)-PrevP;     if Cnt>0 then Move(d^,c^,Cnt);   end; end; ---------------------------------------------------------------------- minesadorada · 2014-10-15 06:30 @jarto: Is there also a UTF version (NewStringReplaceUTF8) and ANSI version (NewAnsiStringReplace) in the pipeline?  They could replace the ones in the fileutil unit. ---------------------------------------------------------------------- BigChimp · 2014-10-15 06:44 Quote from: jarto on October 15, 2014, 04:56:42 amBefore submitting to the bugtracker, I wonder if it's ok that the function uses posex? If the unit posex is in is already referenced by/present in the unit stringreplace is in (sorry to lazy to look it up now) I don't see a problem? ---------------------------------------------------------------------- jarto · 2014-10-15 07:17 Quote from: minesadorada on October 15, 2014, 08:30:18 am@jarto: Is there also a UTF version (NewStringReplaceUTF8) and ANSI version (NewAnsiStringReplace) in the pipeline?  They could replace the ones in the fileutil unit. I'm not familiar enough with what's exactly needed, but basically that one works with any data. It can be UTF8, binary data, whatever. Unless you call it with rfIgnoreCase, of course. ---------------------------------------------------------------------- BigChimp · 2014-10-15 07:35 Quote from: jarto on October 15, 2014, 09:17:56 amI'm not familiar enough with what's exactly needed, but basically that one works with any data. It can be UTF8, binary data, whatever. I suspect not. UTF8 e.g. often has 1 byte per character but can have multiples. That needs to be taken into account. However, I'd focus first on the current version you have and see what the fpc devs have to say when you submit your patch... ---------------------------------------------------------------------- BigChimp · 2014-10-15 11:59 BTW see http://bugs.freepascal.org/view.php?id=22501 where alternative stringreplace functions in conjunction with FPC trunk are discussed. ---------------------------------------------------------------------- circular · 2014-10-15 15:16 Quote from: BigChimp on October 15, 2014, 09:35:16 amQuote from: jarto on October 15, 2014, 09:17:56 amI'm not familiar enough with what's exactly needed, but basically that one works with any data. It can be UTF8, binary data, whatever. I suspect not. UTF8 e.g. often has 1 byte per character but can have multiples. That needs to be taken into account.In my view, if it is a case sensitive search, it works with UTF8 too, because each UTF8 character start with a byte that indicate its length and the next bytes are always in a certain range. So when providing an UTF8 search string and an UTF8 pattern, it is not possible to identify a pattern without matching the beginning and ending of characters. However, this makes a difference when the search is not case sensitive. The resulting string will certainly be corrupted if the search pattern or search string contains non ASCII characters when the AnsiUpperCase is applied on an UTF8 string. Also it will not work for sure with widestrings. ---------------------------------------------------------------------- minesadorada · 2014-10-15 15:33 In StrUtils, AnsiReplaceText just calls StringReplace with the flagset [rfReplaceAll,rfIgnoreCase] =edit: oops! I meant StrUtils== ---------------------------------------------------------------------- jarto · 2014-10-15 15:54 Well, it's submitted. Let's see if it's accepted. http://bugs.freepascal.org/view.php?id=26864 ---------------------------------------------------------------------- User137 · 2014-10-15 19:57 Would it be faster if you would save the search results in TList (faster than dynamic array for this case) from first pass? Just in theory, it could be more optimal. You would need to store the PosEx result, an integer. ---------------------------------------------------------------------- jarto · 2014-10-16 15:57 I'm not sure that StringReplace should be more complicated to the point of using a TList to store values. It would probably cause it to be slower with short strings while speeding up faster ones. ---------------------------------------------------------------------- skalogryz · 2014-10-16 16:53 you need more efficient substring search than PosEx(). Check this out. Code: [Select]uses Classes, SysUtils, StrUtils, dateutils; ...function NewStringReplace... var   t: TDateTime;   src : string;   i   : integer;   tmp : string;   l   : string; begin   tmp:='aaaaaaaab';   l:=Copy(tmp,1,length(tmp)-1);   src:='';   for i:=0 to 500000 do src:=src+l;   writeln('prepared!');   src:=src+tmp;    t:=now;    StringReplace(src,tmp,'*',[rfReplaceAll]);    writeln('rtl: ',MilliSecondsBetween(now,t));    t:=now;    NewStringReplace(src,tmp,'*',[rfReplaceAll]);    writeln('new: ', MilliSecondsBetween(now,t)); end. ---------------------------------------------------------------------- jarto · 2014-10-16 17:49 Quote from: skalogryz on October 16, 2014, 06:53:57 pmyou need more efficient substring search than PosEx(). This is too funny. PosEx() uses indexbyte to find the first byte. It's even assembler code. Pos() uses a simple while loop and is faster :-D ---------------------------------------------------------------------- CM630 · 2018-01-22 11:36 Hi, any idea when will these fixes will be applied in an official Lazarus release? ---------------------------------------------------------------------- ASerge · 2018-01-22 15:03 Quote from: BigChimp on October 15, 2014, 08:44:46 amIf the unit posex is in is already referenced by/present in the unit stringreplace is in (sorry to lazy to look it up now) I don't see a problem? New StringReplace use PosEx, which in StrUtils, which uses SysUtils, which contain StringReplace. Of course, the cyclic dependency can be broken through the implementation section, but I do not like this decision. ---------------------------------------------------------------------- marcov · 2018-01-22 15:06 Afaik in some recent version POS() has gotten an extra parameter and is now posex like. Might change the cycle. IIRC it is even merged to 3.0.4, but am not 100% sure. ---------------------------------------------------------------------- RAW · 2018-01-22 18:23 Maybe this works for you.... ---------------------------------------------------------------------- CM630 · 2018-01-23 07:08 Thanks, I looked at the code... since it uses Posex, I have a bad feeling it won't works with UTF8... ? Until now I used this code from Lazarus bugzilla: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i //ANSIFunction StringReplaceNew(const S, OldPattern, NewPattern: string;  Flags: TReplaceFlags): string;var  OldPat,Srch: string; // Srch and Oldp can contain uppercase versions of S,OldPattern  PatLength,NewPatLength,P,Cnt,PatCount,PrevP: Integer;  c,d: pchar;begin  PatLength:=Length(OldPattern);  if PatLength=0 then begin    Result:=S;    exit;  end;    if rfIgnoreCase in Flags then begin    Srch:=AnsiUpperCase(S);    OldPat:=AnsiUpperCase(OldPattern);  end else begin    Srch:=S;    OldPat:=OldPattern;  end;    PatLength:=Length(OldPat);  if Length(NewPattern)=PatLength then begin    //Result length will not change    Result:=S;    P:=1;    repeat      P:=PosEx(OldPat,Srch,P);      if P>0 then begin        move(NewPattern[1],Result[P],PatLength);        if not (rfReplaceAll in Flags) then exit;        inc(P,PatLength);      end;    until p=0;  end else begin    //Different pattern length -> Result length will change    //To avoid creating a lot of temporary strings, we count how many    //replacements we're going to make.    P:=1; PatCount:=0;    repeat    P:=PosEx(OldPat,Srch,P);      if P>0 then begin        inc(P,PatLength);        inc(PatCount);        if not (rfReplaceAll in Flags) then break;      end;    until p=0;    if PatCount=0 then begin      Result:=S;      exit;    end;    NewPatLength:=Length(NewPattern);    SetLength(Result,Length(S)+PatCount*(NewPatLength-PatLength));    P:=1; PrevP:=0;    c:=pchar(Result); d:=pchar(S);    repeat      P:=PosEx(OldPat,Srch,P);      if P>0 then begin        Cnt:=P-PrevP-1;        if Cnt>0 then begin          Move(d^,c^,Cnt);        inc(c,Cnt);          inc(d,Cnt);        end;        if NewPatLength>0 then begin          Move(NewPattern[1],c^,NewPatLength);          inc(c,NewPatLength);      end;        inc(P,PatLength);        inc(d,PatLength);        PrevP:=P-1;        if not (rfReplaceAll in Flags) then break;      end;    until p=0;    Cnt:=Length(S)-PrevP;    if Cnt>0 then Move(d^,c^,Cnt);  end;end;     but it makes mess with UTF8 strings (even though I am replacing ansi strings with ansi strings). So I tried to modify it to: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i //UTF8Function StringReplaceNew(const S, OldPattern, NewPattern: string;  Flags: TReplaceFlags): string;var  OldPat,Srch: string; // Srch and Oldp can contain uppercase versions of S,OldPattern  PatLength,NewPatLength,P,Cnt,PatCount,PrevP: Integer;  c,d: pchar;begin  PatLength:=utf8Length(OldPattern);  if PatLength=0 then begin    Result:=S;    exit;  end;    if rfIgnoreCase in Flags then begin    Srch:=UTF8UpperCase(S);    OldPat:=UTF8UpperCase(OldPattern);  end else begin    Srch:=S;    OldPat:=OldPattern;  end;    PatLength:=utf8Length(OldPat);  if utf8Length(NewPattern)=PatLength then begin    //Result length will not change    Result:=S;    P:=1;    repeat      P:=utf8Pos(OldPat,Srch,P);      if P>0 then begin        move(NewPattern[1],Result[P],PatLength);        if not (rfReplaceAll in Flags) then exit;        inc(P,PatLength);      end;    until p=0;  end else begin    //Different pattern length -> Result length will change    //To avoid creating a lot of temporary strings, we count how many    //replacements we're going to make.    P:=1; PatCount:=0;    repeat//      P:=utf8Pos(OldPat,Srch,P);   //This counts only the occurences of the sougth string, so there is possibly no reason to use UFT8Pos, whish is extremely slow      P:=Posex(OldPat,Srch,P);      if P>0 then begin        inc(P,PatLength);        inc(PatCount);        if not (rfReplaceAll in Flags) then break;      end;    until p=0;    if PatCount=0 then begin      Result:=S;      exit;    end;    NewPatLength:=utf8Length(NewPattern);    SetLength(Result,utf8Length(S)+PatCount*(NewPatLength-PatLength));    P:=1; PrevP:=0;    c:=pchar(Result); d:=pchar(S);    repeat      P:=utf8Pos(OldPat,Srch,P);      if P>0 then begin        Cnt:=P-PrevP-1;        if Cnt>0 then begin          Move(d^,c^,Cnt);        inc(c,Cnt);          inc(d,Cnt);        end;        if NewPatLength>0 then begin          Move(NewPattern[1],c^,NewPatLength);          inc(c,NewPatLength);      end;        inc(P,PatLength);        inc(d,PatLength);        PrevP:=P-1;        if not (rfReplaceAll in Flags) then break;      end;    until p=0;    Cnt:=utf8Length(S)-PrevP;    if Cnt>0 then Move(d^,c^,Cnt);  end;end;I have no idea if it works properly, but is extremely slow.    It seems to me that utf8Pos is the bottle neck. A very narrow one. ---------------------------------------------------------------------- Thaddy · 2018-01-23 08:10 If you replace ansi with ansi, plz do NOT use string in Lazarus: that is UTF8. Rename string to AnsiString. ---------------------------------------------------------------------- CM630 · 2018-01-23 09:12 @Thaddy, I am not sure that I understood you right and that you understood me. I am replacing Ansi with Ansi in an UTF8 string. In other words: Strings which I am replacing are with ascii code <127 (basic Latin, &%$# etc). But the sting which contains them may also contain uтf8 characters. ---------------------------------------------------------------------- JuhaManninen · 2018-01-23 11:01 Quote from: Thaddy on January 23, 2018, 09:10:24 amIf you replace ansi with ansi, plz do NOT use string in Lazarus: that is UTF8. Rename string to AnsiString. Thaddy, in Lazarus by default String = AnsiString. You should however use type String always because the code is then compatible with Delphi and with future mode DelphiUnicode solutions. Remember, Ansi...() functions like AnsiUpperCase() work transparently with different encodings then. If someone still uses the old Windows codepages, then please switch to Unicode. CM630, please see the example:  http://wiki.lazarus.freepascal.org/UTF8_strings_and_characters#Search _and_copy Pos() and PosEx() work in StringReplaceNew for the same reasons they work in the given example. CodeUnit (Pascal Char) resolution is still very useful with variable width encodings, and is fast. ---------------------------------------------------------------------- CM630 · 2018-01-23 11:58 1. What I got from the example is that I should keep PosEx and Length in StringReplaceNew. 2. CodeUnit is some type of variable. But I could no find more info about it. 3. My current (old) implementation of StringReplaceNew messes the strings up. 4. I will try RAW's solution and feedback.     Edit: I tried it. My app crashes with a range check error. ---------------------------------------------------------------------- JuhaManninen · 2018-01-23 12:11 Quote from: CM630 on January 23, 2018, 12:58:59 pm2. CodeUnit is some type of variable. But I could no find more info about it. No. It is a concept of Unicode. It is the smallest atomic building block. A code point can consist of one or more code units. In Pascal terms it is type Char, a byte with UTF-8 and a word with UTF-16. See:  https://en.wikipedia.org/wiki/Character_encoding#Terminology and search for "code unit". ---------------------------------------------------------------------- engkin · 2018-01-24 15:41 Quote from: CM630 on January 23, 2018, 12:58:59 pm3. My current (old) implementation of StringReplaceNew messes the strings up. You did not show the current implementation. I assume something like: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i Function StringReplaceNew(const S, OldPattern, NewPattern: string;  Flags: TReplaceFlags): string;var  OldPat,Srch: string; // Srch and Oldp can contain uppercase versions of S,OldPattern  PatLength,NewPatLength,P,Cnt,PatCount,PrevP: Integer;  c,d: pchar;begin  PatLength:=Length(OldPattern);  if PatLength=0 then    exit(S);   if rfIgnoreCase in Flags then begin    Srch:=UTF8UpperCase(S);    OldPat:=UTF8UpperCase(OldPattern);  end else begin    Srch:=S;    OldPat:=OldPattern;  end;   PatLength:=Length(OldPat);  if Length(NewPattern)=PatLength then begin    //Result length will not change    Result:=S;    P:=1;    repeat      P:=PosEx(OldPat,Srch,P);      if P>0 then begin        move(NewPattern[1],Result[P],PatLength);        if not (rfReplaceAll in Flags) then exit;        inc(P,PatLength);      end;    until p=0;  end else begin    //Different pattern length -> Result length will change    //To avoid creating a lot of temporary strings, we count how many    //replacements we're going to make.    P:=1; PatCount:=0;    repeat      P:=PosEx(OldPat,Srch,P);      if P>0 then begin        inc(P,PatLength);        inc(PatCount);        if not (rfReplaceAll in Flags) then break;      end;    until p=0;    if PatCount=0 then begin      Result:=S;      exit;    end;    NewPatLength := Length(NewPattern);    SetLength(Result, Length(S) + PatCount*(NewPatLength-PatLength));    P:=1; PrevP:=0;    c:=pchar(Result);    d:=pchar(S);    repeat      P:=PosEx(OldPat,Srch,P);      if P>0 then begin        Cnt:=P-PrevP-1;        if Cnt>0 then begin          Move(d^,c^,Cnt);        inc(c,Cnt);          inc(d,Cnt);        end;         Move(NewPattern[1],c^,NewPatLength);        inc(c,NewPatLength);      inc(P,PatLength);        inc(d,PatLength);        PrevP:=P-1;        if not (rfReplaceAll in Flags) then break;      end;    until p=0;    Cnt:=Length(S)-PrevP;    Move(d^,c^,Cnt);  end;end; I believe you have a problem with rfIgnoreCase. Your code assumes that Srch and S have the same length. Upper case and lower case do *not* necessarily have the same length. Check the source code for UTF8UpperCase: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i       if IsTurkish and (AInStr[InCounter] = 'i') then      begin        SetLength(Result,Length(Result)+1);// Increase the buffer...        inc(InCounter);        inc(OutCounter,2);and other calls to CorrectOutStrSize. You'll have to build the index for both strings and shift the location based on it. Edit: UTF8StringReplace from unit LazUTF8 has the same bug. It uses UTF8LowerCase instead. Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i   { To test the bug with UTF8StringReplace }  { LATIN CAPITAL LETTER A WITH STROKE' (U+23A) Ⱥ}  SetLength(sC8BA,2);  sC8BA[1] := #$C8;  sC8BA[2] := #$BA; //or #$BE   { To test the bug with NewStringReplace }  { LATIN SMALL LETTER TURNED ALPHA' (U+0252) ɒ}  SetLength(sC992,2);  sC992[1] := #$C9;  sC992[2] := #$92; ---------------------------------------------------------------------- JuhaManninen · 2018-01-25 21:45 Quote from: engkin on January 24, 2018, 04:41:56 pmI believe you have a problem with rfIgnoreCase. Your code assumes that Srch and S have the same length. Upper case and lower case do *not* necessarily have the same length. True. It also means functions UTF8SwapCase and my recently added UTF8ProperCase in unit LazUTF8 are buggy. On the positive side the lengths are the same in most cases but that is no excuse for a bug of course. QuoteUTF8StringReplace from unit LazUTF8 has the same bug. It uses UTF8LowerCase instead. Ok, I didn't realize that one. The function has been there forever. If somebody wants to fix those bugs, please do. I can commit the fixes. ---------------------------------------------------------------------- CM630 · 2018-01-26 07:44 Quote from: engkin on January 24, 2018, 04:41:56 pmYou did not show the current implementation. I assume something like: I did in post 20 (first function). Or you mean sth. else? Quote from: engkin on January 24, 2018, 04:41:56 pm I believe you have a problem with rfIgnoreCase. Your code assumes that Srch and S have the same length. Upper case and lower case do *not* necessarily have the same length. Check the source code for UTF8UpperCase: I think your belief is right! I removed all rfIgnoreCase and now I do not notice any corruption of the string (but I cannot say for sure that there is not any). Quote from: JuhaManninen on January 25, 2018, 10:45:22 pmOn the positive side the lengths are the same in most cases but that is no excuse for a bug of course. On the negative side StringReplace will never work for Turkish and some other languages. ---------------------------------------------------------------------- engkin · 2018-01-27 04:20 Here is a possible replacement for UTF8StringReplace. It needs to be tested. Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i function UTF8StringReplace_1(const S, OldPattern, NewPattern: string;  Flags: TReplaceFlags): string;var  Matches: array of integer;  OldPat,Srch: string; // Srch and Oldp can contain uppercase versions of S,OldPattern  PatLength,P,Count, Capacity, i: Integer;  p1,p2, MemPos: pchar;  i1,i2, l: integer;begin  if (Length(OldPattern)=0) or (Length(S)=0) then    exit(S);   if rfIgnoreCase in Flags then begin  Srch := UTF8LowerCase(S);    OldPat := UTF8LowerCase(OldPattern);  end else begin    Srch := S;    OldPat := OldPattern;  end;   { Find matches }  Count := 0;  Capacity := 10;  SetLength(Matches, Capacity);  P:=PosEx(OldPat, Srch, 1);  if P=0 then exit(s);  if rfReplaceAll in Flags then  { TODO : Consider using FindMatchesBoyerMooreCaseSensitive }    while p<>0 do begin      Matches[Count] := p;      inc(Count);      if Count=Capacity then begin { Grow }        inc(Capacity, 10);        SetLength(Matches, Capacity);      end;      P:=PosEx(OldPat,Srch,P+1);    end  else begin      Matches[Count] := p;      inc(Count);  end;   if rfIgnoreCase in Flags then begin { Correct match positions }    p1 := @Srch[1]; p2 := @S[1];    for i := 0 to Count-1 do begin      MemPos := @Srch[Matches[i]];      while p1-1 then     WriteLn('Invalid UTF8 string'); Edit: If the text is valid, does the problem happen with and without rfIgnoreCase? ---------------------------------------------------------------------- CM630 · 2018-01-29 07:20 I did: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i function StringReplaceNew(const S, OldPattern, NewPattern: string;  Flags: TReplaceFlags): string;var  Matches: array of integer;  OldPat,Srch: string; // Srch and Oldp can contain uppercase versions of S,OldPattern  PatLength,P,Count, Capacity, i: Integer;  p1,p2, MemPos: pchar;  i1,i2, l: integer;  LocationOfBadCode: integer;begin  LocationOfBadCode := FindInvalidUTF8Character(@S[1], Length(S), True);  If LocationOfBadCode<>-1 then  begin    writeln('Invalid UTF8 string' + LineEnding       + OldPattern + LineEnding +  NewPattern);  end;  ...and I have put a breakpoint on  writeln... Everything seems to be valid. Removing   rfIgnoreCase did not change behaviour. EDIT: I have insulated a troublesome string and executed it in a standalone app. Then replacements are done just fine!??!? ---------------------------------------------------------------------- engkin · 2018-01-29 18:45 Quote from: CM630 on January 29, 2018, 08:20:52 amEDIT: I have insulated a troublesome string and executed it in a standalone app. Then replacements are done just fine!??!?[/font] YES! You have range check activated in the main project, but not in the standalone app. That explains the exceptions with and without rfIgnoreCase. Either way, I fell in the same bug that I was fixing. I assumed that all patterns have equal lengths, but they are not necessarily equal. This is another version that should correct these problems: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i function UTF8StringReplace(const S, OldPattern, NewPattern: string;  Flags: TReplaceFlags): string;type  TMatch=Record    Pos,Length: integer;  end; var  Matches: array of TMatch;  OldPat,Srch: string; // Srch and Oldp can contain uppercase versions of S,OldPattern  PatLength,P,Count, Capacity, i: Integer;  p1,p2, MemPos, p3: pchar;  i1,i2, l, j: integer;  OldPatLengths: integer;begin  if (Length(OldPattern)=0) or (Length(S)=0) then    exit(S);   if rfIgnoreCase in Flags then begin    Srch := UTF8LowerCase(S);    OldPat := UTF8LowerCase(OldPattern);  end else begin    Srch := S;    OldPat := OldPattern;  end;   { Find matches }  Count := 0;  Capacity := 10;  SetLength(Matches, Capacity);  P:=PosEx(OldPat, Srch, 1);  if P=0 then exit(s);  if rfReplaceAll in Flags then  { TODO : Consider using FindMatchesBoyerMooreCaseSensitive }    while p<>0 do begin    Matches[Count].pos := p;      Matches[Count].Length := Length(OldPat);      inc(Count);      if Count=Capacity then begin { Grow }        inc(Capacity, 10);        SetLength(Matches, Capacity);      end;      P:=PosEx(OldPat,Srch,P+1);    end  else begin      Matches[Count].pos := p;      Matches[Count].Length := Length(OldPat);      inc(Count);  end;   if rfIgnoreCase in Flags then begin { Correct match positions and lengths }    OldPatLengths := 0;  p1 := @Srch[1]; p2 := @S[1];    for i := 0 to Count-1 do begin      MemPos := @Srch[Matches[i].pos];      while p10 then        move(S[i1], Result[i2], l); { Copy text before pattern }      inc(i2, l); { Move to the location of the pattern }      i1 := Matches[i].pos+Matches[i].Length; { Move over the old pattern }       if Length(NewPattern)>0 then        move(NewPattern[1], Result[i2], Length(NewPattern));  { Copy the new pattern }      inc(i2, Length(NewPattern));    end;    if (i1>0) and (i1<=Length(S)) then      move(S[i1], Result[i2], Length(S)-i1+1); { Copy leftover text }  end;end; It does need testing. ---------------------------------------------------------------------- CM630 · 2018-01-30 07:36 Lots of thanks, so far it seems to work fine!!! I will report immediately if I see it misbehaving. Or within 2 weeks if I see that everything is fine. ---------------------------------------------------------------------- CM630 · 2018-03-26 08:35 Quote from: CM630 on January 30, 2018, 08:36:04 am...Or within 2 weeks if I see that everything is fine. I as have promised to provide feedback- so far I have no issues with this code. ---------------------------------------------------------------------- engkin · 2018-03-26 20:59 Thanks for the feedback.