Why is my code running slower if I call it from a separate thread?

Question

I just added threads to the application using the Delphi TThread class. A thread calls a function that compares two files and prints bits that are different from each other. Before I entered the streams, the application could complete this procedure and print the output after about 1 - 2 seconds in a 300 KB file. After introducing a thread check, the same file can take up to 30-45 seconds and cause a 50% surge in the processor (AMD Phenom II Triple Core), previously you did not notice a surge.

Code executed by the thread:

procedure TForm1.CompareFiles(fil1, fil2 : ansistring; sg : TStringGrid; option : integer; progb : TProgressBar); var forg, fpat : file; byteorg, bytepat : Byte; byteorgc,bytepatc : ansistring; arrby : Array Of ansistring; arrpos : Array Of ansistring; i,x : integer; begin if CRCAdlerGenFile(fil1,1) <> CRCAdlerGenFile(fil2,1) then //Only Run if files arn't same begin sg.Cols[0].Clear; sg.Cols[1].Clear; i := 0; x := 0; AssignFile(forg,fil1); FileMode := fmOpenRead; Reset(forg,1); AssignFile(fpat,fil2); FileMode := fmOpenRead; Reset(fpat,1); //Set Progress Bar progb.Min := 0; progb.Max := FileSize(forg); while NOT eof(forg) do begin BlockRead(forg,byteorg,1); BlockRead(fpat,bytepat,1); Progb.Position := Progb.Position + 1; byteorgc := IntToHex(byteorg,2); bytepatc := IntToHex(bytepat,2); if byteorg <> bytepat then begin x := x + 1; SetLength(arrby,x); SetLength(arrpos,x); arrpos[i] := IntToStr(FilePos(forg)); arrby[i] := bytepatc; i := i + 1; end; end; CloseFile(forg); CloseFile(fpat); case option of 0 : begin //Base 2 for I := 0 to (Length(arrpos) - 1) do begin arrpos[i] := IntToBin(StrToInt(arrpos[i]),8); end; end; 1 : ; //Base 10 2 : begin //Base 16 for I := 0 to (Length(arrpos) - 1) do begin arrpos[i] := IntToHex(StrToInt(arrpos[i]),1); end; end; 3 : begin //Append $ for I := 0 to (Length(arrpos) - 1) do begin arrpos[i] := '$'+IntToHex(StrToInt(arrpos[i]),1); end; end; 4 : begin //Append 0x for I := 0 to (Length(arrpos) - 1) do begin arrpos[i] := '0x'+IntToHex(StrToInt(arrpos[i]),1); end; end; end; Sg.RowCount := Length(arrpos); for I := 0 to (Length(arrpos) - 1) do begin sg.Cells[0,i] := arrpos[i]; sg.Cells[1,i] := arrby[i]; end; if sg.RowCount >= 16 then sg.DefaultColWidth := 222 else sg.DefaultColWidth := 231; end; end; 

The multithreading code used was pretty much used in the previous question, which I asked with minor name changes, as well as information about the introduction and the progress variable, however this was added after I noticed slow processing as a way to control it.

Link to previous question for stream code .

UPDATE:

Fixed Code It looks something like this. I will completely remove the CompareFiles function and move the code to Thread.Execute for read / use convenience.

  procedure TCompareFilesThread.Execute; var forg, fpat : file; byteorg, bytepat : Array[0..1023] of byte; i,z,o : integer; fil1,fil2 : TFilename; begin //Form1.CompareFiles(FEdit3Text, FEdit4Text, FGrid, FOp, FProg); begin fil1 := Form1.Edit3.Text; fil2 := Form1.Edit4.Text; if Form1.CRCAdlerGenFile(fil1,1) <> Form1.CRCAdlerGenFile(fil2,1) then //Only Run if files arn't same begin i := 0; x := 1; o := 0; AssignFile(forg,fil1); FileMode := fmOpenRead; Reset(forg,1); AssignFile(fpat,fil2); FileMode := fmOpenRead; Reset(fpat,1); //Set Progress Bar while NOT eof(forg) do begin while Terminated = False do begin BlockRead(forg,byteorg,1024); BlockRead(fpat,bytepat,1024); for z := 0 to 1023 do begin if byteorg[z] <> bytepat[z] then begin synchronize(sProgBarNext); by := bytepat[z]; off := IntToStr(o); synchronize(SyncGrid); inc(x); end; inc(o); end; end; end; CloseFile(forg); CloseFile(fpat); end; end; Free; end; 

Sync grid

 procedure TCompareFilesThread.SyncGrid; begin form1.StringGrid2.RowCount := x; if x >= 16 then form1.StringGrid2.DefaultColWidth := 222 else Form1.StringGrid2.DefaultColWidth := 232; case op of 0 : off := IntToBin(StrToInt(off),8); //Base 2 1 : ; //Base 10 2 : off := IntToHex(StrToInt(off),1);//Base 16 3 : off := '$'+IntToHex(StrToInt(off),1); //Append $ 4 : off := '0x'+IntToHex(StrToInt(off),1);//Append 0x end; form1.StringGrid2.Cells[0,(x-1)] := off; form1.StringGrid2.Cells[1,(x-1)] := IntToHex(by,2); end; 

Sync prog

 procedure TCompareFilesThread.SProgBarNext; begin Form1.ProgressBar1.Position := Form1.ProgressBar1.Position + 1; end; 

Answer 1

Does this code work in another thread? Well, one obvious problem is the use of VCL controls. VCL is not thread safe, and trying to update VCL properties from outside the main thread will inevitably cause problems. This needs to be reorganized quite strongly. The point of your flow procedure is to do the calculations. You should not pass in TStringGrid, and you should not update progress indicators.

See the Synchronize and Queue methods of the TThread class for the right ways to interact with the main thread from the workflow. It will take a little work, but what you finish will be faster and cleaner.

Answer 2

The default thread priority in Delphi is tpLower, which can take into account the fact that it is slower than you expect. Others correctly pointed out that this piece of code is terribly dangerous. Don't even consider updating a user interface control from a secondary stream in Delphi.