Delphi Performance: Reading all values under a field in a dataset
Asked Answered
H

8

7

We're trying to find out some performance fixes reading from a TADOQuery. Currently, we loop through the records using 'while not Q.eof do begin ... Q.next method. For each, we read ID and Value of each record, and add each to a combobox list.

Is there a way to convert all values of a specified field into a list in one shot? Rather than looping through the dataset? It would be really handy if I can do something like...

TStrings(MyList).Assign(Q.ValuesOfField['Val']);

I know that's not a real command, but that's the concept I'm looking for. Looking for a fast response and solution (as always but this is to fix a really urgent performance issue).

Heterophyte answered 4/11, 2011 at 22:47 Comment(10)
To be more specific, we have to loop through every record in the dataset, read 2 fields (ID: Integer and Val: String) and add it to a combobox. Well when there's over 20,000 records, this takes a very long time, and I'm looking for a faster way.Heterophyte
20,000 rows in a combobox is a lot to handle, not least for some poor end user. Perhaps you should rethink your design?Adrea
Who are we? How long is very long time? 20k rows is not much data. Which code you want us to optimize urgently? Negative. @Mikael Eriksson, there are some applications for very large simple comboboxes or list boxes, eg: index in electronic dictionary.Euglena
Our company has a large software package for inventory. A customer reported to us today that a certain screen takes about 30 seconds to load. We traced it to this combo box loading 22,000 values (specifically color names of an item, but that's besides the point). The fastest we've gotten it to is about 5 seconds (on a fast server and fast client) but their equipment isn't as fast. I'm just doing some last minute research to see if there's anything else we can do to speed this up, before telling them "sorry, that's as fast as we can get it." I suspect the load time comes from the ADOQuery.Heterophyte
You can fetch whole rowset into TStrings decendant and then assign it to control (with profiling). Nothing more could be said w/o actual code.Euglena
@PrematureOptimization: Looking at TStrings.Assign, that's not likely to be any faster than what he's already doing.Nullify
@Mason Wheeler, thats i problem, with mysterious code, i do not know what exactly he's doing. So that was my last guessing attempt.Euglena
The whole idea is to find the fastest way possible to just transfer all the values under one field in the dataset to a TStrings object, that's all.Heterophyte
May you try to make a test : change to fill a simple tstringList instead of the combobox. Is there any performance enhancement ?Wellbeloved
The following changes have helped speed this up: A) Using BeginUpdate/EndUpdate, B) Using TADODataSet instead of TADOQuery, C) Setting CursorLocation to Server instead of Client, D) Setting CursorType to OpenForwardOnly, E) Setting LockType to ReadOnly and now not only does the data come faster, but also loops through faster. We have no choice but to let these 22K records get dumped in here, so it's a done deal for now i guess, will try some more advanced trials later. Thanks for all the suggestions.Heterophyte
T
3

There are some great performance suggestions made by other folks that you should implement in Delphi. You should consider them. I will focus on ADO.

You haven't specified what the back end database server is, so I can't be too specific, but there are some things that you should know about ADO.

ADO RecordSet

In ADO, there is a RecordSet object. That RecordSet object is basically your ResultSet in this case. The interesting thing about iterating through the RecordSet is that it's still coupled with the provider.

Cursor Type

If your cursor type is Dynamic or Delphi's default Keyset, then each time the RecordSet requests a new row from the provider, the provider will check to see if there were any changes before it returns the record.

So, for the TADOQuery where all you're doing is reading the result set to populate the combobox, and it's not likely to have changed, you should use the Static cursor type to avoid checking for updated records.

In case you don't know what a cursor is, when you call a function like Next, you are moving the cursor, which represents the current record.

Not every provider supports all of the cursor types.

CacheSize

Delphi's and ADO's default cache size for a RecordSet is 1. That's 1 record. This works in combination with the cursor type. The cachesize tells the RecordSet how many records to fetch and store at a time.

