How can i use PChars correctly to improve this optimization?

Question

I have a function that is called heavily in my app. It is basically a csv parser, which 'pops' the first value off and changes the input string to the remaining string.

function StripString(mask: string; var modifiedstring: string): string;
var
  index,len: integer;
  s: string;
begin
    Index := pos(Mask, ModifiedString);
    len := Length(ModifiedString);
    if index <> 0 then
    begin
        if Index <> 1 then
        begin
            s := LeftStr(ModifiedString, index - 1);
            ModifiedString := RightStr(ModifiedString, len-index);
        end else begin
            if Length(ModifiedString)>1 then
              ModifiedString := Copy(ModifiedString,2,len)
            else
              ModifiedString := '';
            s := '';
        end;
    end else begin
        s := ModifiedString;
        ModifiedString := '';
    end;
    result := s
end;

I want to try and optimize this routine, using PChars. So i came up with this method, but unfortunately i get weird characters in the resultant output. Im guessing its because of incorrect pointers.

//faster method - uses PChars
function StripStringEx(mask: char; var modifiedstring: string): string;
var
  pSt,pCur,pEnd : Pchar;
begin
    pEnd := @modifiedString[Length(modifiedString)];
    pSt := @modifiedString[1];
    pCur := pSt;
    while pCur <= pEnd do
    begin
         if pCur^ = mask then break;
         inc(pCur);
    end;
    SetString(Result,pSt,pCur-pSt);
    SetString(ModifiedString,pCur+1,pEnd-pCur);
end;

Anyone "point":) me in the right direction?

Answer 1

Even if you get your pointer version to work I don't see why it would be faster.

Calling Pos is faster than your loop since Pos is reasonably optimized. The allocation patterns are the same with both versions, two heap allocations and a heap deallocation. I'd stick with the version that works.

You can get rid of the local variable s and assign direct to Result to skip a bit of reference counting.

Answer 2

I think you get strange results because of the SetString on ModifiedString. SetString first sets the length of the string, and then copies the content from the buffer to the newly created string. But in your case the buffer is the destination, and the length of the buffer just got adjusted.

Just follow David's advise and do not use PChars.

If you want, you can make it somewhat shorter:

function StripString(const Mask: string; var ModifiedString: string): string;
var
  Index: Integer;
begin
  Index := Pos(Mask, ModifiedString);
  if Index <> 0 then
  begin
    Result := LeftStr(ModifiedString, Index - 1);
    Delete(ModifiedString, Index);
  end
  else
  begin
    Result := ModifiedString;
    ModifiedString := '';  
  end;
end;