Code taking too long to run, unsure of the cause or the resolution

[DonEB]

I’m attempting to developed a program that dynamically assigns tennis players to a court based on how frequently they’ve played, who they previously played against, etc. In this program, I am currently using a Dictionary and I’ve also tried using a Collection to see which may be more efficient. I've been able to get the program to work In both cases, but the time to process court time for one week took approx 14+ seconds which is only for 8 players (70 combinations) and for two courts.

I don't know if the problem resides with how I'm utilizing the Dictionary or if it is simply the inefficiency of my code. I would like to try and provide a little background then then show a chunk of my code which may very will be dragging the time out with hopes that someone may be able to find a more efficient way of performing the same function. Since I would eventually would like to expand this out to three courts and 12 players, improved efficiency is critical and aany help or guidance would be appreciated.

This all starts with the following:
Subroutine # 1

• A list of 8 player numbers from Sheets("CommonData").Range("$E$25:$L$25").Value2. These are simply 1, 2, 4, 5, 6, 7, 9, 10 (or any combination from 1 to 11)
• This range of numbers is placed in a Dictionary for future manipulation using

      For Each c In rngDic
            dicPrimary1.Add Key:=c.Value, item:=c.Value
      Next

• an array (called lNumbers) which includes a list of 70 combination of 8 player numbers (listed above) taken 4 at a time.
• Then I begin to loop thru each one of the combinations in lNumbers using For i = 1 To UBound(lNumbers)
• lNumbers contains the first potential 4 players and removed them from the Dictionary thus leaving me with the remaining four players to consider. Let’s say players 1, 2, 4 and 5 were first on the list.

      If NumCourts > 1 Then                                                          'If working on filling more than one court
            dicPrimary1.Remove Key:=lNumbers(i, 1)
            dicPrimary1.Remove Key:=lNumbers(i, 2)
            dicPrimary1.Remove Key:=lNumbers(i, 3)
            dicPrimary1.Remove Key:=lNumbers(i, 4)
            Call Subroutine2
      End If

Subroutine #2

• List of players is now down to 4 from Sheets("CommonData").Range("$E$25:$H$25").Value2. Based on above example, players 6, 7, 9, and 10 remain.
• The four player numbers from Subroutine 1 is passed along to Subroutine 2 via Global Variables called Player1, Player2, Player3 and Player 4
• This following may be one location which may cause the system to bog down.
• Notes: FirstCourtT = a Gloval Variable which will contain a calculated value associated with the four player numbers being forwarded from Subroutine 1.
• lNumber(1,5) = a value associated with the remaining four player numbers for Subroutine 2 to examine

If NumCourts = 2 Then                                                                          'Determines if we are dealing with only 2 courts
If nFirstTime1 = 1 Then                                                                          'First time thru, sets values to arrTestCombo()

arrTestCombo(1) = FirstCourtT + lNumbers(1, 5)
arrTestCombo(2) = Player1                                                                     'Player 1 from Court 1, etc.
arrTestCombo(3) = Player2
arrTestCombo(4) = Player3
arrTestCombo(5) = Player4
arrTestCombo(6) = lNumbers(1, 1)                                                         'Player 1 from Court 2, etc.
arrTestCombo(7) = lNumbers(1, 2)
arrTestCombo(8) = lNumbers(1, 3)
arrTestCombo(9) = lNumbers(1, 4)

nFirstTime1 = 2
FirstCourtT = 0
GoTo NextTeam

ElseIf arrTestCombo(1) < (FirstCourtT + lNumbers(1, 5)) Then
arrTestCombo(1) = FirstCourtT + lNumbers(1, 5)
arrTestCombo(2) = Player1
arrTestCombo(3) = Player2
arrTestCombo(4) = Player3
arrTestCombo(5) = Player4
arrTestCombo(6) = lNumbers(1, 1)
arrTestCombo(7) = lNumbers(1, 2)
arrTestCombo(8) = lNumbers(1, 3)
arrTestCombo(9) = lNumbers(1, 4)
EndIf
End if
NextTeam:

I hope this make sense and someone is able to provide some guidance on how to improve the efficiency of this program. Thanks for any thoughts you may pass along this way.

 

[offthelip]

One of the main reasons that Vba is slow is the time taken to access the worksheet from VBa is a relatively long time.
To speed up vba the easiest way is to minimise the number of accesses to the worksheet. What is interesting is that the time taken to access a single cell on the worksheet in vba is almost identical as the time taken to access a large range if it is done in one action.
So you can speed up this little bit:

      For Each c In rngDic
            dicPrimary1.Add Key:=c.Value, item:=c.Value
      Next

by avoiding accessing the worksheet in loop at all like this:

Dim inarr As Variant
 inarr = rngdic
For i = 1 To UBound(inarr, 1)
 For j = 1 To UBound(inarr, 2)
              dicPrimary1.Add Key:=inarr(i, j), Item:=inarr(i, j)
  Next j
Next i

This may not solve your problem but , hopefully it get you to look at where you are accessing the worksheet anywhere else in your code because if you are doing it in a loop it is very slow.
The other possible cause of really slow code could because you are doing multiple loops, do you know what is the maximum depth of looping that you have got in your code?? Do a quick calculation multiply the length of each loop at each level to get the total number of iterations round the most inner bit of the loops. ( Is this a large number??)
This can be a problem, one technique that can help with this is to work whether you can do any early exits from the loops, I.e. are you completing the loop after you have already found your first 4 players . ( Without seeing all your code it is difficult to be specific) These are two things that I would usually look at to speed up code both are usually very easy to implement without much modification.

 

