Memory Leak

[xGTx]

This function is causing a horrible memory leak, i am unsure as too why and was hoping you smart people could educate me!

function Split(Source : String; Delimiter : String) : TStringList;
Var Cur, NCur:string;
begin
Result := TStringList.Create;
Cur := Source;
If Pos(Delimiter, Cur) = 0 Then
Begin
Result.Add(Cur);
End;
While Pos(Delimiter, Cur) > 0 Do
Begin
NCur := Copy(Cur, 0, Pos(Delimiter, Cur) - 1);
Result.Add(NCur);
Cur := Copy(Cur, Pos(Delimiter, Cur) + (Length(Delimiter) - 1) + 1, Length(Cur));
If Pos(Delimiter, Cur) = 0 Then
Begin
Result.Add(Cur);
End;
End;
Result.Free;
end;

[cairnswm]

That shouldn't make a Memory Leak but will probably give Access Violation errors when you try to use the result.

At the end of the function you free the result - what this does is remove the memory you have allocated and effectivly returns an invalid pointer as the result of the function. The Result.Free should be removed and after you have finished using the resultant TStringlist in the calling function, it should be freed.





function Split(Source : String; Delimiter : String) : TStringList;
Var Cur, NCur:string;
begin
Result := TStringList.Create;
Cur := Source;
If Pos(Delimiter, Cur) = 0 Then
Begin
Result.Add(Cur);
End;
While Pos(Delimiter, Cur) > 0 Do
Begin
NCur := Copy(Cur, 0, Pos(Delimiter, Cur) - 1);
Result.Add(NCur);
Cur := Copy(Cur, Pos(Delimiter, Cur) + (Length(Delimiter) - 1) + 1, Length(Cur));
If Pos(Delimiter, Cur) = 0 Then
Begin
Result.Add(Cur);
End;
End;
end;

Procedure DoSomthing;
Var
S :TStringlist;
Begin
S := Split(Source, Delimiter);
..
// Do Whatever with S
..
S.Free;
End;



Note: Have a Look at the CommaText Property of TStringlist for splitting it into tokens.

[xGTx]

Actually, it works fine. But every time its called... a couple KB of memory gets used up and doesnt go back down.

[{MSX}]

It works only becouse there is no memory allocation between the free and the use of the result, but that's definitely wrong.
You should free the result on the caller as cairnswm said, or pass an already created TStringList as a parameter.
I use this second version becouse i prefer the creator of a class to be also responsible for freeing it. It make it easier to avoid memory leaks.


procedure Split(Source : String; Delimiter : String; destination:TStringList);
Var Cur, NCur:string;
begin
Destination.clear;
Cur := Source;
..
..
..

[Harry Hunt]

Not really related to the memory leak question, but...

I've written a couple of CSV parsers in the past and I always wondered how to make something like that very fast.

I wrote one parser that worked pretty much like xGTx's, one that essentially loops over the entire length of the string (see below) and one that works kinda like the one below but uses pointers instead of string operations.
The pointer one was pretty fast, but if you've ever tried MS Excel's CSV import, you'll be quite impressed and mine wasn't nearly as fast as that.
Any idea how to write a REALLY fast CSV parser?


procedure Split(S: string; Delimiter: string; OutList: TStringList);
var
I: Integer;
Buf: string;
Current: string;
begin
Buf := '';
Current := '';
for I := 1 to Length(S) do
begin
if Length(Buf) = Length(Delimiter) then
Delete(Buf, 1, 1);
Buf := Buf + S[I];
Current := Current + S[I];
if Buf = Delimiter then
begin
Buf := '';
Delete(Current, Length(Current) - (Length(Delimiter) - 1), Length(Delimiter));
OutList.Add(Current);
Current := '';
end
end;
if Length(Current) > 0 then
OutList.Add(Current)
end;


the code above takes ~460 msecs for 10,000 elements.
xGTx's takes ~16,578 msecs for the same 10,000 elements (I guess because Pos is pretty slow).

[xGTx]

And here I thought Pos was fast haha

Harry, can I use that code?

Edit:

I just tried timing both functions and it appears mine is going almost twice as fast as Harry's.



