views:

654

answers:

6

I have a VB6 application with a search screen. On the search, I have 9 combo boxes. Some of the combo boxes only have a couple items, but some have a couple hundred items. It takes a long time (couple of seconds) to populate the data.

Each combo box is configured the same: Sorted = False, Style = 2 - Dropdown List

3 of the combo boxes have less that 20 items. 1 has 130 items. 4 have approximately 250 items 1 has almost 700 items.

I fill all nine combo boxes with similar code.

While Not RS.EOF

    cmbX.List(i) = RS("Description")
    cmbX.ItemData(i) = RS("Id")

    i = i + 1

    RS.MoveNext
Wend

I tried setting Visible = False but it had no effect on performance.

Is there another way to fill the combo box that will perform better than my existing method?

+1  A: 
Lucas McCoy
Hiliarious illustration, Lucas! A picture is worth a thousand words.
DOK
@DOK: Thanks. I laughed really hard the first time it was show to me. ;-)
Lucas McCoy
The iPod should have smoke coming out of it thoughhttp://www.guardian.co.uk/technology/blog/2009/aug/03/uk-ipod-explodes
MarkJ
A: 

That does seem like a long time. How are you opening your recordsets? Are you using firehose (readonly, forward-only recordset) cursors? If you aren't, you might get some performance improvement from that. Make sure your SQL statements are ONLY returning the data they need for the combo boxes (i.e. DO NOT use a SELECT *).

If your SQL statements contain WHERE clauses or JOINS, make sure you have indexes on the appropriate fields.

If you are using ACCESS as a backend you will get an immediate speed improvement by upsizing to SQL Server Express.

Robert Harvey
Thanks for the helpful comment!I'm using SQL Server. All the queries are optimized to the best of my ability. In fact, I am returning multiple recordset from a single stored procedure. The stored procedure runs in 50 milliseconds in a SQL Server query window. It returns to vb in about 200 milliseconds.I feel comfortable saying that it is NOT a database issue.
G Mastros
So clearly it's taking a long time to populate the combos. OK I have a couple more suggestions. One moment while I edit my answer...
Robert Harvey
+5  A: 

Here is something you can try. According to this post you can shave about 60% off your overhead by using a Windows API function to populate the combo box, instead of the usual AddItem method:

Private Const CB_ERR As Long = -1
Private Const CB_ADDSTRING As Long = &H143
Private Const CB_RESETCONTENT As Long = &H14B
Private Const CB_SETITEMDATA As Long = &H151

Private Declare Function SendMessage Lib "user32" Alias "SendMessageA" (ByVal _
hwnd As Long, ByVal wMsg As Long, ByVal wParam As Long, lParam As Any) As Long

Public Sub AddItem(cmb As ComboBox, Text As Variant, Optional ItemData As Long)

   Dim l As Long
   Dim s As String

   If VarType(Text) = vbString Then
      s = Text
   Else
      s = Trim$(Str$(Text))
   End If

   l = SendMessage(cmb.hwnd, CB_ADDSTRING, 0&, ByVal s)
   If l <> CB_ERR Then
      SendMessage cmb.hwnd, CB_SETITEMDATA, l, ByVal ItemData
   End If

End Sub

Public Sub Clear(cmb As ComboBox)
   SendMessage cmb.hwnd, CB_RESETCONTENT, 0, 0&
End Sub

You might be able to shave a little more off by omitting the function call, and just calling the API function directly.

Robert Harvey
This is much faster. Thanks. Currently, I am putting the ID in the ItemData property of the combo box. I use this when the user clicks the search button. I could store the data in arrays and use the ListIndex property of the combo box to get the ID, but.... is there a way to also set the ItemData property?'
G Mastros
I added some code that includes the call to add the ItemData.
Robert Harvey
Thank you. It works, and it's faster. Exactly what I was looking for.
G Mastros
+1. @G Mastros, can you give us the timing when you do it this way? You said a couple of seconds before. (The timings will be very machine and OS-dependant, but it's still interesting to see a figure.)
MarkJ
I have not yet implemented this code for all the combo boxes. For one of them, it originally took .2 seconds. Adding with blocks brought it down to 0.17 seconds. Using API calls to set the data brought it down to 0.07 seconds.
G Mastros
+2  A: 

Some suggestions:

  • Use With RS.

  • Use the Recordset object's RecordCount property rather than test for EOF on every iteration (if RecordCount = -1 then you should alter the cursor type, cursor location, etc to ensure RecordCount is supported).

  • Use a For..Next loop rather than maintain you own iterator variable.

  • Use the bang operator (!).

For example:

With RS

  Debug.Assert .RecordCount >= 0

  Dim counter As Long
  For counter = 0 To .RecordCount - 1

    cmbX.List(counter) = !Description
    cmbX.ItemData(counter) = !Id

    .MoveNext
  Next

End With

Perhaps something else to consider is setting the combo's Sorted property to False and if sorting is required then either use the Recordset's Sort property or do the sorting at source (e.g. using an ORDER BY clause in SQL code).

onedaywhen
sorted is set to false already (as stated in my original post). Using a with block did speed things up a little, but didn't make too much difference. I do appreciate the suggestion though. Thanks.
G Mastros
+2  A: 

You can try telling the combobox not to repaint itself while you add the new items. You can do this with WM_SETREDRAW. EDIT - apparently this didn't help, probably because the combo box is being hidden while it's filled, which probably gives you all the same benefits.

  Private Declare Function SendMessage Lib "user32" Alias "SendMessageA"( _
    ByVal hwnd As Long, ByVal wMsg As Long, _
    ByVal wParam As Long, lParam As Any) As Long
  Private Const WM_SETREDRAW = &HB

  Call SendMessage(cmbX.hWnd, WM_SETREDRAW, 0, 0&)
  'DO STUFF'
  Call SendMessage(cmbX.hwnd, WM_SETREDRAW, 1, 0&)

Warning: lots of otherwise excellent VB6 websites tell you to use LockWindowUpdate instead. Do not do this, or you will get bugs. Also the disfavour of Raymond Chen!

MarkJ
Thanks for the suggestion. Unfortunately, it didn't make any noticeable improvement.
G Mastros
A: 

Why are you prepopulating the list and then repopulating List(i) and ItemData(i)? Instead you should do the following

While Not RS.EOF

    cmbX.AddItem RS("Description")
    cmbX.ItemData(cmbX.NewIndex) = RS("Id")

    RS.MoveNext
Wend

You will see zero performance difference between Robert Harvey's answer and the code above. For added speed, apply MarkJ's answer.

Another problem could be the cursor you are using to bring the data back from the database. Since you are loading every single item from the recordset, there is very little reason to have a server-side cursor. So you might want to specify Client side cursor when retrieving the recordset.

AngryHacker
Thanks for your suggestion. I am not prepopulating the list and the repopulating list(i) and ItemData(i). AddItem is many times slower than setting the list property. Your suggestion is many times slower than Robert Harvey's. In fact, it's up to 10 times slower. I am already using a client side cursor, the problem is not with the database.
G Mastros
Ok, maybe not prepopulating, but you are creating rows first, and then populating them. In a nutshell you do this: cmbX.AddItem "" cmbX.List(i) = "blah" cmbX.ItemDate(i) = 123 I am not sure your way is faster
AngryHacker