Delphi PPL TTask Procedure with Parameters , ADOQuery

Question

This is the followup of my previous post . I now have a few ADOQueries in the Procedure and one ADOConnection . All of these are Dynamically Created and Freed .

QUESTION : I was hoping that this entire Compare Procedure would be a lot faster if I use 4 Threads , but instead I have the same speed as without Threads, only it is a lot more complicated now . Or maybe I am doing something completely wrong here ?

The Code is here , sorry I know it is a bit to much , but without the code my Question makes no sense and the answer would only be guessing . MSSQL is not wrapped in a Try Finally I know , I just wanted first to check if there is any speed gain here .

Entire Procedure pasted as asked . I also altered it I do the CREATE of Objects outside the Loop and the Free After the Loop .

procedure TMain.SYNC(AProgressBar: TProgressBar; AData : array of RemoteDATA);
var i : integer;
    isFound : boolean;
    LStatus, LSU_Stueck, LHistLines , LPosLines, LSTLLines, LTXTLines, LKTXTLines : integer;
    ABQuery , HistQuery, PosQuery, STLQuery, TXTQuery, KTXTQuery : TADOQuery;
    MSSQL : TADOConnection;
begin
  MSSQL := TADOConnection.Create(nil);
  MSSQL.ConnectionString:='FILE NAME='+ExtractFilePath(Application.ExeName)+'\xlr_main.udl';
  MSSQL.Provider:='SQLOLEDB.1';
  MSSQL.KeepConnection:=true;
  MSSQL.LoginPrompt:=false;

  ABQuery := TADOQuery.Create(nil);
  ABQuery.Connection:=MSSQL;

  HistQuery := TADOQuery.Create(nil);
  HistQuery.Connection:=MSSQL;

  PosQuery := TADOQuery.Create(nil);
  PosQuery.Connection:=MSSQL;

  STLQuery := TADOQuery.Create(nil);
  STLQuery.Connection:=MSSQL;

  TXTQuery := TADOQuery.Create(nil);
  TXTQuery.Connection:=MSSQL;

  KTXTQuery := TADOQuery.Create(nil);
  KTXTQuery.Connection:=MSSQL;

  for i := Low(AData) to High(AData) do
    begin
      isFound:=false;
      LStatus:=0;
      LSU_Stueck:=0;
      LHistLines:=0;
      LPosLines:=0;
      LSTLLines:=0;
      LTXTLines:=0;
      LTXTLines:=0;


      ABQuery.SQL.Clear;
      ABQuery.SQL.Add('select AB,STATUS,SU_STUECK,DB_YEAR from BW_AUFTR_KOPF where AB='+inttostr(AData[i].AB)+' and DB_YEAR='+inttostr(AData[i].DB_YEAR));
      ABQuery.Open;

      if ABQuery.RecordCount <> 0 then
        begin
          isFound:=true;

          LStatus:=ABQuery.FieldByName('Status').AsInteger;
          LSU_Stueck:=ABQuery.FieldByName('SU_STUECK').AsInteger;
        end;


      HistQuery.SQL.Clear;
      HistQuery.SQL.Add('select COUNT(Datum) as HistLines from BW_AUFTR_HIST where AB='+inttostr(AData[i].AB)+' and DB_YEAR='+inttostr(AData[i].DB_YEAR));
      HistQuery.Open;

      LHistLines:=HistQuery.FieldByName('HistLines').AsInteger;

      PosQuery.SQL.Clear;
      PosQuery.SQL.Add('select COUNT(POS_NR) as PosLines from BW_AUFTR_POS where AB='+inttostr(AData[i].AB)+' and DB_YEAR='+inttostr(AData[i].DB_YEAR));
      PosQuery.Open;

      LPosLines:=PosQuery.FieldByName('PosLines').AsInteger;

      STLQuery.SQL.Clear;
      STLQuery.SQL.Add('select COUNT(POS_NR) as STLLines from BW_AUFTR_STL where AB='+inttostr(AData[i].AB)+' and DB_YEAR='+inttostr(AData[i].DB_YEAR));
      STLQuery.Open;

      LSTLLines:=STLQuery.FieldByName('STLLines').AsInteger;

      TXTQuery.SQL.Clear;
      TXTQuery.SQL.Add('select COUNT(POS_NR) as TXTLines from BW_AUFTR_TXT where AB='+inttostr(AData[i].AB)+' and DB_YEAR='+inttostr(AData[i].DB_YEAR));
      TXTQuery.Open;

      LTXTLines:=TXTQuery.FieldByName('TXTLines').AsInteger;

      KTXTQuery.SQL.Clear;
      KTXTQuery.SQL.Add('select COUNT(LFD_NR) as KTXTLines from BW_AUFTR_KTXT where AB='+inttostr(AData[i].AB)+' and DB_YEAR='+inttostr(AData[i].DB_YEAR));
      KTXTQuery.Open;

      LKTXTLines:=KTXTQuery.FieldByName('KTXTLines').AsInteger;


      if isFound = true then
        begin
            if (AData[i].STATUS <> LStatus) or (AData[i].SU_STUECK <> LSU_Stueck)
            or (AData[i].HISTLINES <> LHistLines) or (AData[i].POSLINES <> LPosLines) or (AData[i].STLLINES <> LSTLLines)
            or (AData[i].TXTLINES <> LTxtLines) or (AData[i].KTXTLINES <> LKTXTLines) then
              if (AData[i].STATUS < 100) and (AData[i].STATUS <> 98) then
                begin
                  setlength(CHANGED_ARRAY,length(CHANGED_ARRAY)+1);
                  CHANGED_ARRAY[High(CHANGED_ARRAY)].AB:=AData[i].AB;
                  CHANGED_ARRAY[High(CHANGED_ARRAY)].DB_YEAR:=AData[i].DB_YEAR;
                end;

            if (AData[i].STATUS = 98) or (AData[i].STATUS = 117) or (AData[i].STATUS = 900)
            or (AData[i].STATUS = 999) then
              begin
                setlength(DELETE_ARRAY,length(DELETE_ARRAY)+1);
                DELETE_ARRAY[High(DELETE_ARRAY)].AB:=AData[i].AB;
                DELETE_ARRAY[High(DELETE_ARRAY)].DB_YEAR:=AData[i].DB_YEAR;
              end;
        end
      else
        begin
          if (AData[i].STATUS <> 98) and (AData[i].STATUS <> 117) and (AData[i].STATUS <> 900)
          and (AData[i].STATUS <> 999) and (AData[i].STATUS <> 120) then
            begin
              setlength(NEW_ARRAY,length(NEW_ARRAY)+1);
              NEW_ARRAY[High(NEW_ARRAY)].AB:=AData[i].AB;
              NEW_ARRAY[High(NEW_ARRAY)].DB_YEAR:=AData[i].DB_YEAR;
            end;
        end;

      TThread.Queue(TThread.CurrentThread,
      procedure
      begin
        AProgressBar.Position:=i;
      end);

    end;

  ABQuery.Free;
  HistQuery.Free;
  PosQuery.Free;
  STLQuery.Free;
  TXTQuery.Free;
  KTXTQuery.Free;

  MSSQL.Free;
