patternModerate
Perf wrapper for Excel VBA
Viewed 0 times
excelperfwrapperforvba
Problem
Spoiler
This code is actually BAD for performances! So don't use it! ;)
For a while now, I've been using this wrapper to avoid retyping most of it :
I use it like this :
Or to pass arguments to the runned procedure :
I was wondering if :
This code is actually BAD for performances! So don't use it! ;)
For a while now, I've been using this wrapper to avoid retyping most of it :
Public Sub PerfWrap(SubNameToRun As String, _
Optional ByVal DispStatusBar As Boolean = False, _
Optional ArgumentsToPass As String = vbNullString)
Dim aWB As Workbook, _
ActiveSH As Worksheet, _
ScreenUpdateState As Boolean, _
StatusBarState As Boolean, _
CalcState As XlCalculation, _
EventsState As Boolean, _
DisplayPageBreakState As Boolean
Set aWB = ActiveWorkbook
Set ActiveSH = aWB.ActiveSheet
DoEvents
With Application
ScreenUpdateState = .ScreenUpdating
StatusBarState = .DisplayStatusBar
CalcState = .Calculation
EventsState = .EnableEvents
DoEvents
.ScreenUpdating = False
.DisplayStatusBar = DispStatusBar
.Calculation = xlCalculationManual
.EnableEvents = False
End With
DisplayPageBreakState = ActiveSH.DisplayPageBreaks
ActiveSH.DisplayPageBreaks = False
If ArgumentsToPass <> vbNullString Then
Application.Run SubNameToRun, ArgumentsToPass
Else
Application.Run SubNameToRun
End If
DoEvents
ActiveSH.Activate
ActiveSH.DisplayPageBreaks = DisplayPageBreakState
With Application
.ScreenUpdating = ScreenUpdateState
.DisplayStatusBar = StatusBarState
.StatusBar = vbNullString
.Calculation = CalcState
.EnableEvents = EventsState
End With
End SubI use it like this :
Public Sub Launcher_CopyAndUpdate()
Call PerfWrap("CopyAndUpdate_Templates", True)
End SubOr to pass arguments to the runned procedure :
Public Sub Launcher_Update()
Call PerfWrap("Update_Templates", True, "Arg1, Arg2")
End SubI was wondering if :
- it can be improved regarding performances
- converted as a function to return the value sent b
Solution
First, I'd like to echo @Mat'sMug's distaste for your multi-lined
The other answers hint around the performance, but I'll just come right out and say it. Your
You always cache the
Brass tacks time - let's see how much overhead this wrapper really adds. This is the called routine:
And the baseline benchmark:
Unwrapped: 0.8515625
Wrapped: 6.492188
Ouch.
Conclusion - Burn it. Burn it with fire. The premise behind the code is that there is a common set of operations that you can do to "speed up" code. In fact, this is never true. In reality, you are turning off Excel functionality that can slow down code. There's a big difference. If you approach performance issues by looking outside of code that you've written, you're starting out looking in the wrong direction. A catch-all "solution" for performance simply doesn't exist, so I wouldn't pretend to be providing it. The best way to look at it is to imagine that any code under performance review to be copied and pasted in place of this...
...then do a code review on the entire thing. Ask yourself, "In the context of the full Sub, what is the reason for every single line of code other than part above"? I'm guessing that in roughly 99.99% of cases, there won't be a good answer for all of them.
Dim statements. There's absolutely no reason to do this at all having them prefixed with Dim makes it completely obvious that I'm looking at a declaration block. It took me a good minute to realize that it wasn't a continuation of Sub declaration, because the Dim literally disappears into a huge wall of code. You aren't saving any time typing it either - last time I checked, Dim and , _ were both 3 characters, and the former is a lot easier to type on a qwerty keyboard.The other answers hint around the performance, but I'll just come right out and say it. Your
Sub should really be named the AntiPerfWrap. First, every single call that Application.Run makes is late bound - even for Excel objects. It has to be, because like CallByName, it is forced to call IDispactch.GetIDsOfNames on itself. This is always slower than an early bound call - period, end of discussion.You always cache the
ActiveWorkbook and ActiveSheet regardless of whether it is necessary or not. I would guess that roughly 1% of all the code I've written requires this. Same thing with .Calculation. Same thing with .EnableEvents. Do I write another PerfWrapWithEvents if I need that turned on? Or maybe PerfWrapWithEventsAllowCalculation? None of this work is free, and it seems like a jumbled collection of "this will make your code go faster anecdotes" collected from SO comments. The cargo cult has found thier messiah in this wrapper.DoEvents is not free. In fact it's the complete opposite. It yields the processor to every other thread that needs to clear it's event queue. It also makes absolutely no sense to call DoEvents when you've explicitly disabled 95% of the functionality that would make the call useful.Brass tacks time - let's see how much overhead this wrapper really adds. This is the called routine:
Public Sub UnderTest()
Dim i As Long
For i = 1 To 10
Debug.Print i
Next
End SubAnd the baseline benchmark:
Public Sub BenchmarkOne()
Dim starting As Single
starting = Timer
Dim i As Long
For i = 1 To 100
Sheet1.UnderTest
Next
Debug.Print "Unwrapped: " & Timer - starting
End SubUnwrapped: 0.8515625
Public Sub BenchmarkTwo()
Dim starting As Single
starting = Timer
Dim i As Long
For i = 1 To 100
PerfWrap "Sheet1.UnderTest"
Next
Debug.Print "Wrapped: " & Timer - starting
End SubWrapped: 6.492188
Ouch.
Conclusion - Burn it. Burn it with fire. The premise behind the code is that there is a common set of operations that you can do to "speed up" code. In fact, this is never true. In reality, you are turning off Excel functionality that can slow down code. There's a big difference. If you approach performance issues by looking outside of code that you've written, you're starting out looking in the wrong direction. A catch-all "solution" for performance simply doesn't exist, so I wouldn't pretend to be providing it. The best way to look at it is to imagine that any code under performance review to be copied and pasted in place of this...
If ArgumentsToPass <> vbNullString Then
Application.Run SubNameToRun, ArgumentsToPass
Else
Application.Run SubNameToRun
End If...then do a code review on the entire thing. Ask yourself, "In the context of the full Sub, what is the reason for every single line of code other than part above"? I'm guessing that in roughly 99.99% of cases, there won't be a good answer for all of them.
Code Snippets
Public Sub UnderTest()
Dim i As Long
For i = 1 To 10
Debug.Print i
Next
End SubPublic Sub BenchmarkOne()
Dim starting As Single
starting = Timer
Dim i As Long
For i = 1 To 100
Sheet1.UnderTest
Next
Debug.Print "Unwrapped: " & Timer - starting
End SubPublic Sub BenchmarkTwo()
Dim starting As Single
starting = Timer
Dim i As Long
For i = 1 To 100
PerfWrap "Sheet1.UnderTest"
Next
Debug.Print "Wrapped: " & Timer - starting
End SubIf ArgumentsToPass <> vbNullString Then
Application.Run SubNameToRun, ArgumentsToPass
Else
Application.Run SubNameToRun
End IfContext
StackExchange Code Review Q#158048, answer score: 10
Revisions (0)
No revisions yet.