function TForm1.Split(Source : String; Delimiter : String) : TStringList;
Var Cur, NCur:string;
begin
Result := TStringList.Create;
Cur := Source;
If Pos(Delimiter, Cur) = 0 Then
Begin
Result.Add(Cur);
End;
While Pos(Delimiter, Cur) > 0 Do
Begin
NCur := Copy(Cur, 0, Pos(Delimiter, Cur) - 1);
Result.Add(NCur);
Cur := Copy(Cur, Pos(Delimiter, Cur) + (Length(Delimiter) - 1) + 1, Length(Cur));
If Pos(Delimiter, Cur) = 0 Then
Begin
Result.Add(Cur);
End;
End;
Result.Free;
end;


procedure TForm1.Split2(S: string; Delimiter: string; OutList: TStringList);
var
I: Integer;
Buf: string;
Current: string;
begin
Buf := '';
Current := '';
for I := 1 to Length(S) do
begin
if Length(Buf) = Length(Delimiter) then
Delete(Buf, 1, 1);
Buf := Buf + S[I];
Current := Current + S[I];
if Buf = Delimiter then
begin
Buf := '';
Delete(Current, Length(Current) - (Length(Delimiter) - 1), Length(Delimiter));
OutList.Add(Current);
Current := '';
end
end;
if Length(Current) > 0 then
OutList.Add(Current)
end;

procedure TForm1.DoIt();
var blah : string;
test : TStringList;
begin
blah := 'a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a ,a,a,a,a,a,a,a,a, a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a, a,a,a,a,a,a,a,a,a,a,a,a, a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a, a,a,a,a,a,a,a';
test := TStringList.Create;
test := Split(blah, ',');
end;

procedure TForm1.DoIt2();
var blah : string;
test : TStringList;
begin
blah := 'a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a ,a,a,a,a,a,a,a,a,a,a,a,a, a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a, a,a,a,a,a,a,a,a,a,a,a,a, a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a,a, a,a,a';
test := TStringList.Create;
Split2(blah, ',', test);
test.Free;
end;


procedure TForm1.Button1Click(Sender: TObject);
var i, c1 : integer;
begin
c1 := GetTickCount();
for i := 1 to 2000 do
DoIt();
Button1.Caption := IntToStr(GetTickCount - c1);
end;


procedure TForm1.Button2Click(Sender: TObject);
var i, c1 : integer;
begin
c1 := GetTickCount();
for i := 1 to 2000 do
DoIt2();
Button2.Caption := IntToStr(GetTickCount - c1);
end;


For my function im getting: 260-280 ms
For harry's im getting: 670-740 ms




Edit again!

Trying with delphi's TStringList.Delimiter thing im getting really fast 125-140ms

test := TStringList.Create;
test.Delimiter := ',';
test.DelimitedText := blah;
test.Free;

[Clootie]

Quick test results:

964 (total items: 110121) - Original split
71 (total items: 110110) - DelimitedText split
474 (total items: 110121) - Harry Hunt split


test code:

unit Unit1;

interface

uses
Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
Dialogs, StdCtrls;

type
TSplitFunction = function (const Source : String; const Delimiter : String): TStringList of object;

TForm1 = class(TForm)
Button1: TButton;
ResultsMemo: TMemo;
procedure Button1Click(Sender: TObject);
procedure FormCreate(Sender: TObject);
private
{ Private declarations }
procedure TestIt(testFunc: TSplitFunction; const FuncInfo: String);
//////
function Split_1(const Source : String; const Delimiter : String) : TStringList;
function Split_2(const Source : String; const Delimiter : String) : TStringList;
function Split_3(const S : String; const Delimiter : String) : TStringList;
public
{ Public declarations }
end;

var
Form1: TForm1;

const
Sconst_init = '0, 1, 2, 3, 4, 5, 6, 7, 8, 9, ';
var
Sconst: String = '0, 1, 2, 3, 4, 5, 6, 7, 8, 9, ';

implementation

uses MMSystem;

{$R *.dfm}

procedure TForm1.Button1Click(Sender: TObject);
begin
TestIt(Split_1, 'Original split');
TestIt(Split_2, 'DelimitedText split');
TestIt(Split_3, 'Harry Hunt split');
end;