When you issue a command like Next (really MoveNext in ADO) with a cache size of 1, the RecordSet only has the current record in memory, so when it fetches that next record, it must request it from the provider again. If the cursor is not Static, that gives the provider the opportunity to get the latest data before returning the next record. So, a size of 1 makes sense for Keyset or Dynamic, because you want the provider to be able to get you the updated data.

Obviously, with a value of 1, there's communication between the provider and RecordSet each time move the cursor. Well, that's overhead that we don't want if the cursor type is static. So, increasing your cache size will reduce the number of round trips between the RecordSet and the provider. This also increases your memory requirements, but it should be faster.

Also note that with a cache size greater than 1 for Keyset cursors, if the record that you want is in the cache, it won't request it from the provider again, which means that you won't see the updates.

Trillby answered 5/11, 2011 at 15:34 Comment(0)
N
13

Looking at your comment, here are a few suggestions:

There are a few things that are likely to be a bottleneck in this situation. The first is looking up the fields repeatedly. If you're calling FieldByName or FindField inside your loop, you're wasting CPU time recomputing a value that's not going to change. Call FieldByName once for each field you're reading from and assign them to local variables instead.

When retrieving values from the fields, call AsString or AsInteger, or other methods that return the data type you're looking for. If you're reading from the TField.Value property, you're wasting time on variant conversions.

If you're adding a bunch of items to a Delphi combo box, you're probably dealing with a string list in the form of the Items property. Set the list's Capacity property and make sure to call BeginUpdate before you start updating, and call EndUpdate at the end. That can enable some internal optimizations that makes loading large amounts of data faster.

Depending on the combo box you're using, it could have some trouble dealing with large numbers of items in its internal list. See if it has a "virtual" mode, where instead of you loading everything up-front, you simply tell it how many items it needs, and when it gets dropped down, it calls an event handler for each item that's supposed to be shown on screen, and you give it the right text to display. This can really speed up certain UI controls.

Also, you should make sure your database query itself is fast, of course, but SQL optimization is beyond the scope of this question.

And finally, Mikael Eriksson's comment is definitely worthy of attention!

Nullify answered 4/11, 2011 at 23:19 Comment(3)
Thank you, great comments, but I already confirmed all of this. I call FieldByName('Val').AsString only once, I am using BeginUpdate/EndUpdate, This is a standard inherited TCustomComboBox component of ours, and the SQL statement literally takes 0 Msec to return. About the "Virtual" mode, can you explain more? Not sure if I can use this or not...Heterophyte
Somehow suspect standard inherited TCustomComboBox component of ours but better use a profiler.Caplan
Good idea to set Capacity in advance!Euglena
A
9

You can use Getrows. You specify the column(s) you are interested in and it will return an array with values. The time it takes to add 22.000 rows to a combo box goes from 7 seconds with the while not ADOQuery1.Eof ... loop to 1.3 seconds in my tests.

Sample code:

var
  V: Variant;
  I: Integer;
begin
  V := ADOQuery1.Recordset.GetRows(adGetRowsRest, EmptyParam, 'ColumnName');

  for I:= VarArrayLowBound(V, 2) to VarArrayHighBound(V, 2) do
    ComboBox1.Items.Add(V[0, I]));
end;

If you want more than one column in the array you should use a variant array as the third parameter.

V := ADOQuery1.Recordset.GetRows(adGetRowsRest, EmptyParam, 
       VarArrayOf(['ColumnName1', 'ColumnName2']);
Adrea answered 5/11, 2011 at 10:28 Comment(0)
T
3

You could try pushing all data into a ClientDataSet and iterating this, but I think copying the data to the CDS does exactly what you are currently doing - looping and assigning.

What I did once was concatenating values on the server, transmitting it in one bulk to the client and splitting it again. This actually made the system faster because it reduced the communication necessary between client and server.

You have to look careful where the performance bottleneck is. It could as well be the combobox if you don't block GUI updates while adding values (ecpecially when we are talking about 20K values - that's a lot to scroll).

