Why is my code so slow?

Question

Top-posted (sorry) answer, for those who don't have time to get into it but may have similar problems.

Rule #1, as always, move as much as you can out of loops.
2, moving TField var := ADODataSet.FieldByname() out of the loop 3, ADODataSet.DisableControls(); and ADODataSet.EnableControls(); around the loop 4, stringGrid.Rows[r].BeginUpdate() and EndUpdate() on each row (cannot do on teh whle control) each of these shaved off a few seconds, but I got it down to "faster than the eye can see" by changing

loop
  stringGrid.RowCount := stringGrid.RowCount + 1;
end loop

to putting stringGrid.RowCount := ADODataSet.RecordCount; before the loop

+1 and heartfelt thanks to all who helped.

(now I will go and see what I can do to optimize drawing a TChart, which is also slow ;-)

---

with about 3,600 rows in the table this takes 45 seconds to populate the string grid. What am I doing wrong?

   ADODataSet := TADODataSet.Create(Nil);
   ADODataSet.Connection := AdoConnection;

   ADODataSet.CommandText := 'SELECT * FROM measurements';
   ADODataSet.CommandType := cmdText;
   ADODataSet.Open();

   while not ADODataSet.eof do
   begin
      TestRunDataStringGrid.RowCount := TestRunDataStringGrid.RowCount + 1;

      measurementDateTime   := UnixToDateTime(ADODataSet.FieldByname('time_stamp').AsInteger);
      DoSQlCommandWithResultSet('SELECT * FROM start_time_stamp', AdoConnection, resultSet);
      startDateTime := UnixToDateTime(StrToInt64(resultSet.Strings[0]));
      elapsedTime   := measurementDateTime - startDateTime;
      TestRunDataStringGrid.Cells[0, Pred(TestRunDataStringGrid.RowCount)] := FormatDateTime('hh:mm:ss', elapsedTime);
      TestRunDataStringGrid.Cells[1, Pred(TestRunDataStringGrid.RowCount)] := FloatToStrWithPrecision(ADODataSet.FieldByname('inputTemperature').AsFloat);
      TestRunDataStringGrid.Cells[2, Pred(TestRunDataStringGrid.RowCount)] := FloatToStrWithPrecision(ADODataSet.FieldByname('outputTemperature').AsFloat);
      TestRunDataStringGrid.Cells[3, Pred(TestRunDataStringGrid.RowCount)] := FloatToStrWithPrecision(ADODataSet.FieldByname('flowRate').AsFloat);
      TestRunDataStringGrid.Cells[4, Pred(TestRunDataStringGrid.RowCount)] := FloatToStrWithPrecision(ADODataSet.FieldByname('waterPressure').AsFloat * convert);
      TestRunDataStringGrid.Cells[5, Pred(TestRunDataStringGrid.RowCount)] := FloatToStrWithPrecision(ADODataSet.FieldByname('waterLevel').AsFloat);
      TestRunDataStringGrid.Cells[6, Pred(TestRunDataStringGrid.RowCount)] := FloatToStrWithPrecision(ADODataSet.FieldByname('cod').AsFloat);
      ADODataSet.Next;
   end;

   ADODataSet.Close();
   ADODataSet.Free();

update:

Function  DoSQlCommandWithResultSet(const command : String; AdoConnection : TADOConnection; resultSet : TStringList): Boolean;
  var
        i : Integer;
        AdoQuery : TADOQuery;

begin
  Result := True;
  resultSet.Clear();

  AdoQuery := TADOQuery.Create(nil);
  try
    AdoQuery.Connection := AdoConnection;
    AdoQuery.SQL.Add(command);
    AdoQuery.Open();
    i := 0;
    while not  AdoQuery.eof do
    begin
      resultSet.Add(ADOQuery.Fields[i].Value);
      i := i + 1;
      AdoQuery.Next;
    end;

  finally
    AdoQuery.Close();
    AdoQuery.Free();
  end;
end;

Answer 1

1. You are executing the command SELECT * FROM start_time_stamp 3,600 times, but it does not appear to me that it is correlated with your outer loop in any way. Why not execute it once before the loop?

2. That SELECT command appears to return only a single column of a single record, yet you use "*" to load all columns, and no WHERE clause to limit the results to a single row (if there's more than one row in the table).

3. You use only a limited number of columns from Measurements, but you retrieve all columns with "*".

4. You don't show the contents of DoSQlCommandWithResultSet, so it's not clear if there's a problem in that routine.

5. It's not clear whether the problem is in your database access or the string grid. Comment out all the lines pertaining to the string grid and run the program. How long does the database access alone take?

Answer 2

Additionally to Larry Lustig points:

1. In general, FieldByName is comparably slow method. You are calling it in loop for the same fields. Move the getting of field references out of the loop and store references in the variables. Like: InputTempField := ADODataSet.FieldByname('inputTemperature');
2. You are resizing the grid in the loop TestRunDataStringGrid.RowCount := TestRunDataStringGrid.RowCount + 1. That is the case, when you should use ADODataSet.RecordCount before the loop: TestRunDataStringGrid.RowCount := ADODataSet.RecordCount.
3. That is a good practice to call ADODataSet.DisableControls before loop and ADODataSet.EnableControls after loop. Even more actual that is for ADO dataset, which has not optimal implementation and those calls help.
4. Depending on a DBMS you are using, you can improve the fetching performance by setting a larger "rowset size". Not sure, how it control in ADO, probably setting ADODataSet.CacheSize to a greater value will help. Also, there are cursor settings :)

Answer 3

instead of calling ADODataSet.FieldByname('Fieldname') inside the loop you should declare local variables of type TField for each field, assign ADODataset.FindField('Fieldname') to the variables and use the variables inside the loop. FindFieldByName searches a list with every call.

Update:

procedure TForm1.Button1Click(Sender: TObject);
var
  InputTemp, OutputTemp: TField;
begin
  ADODataSet := TADODataSet.Create(Nil);
  try
    ADODataSet.Connection := ADOConnection;
    ADODataSet.CommandText := 'SELECT * FROM measurements';
    ADODataSet.Open;
    InputTemp := ADODataSet.FindField('inputTemperature');
    OutputTemp := ADODataSet.FindField('outputTemperature');
    // assign more fields here
    while not ADODataSet.Eof do begin
      // do something with the fields, for example:
      // GridCell := Format ('%3.2f', [InputTemp.AsFloat]);
      // GridCell := InputTemp.AsString;
      ADODataSet.Next;
    end;
  finally
    ADODataSet.Free;
  end;
end;

Another option would be to drop the TADODataset Componont on the form (or use a TDataModule) and define the fields at designtime.

Answer 4

Additional to the Larry Lustig answer, consider using data-aware controls instead, like the TDbGrid component.