function TForm1.Split_1(const Source, Delimiter: String): TStringList;
var
Cur, NCur: string;
begin
Result := TStringList.Create;
Cur := Source;
If Pos(Delimiter, Cur) = 0 Then
Begin
Result.Add(Cur);
End;

While Pos(Delimiter, Cur) > 0 Do
Begin
NCur := Copy(Cur, 0, Pos(Delimiter, Cur) - 1);
Result.Add(NCur);
Cur := Copy(Cur, Pos(Delimiter, Cur) + (Length(Delimiter) - 1) + 1, Length(Cur));
If Pos(Delimiter, Cur) = 0 Then
Begin
Result.Add(Cur);
End;
End;
// Result.Free;
end;

function TForm1.Split_2(const Source, Delimiter: String): TStringList;
var
delBackup: Char;
begin
Assert(Length(Delimiter) = 1);
Result := TStringList.Create;
delBackup:= Result.Delimiter;
Result.Delimiter := Delimiter[1];

Result.DelimitedText:= Source;
Result.Delimiter:= delBackup;
end;

function TForm1.Split_3(const S, Delimiter: String): TStringList;
var
OutList: TStringList;
I: Integer;
Buf: string;
Current: string;
begin
OutList := TStringList.Create;
Buf := '';
Current := '';
for I := 1 to Length(S) do
begin
if Length(Buf) = Length(Delimiter) then
Delete(Buf, 1, 1);
Buf := Buf + S[I];
Current := Current + S[I];
if Buf = Delimiter then
begin
Buf := '';
Delete(Current, Length(Current) - (Length(Delimiter) - 1), Length(Delimiter));
OutList.Add(Current);
Current := '';
end
end;
if Length(Current) > 0 then
OutList.Add(Current);

Result:= OutList;
end;

procedure TForm1.TestIt(testFunc: TSplitFunction; const FuncInfo: String);
var
Time: Cardinal;
i, c: Integer;
m: TStrings;
begin
timeBeginPeriod(1);
Time:= timeGetTime;

c:= 0;
for i:= 0 to 10 do
begin
m:= testFunc(Sconst, ',');
Inc(c, m.Count);
m.Free;
end;

Time:= timeGetTime - Time;
ResultsMemo.Lines.Add(Format('%6d (total items: %d) - %s', [Time, c, FuncInfo]));

timeEndPeriod(0);
end;

procedure TForm1.FormCreate(Sender: TObject);
var
i: Integer;
begin
Sconst:= '';
for i:= 0 to 1000 do
Sconst:= Sconst + Sconst_init;
end;

end.

[Clootie]

NOTE: changing initial state:
(1) shorter string to split
(2) increase number of strings to split
AND RESULT CHANGE DRAMATICALLY!!!


123 (total items: 111111) - Original split
68 (total items: 110110) - DelimitedText split
381 (total items: 111111) - Harry Hunt split


changed parts:

procedure TForm1.TestIt(testFunc: TSplitFunction; const FuncInfo: String);
...
begin
...
for i:= 0 to 1000 do
...
end;

procedure TForm1.FormCreate(Sender: TObject);
begin
...
for i:= 0 to 10 do
...
end;

[xGTx]

Well these tests seem to prove that TStringList's in-built split method is fastest. But i've had wierd issues in the past where the StringList didnt like to split by chr(0) and it would split by spaces without even being told too...

as seen here:
procedure TForm1.Button5Click(Sender: TObject);
var test: string; blah : tstringlist; i, c1 : integer;
begin

for i := 0 to 2000 do
test := test + 'a' + chr(0);

blah := tstringlist.Create;
c1 := gettickcount();
blah.Delimiter := chr(0);
blah.DelimitedText := test;
button5.Caption := inttostr(gettickcount - c1);

showmessage(inttostr(blah.Count));

end;

blah.count is 2.... and the timing is 0. I dont understand why this wont work with chr(0);

[Harry Hunt]

Interesting results. Note that the DelimitedText property will only allow a single character as delimiter whereas both my routine and xGTx's routine will allow a string as delimiter (which is a major slowdown, at least in my routine).

