Debugging slow execution

[Rainmanne]

I've got the below piece of code in a macro:

If UserForm1.wipe_format = True Then
    
Sheets("tables_for_output").Select

    Range("F4:O4").Select

    For Each r In Selection
        r.Interior.Color = r.DisplayFormat.Interior.Color
    Next r
    Selection.FormatConditions.Delete

    Range("B11:E20").Select

     For Each r In Selection
        r.Interior.Color = r.DisplayFormat.Interior.Color
    Next r
    Selection.FormatConditions.Delete
       ActiveSheet.Range("A1").Select

''<<SLOW PART>>

    For Each ws In Sheets
        With ws
            If .Visible Then
                If UCase(ws.Name) Like "SET*" Then
                ws.Select
                    Range("F6:I15").Select
                        For Each r In Selection
                            r.Interior.Color = r.DisplayFormat.Interior.Color
                            r.Value = r.Value
                        Next r
                        Selection.FormatConditions.Delete
                        ActiveSheet.Range("A1").Select
                End If
            End If
        End With
    Next ws
End If

And the part after <<SLOW PART>> runs in one case fine and in the other case slow. Not significantly slow but quite noticeably. In one case it's around 2 sec and in the other it's 8 sec. The only difference between these cases is that in the second case there are one extra worksheet to check for the name. I have counted the number of iterations (steps) and in the first case there are 200 steps and in the second 218 steps. So it's not that much to account for 6 seconds difference in execution. I wonder what could be a problem or what steps should I take to debug it?

 

[6StringJazzer]

You didn't show the whole Sub so I don't know if your variables are declared. But using Select on everything you want to touch is what is slowing you down. Also disable screen updating.

Here is a rewrite, also with corrected indentation.

   If UserForm1.wipe_format Then
   
      Application.ScreenUpdating = False
      
      With Sheets("tables_for_output")
      
         For Each r In .Range("F4:O4")
            r.Interior.Color = r.DisplayFormat.Interior.Color
         Next r
         .Range("F4:O4").FormatConditions.Delete
         
         For Each r In .Range("B11:E20")
            r.Interior.Color = r.DisplayFormat.Interior.Color
         Next r
         .Range("B11:E20").FormatConditions.Delete
         
      End With
      ''<<SLOW PART>>
      
      For Each ws In Sheets
         With ws
            If .Visible Then
               If UCase(ws.Name) Like "SET*" Then
                  For Each r In ws.Range("F6:I15")
                     r.Interior.Color = r.DisplayFormat.Interior.Color
                     r.Value = r.Value
                  Next r
                  ws.Range("F6:I15").FormatConditions.Delete
                  End If
               End If
            End If
         End With
      Next ws
   
      Application.ScreenUpdating = True

   End If

 

[mumps]

Try:

Sub test()
    Application.ScreenUpdating = False
    Dim ws As Worksheet, r As Range
    If UserForm1.wipe_format = True Then
        With Sheets("tables_for_output")
            For Each r In .Range("F4:O4,B11:E20")
                r.Interior.Color = r.DisplayFormat.Interior.Color
            Next r
            .Range("F4:O4,B11:E20").FormatConditions.Delete
        End With
        For Each ws In Sheets
            With ws
                If .Visible Then
                    If UCase(.Name) Like "SET*" Then
                        For Each r In .Range("F6:I15")
                            r.Interior.Color = r.DisplayFormat.Interior.Color
                            r.Value = r.Value
                        Next r
                    End If
                End If
                .Range("F6:I15").FormatConditions.Delete
            End With
        Next ws
    End If
    Application.ScreenUpdating = True
End Sub

 

[Rainmanne]

Thanks a lot for your suggestions. Unfortunately, they both run with the same difference.

Just to answer @6StringJazzer question, yes all the variables are declared and I disable screen updating:

Option Explicit
Sub UploadData()
...
Dim r As Range
...
Dim ws As Worksheet
...

''Optimize Code
  
Application.ScreenUpdating = False
Application.EnableEvents = False
Application.Calculation = xlCalculationManual
ActiveSheet.DisplayPageBreaks = False
Application.DisplayAlerts = False

...

End Sub

 

[Rainmanne]

I have set a timer in the code to see how long it takes to execute the pieces of the code. The first part regardless whether it's the original code or more elegant solutions from @6StringJazzer or @mumps took 0 sec. The second part, which was not changed in the suggestions:

    For Each ws In Sheets
        With ws
            If .Visible Then
                If UCase(ws.Name) Like "SET*" Then
                ws.Select
                    Range("F6:I15").Select
                        For Each r In Selection
                            r.Interior.Color = r.DisplayFormat.Interior.Color
                            r.Value = r.Value
                        Next r
                        Selection.FormatConditions.Delete
                        ActiveSheet.Range("A1").Select
                End If
            End If
        End With
    Next ws
End If

With one type of data takes between 0.27 and 0.37 sec and with another type of data (with an extra w to check) takes around 12 sec. I don't really understand why adding one worksheet causes such a huge difference in running time. And I tried it with multiple sets of data for each type with the same results.

 

[Rainmanne]

OK, I have figured it out! First, I amended the main code to have the same number of worksheets regardless the type of the data and I have the same difference. It's got me thinking that probably, the issue was with the calculations as an extra workbook adds more references and probably the formulas are recalculated after each iteration which makes it run longer. So I have set calculations on manual before the code and back to auto afterwards. And it worked. No I have the same timing regardless a data type. Thanks a lot to @6StringJazzer and @mumps for the suggestion on optimising the code. I will use it!

 

[Alex Blakenburg]

Not sure why you aren't adopting @6StringJazzer and @mumps changes for the second part but at least take the .value = .value out of the loop.

Using your last code it would change to this:

    For Each ws In Sheets
        With ws
            If .Visible Then
                If UCase(ws.Name) Like "SET*" Then
                    ws.Select
                    Range("F6:I15").Select
                        For Each r In Selection
                            r.Interior.Color = r.DisplayFormat.Interior.Color
                            ' r.Value = r.Value     ' Take outside of loop
                        Next r
                       
                        With Selection
                            .FormatConditions.Delete
                            .Value = .Value
                        End With
                        ActiveSheet.Range("A1").Select
                End If
            End If
        End With
    Next ws

 

[Rainmanne]

I have adopted @mumps changes actually. But they did not resolve the issue, which was in calculations. Thanks a lot for your suggestion! I have changed the code accordingly.

 

[mumps]

Does this help?

Sub test()
    Application.ScreenUpdating = False
    Application.Calculation = xlCalculationManual
    Dim ws As Worksheet, r As Range
    If UserForm1.wipe_format = True Then
        With Sheets("tables_for_output")
            For Each r In .Range("F4:O4,B11:E20")
                r.Interior.Color = r.DisplayFormat.Interior.Color
            Next r
            .Range("F4:O4,B11:E20").FormatConditions.Delete
        End With
        For Each ws In Sheets
            With ws
                If .Visible Then
                    If UCase(.Name) Like "SET*" Then
                        For Each r In .Range("F6:I15")
                            r.Interior.Color = r.DisplayFormat.Interior.Color
                            r.Value = r.Value
                        Next r
                    End If
                End If
                .Range("F6:I15").FormatConditions.Delete
            End With
        Next ws
    End If
    Application.Calculation = xlCalculationAutomatic
    Application.ScreenUpdating = True
End Sub