Edit: When you cannot change the communication then you perhaps could make it asynchronous. Request new data in a thread, keep the GUI responsive, fill the combobox when the data is there. It means the user sees an empty combobox for 5 seconds, but at least he can do something else in the meantime. Doesn't change the amount of time needed, though.

Toot answered 4/11, 2011 at 23:3 Comment(1)
Thanks for the response. Blocking GUI updates is actually one thing I already tried (after spending 20 minutes trying to convince the lead developer to do so) and it did trim 1 second off the 6 seconds it's taking. This was simply by using TStrings.BeginUpdate and EndUpdate in a try..finally block. 1 second isn't quite enough though. The work on the server side is a good idea, unfortunately this is a huge 16 year old software package with strict SQL communication only, so we can't introduce a new form of connection for loading 1 list.Heterophyte
C
3

You can't avoid looping. "Very long time" is relative but if retrieving 20000 records takes too long there's something wrong.

  • Check your query; perhaps the SQL can be improved (missing index?)
  • Show the code of your loop where you add items to the combobox. Maybe it can be optimized. (calling FieldByName repeatedly in a loop? using variants to retrieve field values?)
  • Make sure to call ComboBox.Items.BeginUpdate; before the loop and ComboBox.Items.EndUpdate after.
  • Use a profiler to find the bottleneck.
Caplan answered 4/11, 2011 at 23:13 Comment(4)
The query takes literally no time at all, 0 Msec. It's the looping that takes time. I am using FieldByName to read the values. I tried BeginUpdate/EndUpdate and although it helped a little, it's not quite enough.Heterophyte
+1 for optimizing FieldByName in a loop away - FieldByName really takes some time if you do it over and over againToot
FieldByName can slow your loop down, especially with many fields in the result set. If that's the problem it's better to use local TField variables, assign them once before the loop and use them within the loop.Caplan
Also don't use the Variant Field.Value, instead use Field.As* property depending on the column data type.Caplan
G
3

Is your query also tied to some data aware controls or a TDataSource? If so, do your looping inside an DisableControls and EnableControls block so your visual controls don't update each time you move to a new record.

Is the list of items fairly static? If so, consider creating a non-visual instance of a combobox at application startup, maybe inside a separate thread, and then assign your non-visual combobox to the visual combobox when your form is created.

Gavel answered 5/11, 2011 at 4:15 Comment(4)
if it is an ADO dataset this is a major bottleneckClemenciaclemency
That's a good idea to try, not sure if the TADOQuery is hooked with any other data sources / controls, but I can see how that can cause this issue.Heterophyte
+1 for the mention of Disable/EnableControls. But a "non-visual combobox" is a bad idea; you can more easily use a TStringList, and there's much less overhead than creating a visual control that will never be seen. TComboBox.Items is a TStrings descendent, after all; you can assign the TStringList to them directly.Garbo
You're right Ken. I use a commercial component set that has an editor repository so I'm a bit spoiled in that regard. The TStringList would be much more efficient in this case.Gavel
T
3

There are some great performance suggestions made by other folks that you should implement in Delphi. You should consider them. I will focus on ADO.

You haven't specified what the back end database server is, so I can't be too specific, but there are some things that you should know about ADO.

ADO RecordSet

In ADO, there is a RecordSet object. That RecordSet object is basically your ResultSet in this case. The interesting thing about iterating through the RecordSet is that it's still coupled with the provider.

Cursor Type

If your cursor type is Dynamic or Delphi's default Keyset, then each time the RecordSet requests a new row from the provider, the provider will check to see if there were any changes before it returns the record.

So, for the TADOQuery where all you're doing is reading the result set to populate the combobox, and it's not likely to have changed, you should use the Static cursor type to avoid checking for updated records.