Check out how the DelimitedText feature of the Memo is implemented. This also explains why it won't split by #0 (because it uses null-terminated strings).


procedure TStrings.SetDelimitedText(const Value: string);
var
P, P1: PChar;
S: string;
begin
BeginUpdate;
try
Clear;
P := PChar(Value);
while P^ in [#1..' '] do
{$IFDEF MSWINDOWS}
P := CharNext(P);
{$ELSE}
Inc(P);
{$ENDIF}
while P^ <> #0 do
begin
if P^ = QuoteChar then
S := AnsiExtractQuotedStr(P, QuoteChar)
else
begin
P1 := P;
while (P^ > ' ') and (P^ <> Delimiter) do
{$IFDEF MSWINDOWS}
P := CharNext(P);
{$ELSE}
Inc(P);
{$ENDIF}
SetString(S, P1, P - P1);
end;
Add(S);
while P^ in [#1..' '] do
{$IFDEF MSWINDOWS}
P := CharNext(P);
{$ELSE}
Inc(P);
{$ENDIF}
if P^ = Delimiter then
begin
P1 := P;
{$IFDEF MSWINDOWS}
if CharNext(P1)^ = #0 then
{$ELSE}
Inc(P1);
if P1^ = #0 then
{$ENDIF}
Add('');
repeat
{$IFDEF MSWINDOWS}
P := CharNext(P);
{$ELSE}
Inc(P);
{$ENDIF}
until not (P^ in [#1..' ']);
end;
end;
finally
EndUpdate;
end;
end;

[Clootie]

One more variant: "Original split - optimized" 8)

First case (VERY long string):


928 &#40;total items&#58; 110121&#41; - Original split
64 &#40;total items&#58; 110110&#41; - DelimitedText split
427 &#40;total items&#58; 110121&#41; - Harry Hunt split
62 &#40;total items&#58; 110121&#41; - Original split - optimized &#40;!&#41;


Second case (MANY average strings):


125 &#40;total items&#58; 111111&#41; - Original split
66 &#40;total items&#58; 110110&#41; - DelimitedText split
405 &#40;total items&#58; 111111&#41; - Harry Hunt split
64 &#40;total items&#58; 111111&#41; - Original split - optimized &#40;!&#41;


:lol: :lol: :lol: :lol: :lol: :lol:

CODE:

function TForm1.Split_4(const Source, Delimiter: String): TStringList;
var
DelLength: Integer;
SearchPos, FoundAt: Integer;
begin
Result := TStringList.Create;
DelLength:= Length(Delimiter);
SearchPos:= 1; // initialize it

FoundAt:= PosEx(Delimiter, Source, SearchPos);
while FoundAt > 0 do
begin
Result.Add(Copy(Source, SearchPos, FoundAt - SearchPos));
SearchPos:= FoundAt + DelLength;
FoundAt:= PosEx(Delimiter, Source, SearchPos);
end;

Result.Add(Copy(Source, SearchPos, MaxInt));
end;

[xGTx]

So now it's xGTx's CLOOTIE Optimized Split version that wins? haha

Clootie, your a genious! thats pretty dang fast.

But now im thinking, as Harry said, delimiting by a string may be a bit slower... What if we change that new split function to delimit by a single char only. :)

[Clootie]

But now im thinking, as Harry said, delimiting by a string may be a bit slower... What if we change that new split function to delimit by a single char only. :)
Well, this will be your home work for tomorrow :)

[xGTx]

Results&#58; &#91;Length of string split&#58; 720036&#93;
Clootie Optimized Original 172ms
My un-optimized clootie optimized of original 266ms


Alright i tried out by char and obviously its slower. So, CLOOTIE WINS

[Clootie]

Why..? Why you wrote it..!

function TForm1.Split_5(const Source, Delimiter: String): TStringList;
var
SourceLength: Integer;
SearchPos, FoundAt: Integer;
Delim: Char;
begin
Assert(Length(Delimiter) = 1); // !
Delim:= Delimiter[1];
Result := TStringList.Create;
SourceLength:= Length(Source);
SearchPos:= 1; // initialize it
FoundAt:= 1;

