Debugging slow execution

R

Rainmanne

Board Regular
Joined
Mar 10, 2016
Messages
134
Office Version
  1. 2019
Platform
  1. Windows
I've got the below piece of code in a macro:
VBA Code: 
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?
 

Excel Facts

Is there a shortcut key for strikethrough?
Ctrl+S is used for Save. Ctrl+5 is used for Strikethrough. Why Ctrl+5? When you use hashmarks to count |||| is 4, strike through to mean 5.
Sort by date Sort by votes
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.
VBA Code: 
   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
 
Upvote 0
Try:
VBA Code: 
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
 
Upvote 0
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:

VBA Code: 
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
 
Upvote 0
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:

VBA Code: 
    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.
 
Upvote 0
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!
 
Upvote 0
Solution
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:
Rich (BB code): 
    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
 
Upvote 0
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.
 
Upvote 0
Does this help?
VBA Code: 
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
 
Upvote 1
You must log in or register to reply here.

Similar threads

M
  • Question Question
Excel Debugger Extremely Slow
2
Replies
13
Views
451
missingsc
M
J
  • Solved
Add an Application InputBox into code
Replies
5
Views
582
julhs
J
J
  • Question Question
Userform inconsistent responses
Replies
1
Views
218
Candyman8019
C
A
  • Question Question
Procedure works in one workbook but not another
Replies
3
Views
81
AlexanderBB
A
B
  • Question Question
VBA Code to Send Mail to skip blank cells
Replies
1
Views
332
Micron
M

Forum statistics

Threads
1,224,827
Messages
6,181,199
Members
453,022
Latest member
RobertV1609

Share this page

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top