end;

And I start the Threads here , then I wait for it to complete , but I also must show the progress .

  if length(RAB_ARRAY) > 10 then
    begin
      Edit1.Text:='Items '+inttostr(length(RAB_ARRAY))+' 1/4 '+inttostr(length(RAB_ARRAY) div 4)+' LeftOver '+inttostr(length(RAB_ARRAY) mod 4);

      Part:=length(RAB_ARRAY) div 4;
      LeftOver:=length(RAB_ARRAY) mod 4;

      setlength(Tasks,4);

      Tasks[0] := TTask.Create(
        procedure
          begin
           SYNC(progressThread1,copy(RAB_ARRAY,0,Part))
          end);
      Tasks[1] := TTask.Create(
        procedure
          begin
            SYNC(progressThread2,copy(RAB_ARRAY,Part-1,Part))
          end);
      Tasks[2] := TTask.Create(
        procedure
          begin
            SYNC(progressThread3,copy(RAB_ARRAY,(2*Part)-2,Part))
          end);
      Tasks[3] := TTask.Create(
        procedure
          begin
            SYNC(progressThread4,copy(RAB_ARRAY,(3*Part)-3,Part+LeftOver))
          end);

      progressThread1.Max:=Part;
      progressThread2.Max:=Part;
      progressThread3.Max:=Part;
      progressThread4.Max:=Part+LeftOver;

      Tasks[0].Start;
      Tasks[1].Start;
      Tasks[2].Start;
      Tasks[3].Start;


      while true do
        begin
          if (Tasks[0].Status = TTaskStatus.Completed) and (Tasks[1].Status = TTaskStatus.Completed)
            and (Tasks[2].Status = TTaskStatus.Completed) and (Tasks[3].Status = TTaskStatus.Completed) then
              break;
          Application.ProcessMessages;
        end;