while (FoundAt <= SourceLength) do
begin
if Source[FoundAt] = Delim then
begin
Result.Add(Copy(Source, SearchPos, FoundAt - SearchPos));
Inc(FoundAt);
SearchPos:= FoundAt;
end;
Inc(FoundAt);
end;

Result.Add(Copy(Source, SearchPos, MaxInt));
end;

[xGTx]

You optimized it!

If i create a string list... Can i use it multiple times before freeing it (in the same function call) or do i have to free it each time i load new data set in it?

Edit:

I don't understand... My loading function is not freeing my string lists even though im clearly freeing them in the code! Ive managed to use 1.6 GB of ram in 60 seconds of loading. :( <--- Im talking about with the Split() function.. Clooties version. I tried it thru a few tests and it doesn't leak any memory, but in my load function it leaks tons. I went from Split back to Delphi's Delimited way and its not leaking anymore. So it's definitly the split function for some odd reason.

With Clootie's Optimized split:

GTRPG Server Runtime Environment 0.3.5.0
Loading Data.
Loading Maps.
2025 maps loaded in 64 seconds.
Initializing Players. Maximum&#58; 500
Socket awaiting connections.

1.6 GB of RAM used.


With Delphi's delimited:

GTRPG Server Runtime Environment 0.3.5.0
Loading Data.
Loading Maps.
2025 maps loaded in 52 seconds.
Initializing Players. Maximum&#58; 500
Socket awaiting connections.

157 MB of RAM used.



But heres the code: Obviously i have the Split() parts commented out replaced by delphi's delimit

tSplit := TStringList.Create;
nSplit := TStringList.Create;
gSplit := TStringList.Create;

//tSplit := Split(Buffer, '^');
tSplit.Delimiter := '^';
tSplit.DelimitedText := Buffer;


SetLength(Buffer,0);

K := 0;

For X := 0 to MAP_WIDTH do
For Y := 0 to MAP_HEIGHT do
Begin
//nSplit := Split(tSplit.Strings[K], ',');
nSplit.Delimiter := ',';
nSplit.DelimitedText := tSplit.Strings[K];
Inc(K);

// - Bunch of Map stuff loaded here -

if nSplit.Count = 17 Then
begin
//gSplit := Split(nSplit.Strings[16],'|');
gSplit.Delimiter := '|';
gSplit.DelimitedText := nSplit.Strings[16];
for B := 0 to gSplit.Count - 1 Do
begin
Int := StrToInt(gSplit.Strings[B]);
// - More map Stuff -
end;
end;
end;

tSplit.Free;
gSplit.Free;
nSplit.Free;

Result := True;


Now, im no Delphi expert, far from it in-fact but one thing that does cross my mind... In the split function we have
Result := TStringList.Create;
That kind of makes me believe a copy of the string list is being created and obviously never freed... I may be far off here but just a thought.

Somewhat supporting my theory, I goto MSX's suggestion of passing the already made TStringList as a parameter and it again works, fast, and no memory leak!
procedure Split(const Source, Delimiter: String; CopyTo : TStringList);
var
DelLength: Integer;
SearchPos, FoundAt: Integer;
begin
//Result := TStringList.Create;
DelLength:= Length(Delimiter);
SearchPos:= 1; // initialize it

FoundAt:= PosEx(Delimiter, Source, SearchPos);
while FoundAt > 0 do
begin
//Result.Add(Copy(Source, SearchPos, FoundAt - SearchPos));
CopyTo.Add(Copy(Source, SearchPos, FoundAt - SearchPos));
SearchPos:= FoundAt + DelLength;
FoundAt:= PosEx(Delimiter, Source, SearchPos);
end;
//Result.Add(Copy(Source, SearchPos, MaxInt));
CopyTo.Add(Copy(Source, SearchPos, MaxInt));
end;


BUT! I'd still like to know why the original doesn't work in the case above ?

[Paulius]

You're creating StringLists inside a loop and try to free them ontside of it, so youre freeing only the last ones

[xGTx]

I thought it'd use the same memory if i used that same StringList, but ok, im going to change up the code a bit when i get home tonight :) thanks