In case you don't know what a cursor is, when you call a function like Next, you are moving the cursor, which represents the current record.

Not every provider supports all of the cursor types.

CacheSize

Delphi's and ADO's default cache size for a RecordSet is 1. That's 1 record. This works in combination with the cursor type. The cachesize tells the RecordSet how many records to fetch and store at a time.

When you issue a command like Next (really MoveNext in ADO) with a cache size of 1, the RecordSet only has the current record in memory, so when it fetches that next record, it must request it from the provider again. If the cursor is not Static, that gives the provider the opportunity to get the latest data before returning the next record. So, a size of 1 makes sense for Keyset or Dynamic, because you want the provider to be able to get you the updated data.

Obviously, with a value of 1, there's communication between the provider and RecordSet each time move the cursor. Well, that's overhead that we don't want if the cursor type is static. So, increasing your cache size will reduce the number of round trips between the RecordSet and the provider. This also increases your memory requirements, but it should be faster.

Also note that with a cache size greater than 1 for Keyset cursors, if the record that you want is in the cache, it won't request it from the provider again, which means that you won't see the updates.

Trillby answered 5/11, 2011 at 15:34 Comment(0)
K
1

try use DisableControls and EnableControls for increase performance of linear process in dataset.

   var
  SL: TStringList;
  Fld: TField;
begin
  SL := TStringList.Create;
  AdoQuery1.DisableControls;
  Fld := AdoQuery1.FieldByName('ListFieldName'); 
  try
    SL.Sorted := False; // Sort in the query itself first
    SL.Capacity := 25000; // Some amount estimate + fudge factor

    SL.BeginUpdate;  
    try
      while not AdoQuery1.Eof do
      begin
        SL.Append(Fld.AsString);
        AdoQuery1.Next;
      end;
    finally
      SL.EndUpdate;
    end;

    YourComboBox.Items.AddStrings(SL);

  finally
    SL.Free;
    AdoQuery1.EnableControls;
  end;
end;
Kryska answered 3/5, 2012 at 18:25 Comment(0)
G
0

Not sure if this will help, but my suggestion would be not to add directly to the ComboBox. Load to a local TStringList instead, make that as fast as possible, and then use TComboBox.Items.AddStrings to add them all at once:

var
  SL: TStringList;
  Fld: TField;
begin
  SL := TStringList.Create;
  Fld := AdoQuery1.FieldByName('ListFieldName'); 
  try
    SL.Sorted := False; // Sort in the query itself first
    SL.Capacity := 25000; // Some amount estimate + fudge factor
    SL.BeginUpdate;  
    try
      while not AdoQuery1.Eof do
      begin
        SL.Append(Fld.AsString);
        AdoQuery1.Next;
      end;
    finally
      SL.EndUpdate;
    end;
    YourComboBox.Items.BeginUpdate;
    try
      YourComboBox.Items.AddStrings(SL);
    finally
      YourComboBox.Items.EndUpdate;
    end;
  finally
    SL.Free;
  end;
end;
Garbo answered 5/11, 2011 at 1:12 Comment(2)
1. SL.BeginUpdate/EndUpdate won't make any difference for speed, since there is no notification events registered. 2. AddStrings() method already has BeginUpdate/EndUpdate calls. 3. AddStrings() already add individual strings one per one - Conclusion: this won't be faster to use a temporary TStringList.Unwelcome
1. Not now; what if notification events get added in the future? Adding Begin/EndUpdate hurts nothing, and adds protection if needed. 2. Begin/EndUpdate can be nested, and adding them (as I said) hurts nothing. 3. Using the TStringList has less overhead than the TComboBox, and unless you have profiling proof I'm not sure I agree. Conclusion: You've not made a point here, and I've at least offered an attempt to help (and pointed out that I'm not sure it would). But thanks for reading my answer. :) Also, you could have pointed out that the try..finally blocks are probably not needed here.Garbo

© 2022 - 2024 — McMap. All rights reserved.