[DonEB]

One of the main reasons that Vba is slow is the time taken to access the worksheet from VBa is a relatively long time.
To speed up vba the easiest way is to minimise the number of accesses to the worksheet. What is interesting is that the time taken to access a single cell on the worksheet in vba is almost identical as the time taken to access a large range if it is done in one action.
So you can speed up this little bit:

      For Each c In rngDic
            dicPrimary1.Add Key:=c.Value, item:=c.Value
      Next

by avoiding accessing the worksheet in loop at all like this:

Dim inarr As Variant
 inarr = rngdic
For i = 1 To UBound(inarr, 1)
 For j = 1 To UBound(inarr, 2)
              dicPrimary1.Add Key:=inarr(i, j), Item:=inarr(i, j)
  Next j
Next i

This may not solve your problem but , hopefully it get you to look at where you are accessing the worksheet anywhere else in your code because if you are doing it in a loop it is very slow.
The other possible cause of really slow code could because you are doing multiple loops, do you know what is the maximum depth of looping that you have got in your code?? Do a quick calculation multiply the length of each loop at each level to get the total number of iterations round the most inner bit of the loops. ( Is this a large number??)
This can be a problem, one technique that can help with this is to work whether you can do any early exits from the loops, I.e. are you completing the loop after you have already found your first 4 players . ( Without seeing all your code it is difficult to be specific) These are two things that I would usually look at to speed up code both are usually very easy to implement without much modification.

If this follow up is too long... my apologies. However, I do want to thank you for your response... you gave me a lot to think about and I had to go back and review the code a bit and I think I may have found the culprit. Not sure exactly how to handle but I think I better understand the cause.

I have a chart (attached) which is critical to reference. This chart, which may contain as many as 20 players, is inside a Loop that goes thru the program 70 times. For now, since I’m only working with trying to fill 2 courts, I will be working with a max of 11 players. There are three subroutines that reference this chart and they are called PreSortReset, PreSort and PlayerCombo. More details about these below.

• Column A and Row 1 are used to identify what players are active. 1 = active, 2 = non-active
• Before the loop begins, this chart (attached) is created
• PreSortReset
  • Resets all the identifiers in Column A and Row 1 back to 2
  • Looks a range(“A5:X24”) and sort selection based on Column B in ascending order
  • Then look at range(“E3:X24”) and sort selection based on Row 3 from Left to Right.
• PreSort
  • Examine Player numbers in Column B and if they match a list of ACTIVE players then the corresponding 2 in Column A is converted from 2 to 1

If Sheets(wsName2).Range("B" & ii).Value = Player Then
Sheets(wsName2).Range("A" & ii).Value = "1"
End If

• Again, examine Player numbers in Row 3 and if they match a list of ACTIVE players then the corresponding 2 in Row 2 is converted from 2 to 1

If Sheets(wsName2).Cells(3, counter).Value = PlayerR Then
Sheets(wsName2).Cells(2, counter).Value = 1
End If

• Then a multiple sort is performed to bring all active players to the upper left corner of the chart.
  • This is for the purpose of the next subroutine automatically making calculations based on pairings
• PlayerCombo
  • This subroutine is designed to return all combinations of players taken four at a time.
    • In this example, 8 players taken 4 at a time results in 70 possible combinations. Thus, I am looping thru 70 times to find the best combination to be placed on two different courts.

I believe, the chart I'm referring to above (and attach for your reference) only needs to be read once each week. Currently, it is being read and written to 70+ times. It would be look at in excess of 480+ times if I try to expand this program to examine players for 3 courts

As I see it, based on your initial review, the issues are as follows:

• Placing the entire chart into an array. This is the simplest to do and can be done BEFORE the LOOP as the guts of the information does not change only that the key sort values will change.
• Once the active players are identified, might you have any thought on how I might go thru the process described above?
• While I'm barely familiar with the VBA Dictionary and Collections, I’ve only just begun to read up on them, I don’t know if sorting in the manner I describe above is possible.
• While a little more familiar with using arrays, finding a value and changing another value from “2” to “1”, I’m not so sure I would know how to do that or, if it’s possible, how to make it work efficiently.

I know I've only provided you some very basic information but any thoughts you might have on making this more efficient would be greatly appreciated.

 

[offthelip]

The two bits of code you posted in your last post are definitely the sort of bits of code which would be faster if you loaded the worksheet into an array.
You say you are doing a lot of sorting specially in a loop this could easily be the major problem. Sorting is a very time consuming business, however you do. It is possible to sort an array entirely in memory,, Chip Pearson had a very good article about sorting arrays here:
Sorting Arrays In VBA
I have never tried using his code for sorting but he is very well known expert on EXCEL
Firstly it worth thinking whether you really need to sort the entire array every time. If you are just looking for the the top 4 values ( or any number) in an array or the top four values of a combination of values it is much faster to write specific code to do this in VBA. this will be much faster than sorting the whole array however you do it.
If you do decide you do really need to sort the array. only do the sorting on the worksheet, do everything else in memory. i.e load the entire worksheet into memory, do your logic processing, write the array back to a worksheet ( it could be just a tempory worksheet) , sort it on the worksheet, load the entire worksheet back into memory, continue your processing. Each time you need a sort, write the array to the worksheet, sort it , read it back to memory..