Instead of the no-no `ProcessMessages` loop, just use `TTask.WaitForAll(tasks);` — LU RD, Jun 22 '18 at 09:08
`I was hoping that this entire Compare Procedure would be a lot faster if I use 4 Threads` Well...one important lesson is that you don't program based on hope. If you're making an architectural change you should be doing it because you **know** it will be better than what you're doing now. Guess and check will just waste a frightful amount of your time. What was your bottleneck? Why did you think this would fix it? If you can't answer those questions I would start there. I might guess memory contention is the problem but profiling is really the answer here. — J..., Jun 22 '18 at 09:15
The mysterious `....` makes it difficult for anyone to say what your problems might be. Maybe your bottleneck is in there. Who knows? — J..., Jun 22 '18 at 09:22
You are creating and freeing your query objects on every loop iteration... this is very inefficient and memory intensive (threads serialize access to the memory manager, remember). Create them once and free them when you are done. You don't need to do it `Length(AData)` times. — J..., Jun 22 '18 at 09:27
And what of `CHANGED_ARRAY`, `DELETE_ARRAY`, and `NEW_ARRAY` - those aren't declared anywhere? Are they globals? Fields? Resizing arrays in every loop iteration is also memory intensive - hopefully they're not shared across threads. — J..., Jun 22 '18 at 09:46
the ARRAYS are Global Variables , and are shared by all four threads . I just commented out the CHANGED_ARRAY, DELETE_ARRAY and NEW_ARRAY parts to see if there is a speed difference . And nope still the same speed . — user1937012, Jun 22 '18 at 09:54
**Bottleneck** : I did not know , the Program Synced the two Databases before in 1 min 30 sec . I just wanted to try and see if I can maybe bring it down to 30 sec with Multi-threading . But it seems like the Database just wont do it faster . — user1937012, Jun 22 '18 at 10:00
Well, regardless of any performance issues, sharing dynamic arrays across threads that are all simultaneously modifying and resizing them is a catastrophic error. Don't do that. — J..., Jun 22 '18 at 10:03
Ok noted, I thought that if I ONLY add to the array , nothing bad can happen . (no delete , no modify existing records ) . Should I then maybe have an ARRAY for each Thread ? Or Lock the Array ? — user1937012, Jun 22 '18 at 10:05
Imagine two threads resize at the same time. Then what? So, yes, you need to solve it some other way. — J..., Jun 22 '18 at 10:07
Profiling would give you the answer you need, here. `SELECT COUNT...` can be very slow. You're likely hitting the disk on the SQL server and this could likely be the big bottleneck. See : [this answer](https://stackoverflow.com/a/11130604/327083). This will probably be an exercise in optimizing your table's indexes - I suspect your query is probably being forced to scan the table for lack of a suitable index. — J..., Jun 22 '18 at 10:10
Also, using parameters in your queries (rather than concatenating) would allow the server to cache the queries and improve performance. — J..., Jun 22 '18 at 11:55
this I will try to , thank you very much for your kind help! — user1937012, Jun 22 '18 at 12:01

score 1 · Accepted Answer · answered Jun 22 '18 at 10:15

1

The answer here is that you need to profile your code and your queries to determine what your real bottleneck is here. Most likely it is that your numerous SELECT COUNT... queries do not have suitable indexes and the SQL server is being forced to scan the table, hitting the disk and providing the major serial bottleneck.

answered Jun 22 '18 at 10:15

J...

30,968
6
66
143

I will accept this, because it seems I really need to check it with a Profiler . The Database is indexed... as you see I check after AB and DB_YEAR ... both fields are indexed... the Count usually is less then 10 ... – user1937012 Jun 22 '18 at 10:52
@user1937012 It's not the size of the returned count that matters, it's the size of the table you are searching to find the count. – J... Jun 22 '18 at 11:38
**UPDATE** , I tried it with LockType := ltReadOnly . And **BANG** , now it is brutally fast , with 4 threads my wors time was 0.19 sec, and the last 3 times were a unbelievable 0.08sec . To be sure that I am not seeing things, I tried the original SIngle Threaded solution with LockType ltReadOnly and it did better 0.47 , and after that a few 0.24 but nowhere near as good as the 4 Threads . I checked Microsofts site about the LockType for ADO . If I understand it correctly ReadOnly means NO LOCKING . **ALSO** I see a bigger variation in the Thread speed , some of them are faster then others . – user1937012 Jun 22 '18 at 11:48
2

@user1937012 then you should un-accept this answer and write one yourself – J... Jun 22 '18 at 11:53

Delphi PPL TTask Procedure with Parameters , ADOQuery

1 Answers1