cristianbuse / VBA-FastExcelUDFs

Excel User Defined Functions (VBA) run faster
MIT License
14 stars 4 forks source link

Slow when column filters are used. #1

Closed hidp123 closed 2 years ago

hidp123 commented 2 years ago

Calculations are slow when column filters are used. It feels as though the code looses its advantage when column filters are used.

Also, when I add a new column, the automatic recalculation seems to be slow as previously, doesn't seem to benefit from your code in this case too. I'll test more.

Regards

cristianbuse commented 2 years ago

@hidp123 That's interesting. I could not replicate the slow bahaviour on both having a filter or inserting a column.

Is there any way you could share a minimal example file so that I can test? Thanks!

hidp123 commented 2 years ago

I'll send the link in a private message. How can I?

cristianbuse commented 2 years ago

@hidp123 Please send to cristian.buse@yahoo.com

cristianbuse commented 2 years ago

@hidp123 I just tested on your file (with code added from this repo) and the speed is exactly the same when filtering or inserting/deleting a column. It takes about 35 seconds to calculate each time. WIthout the bug fix from this repository it takes a lot longer. Those 35 seconds are not related to the UDF bug but rather to how your file is structured and how code is written. There are many improvements you could do but unfortunately I do not have the time to go into details. Good luck!

I will close this issue. Thanks for reporting!

hidp123 commented 2 years ago

Ok. I'll test further.

I have updated the file with the UDFs throughout more columns.

When I opened the excel file it took about 65 seconds for the calculations to complete. When I added a column after opening it took 130 seconds. When I added another column it took 60 seconds.

Adding a filter takes about 60 seconds (Coloumn AP). Adding a second filter on another column took similar time.

PS. Any help to improve my code is always welcome :)

Regards

hidp123 commented 2 years ago

Any idea why the sheet recalculates every time I open the excel file?

cristianbuse commented 2 years ago

@hidp123 Try marking all your UDFs as non-volatile. All my UDFs start with these 2 lines:

    Application.Volatile False
    LibUDFs.TriggerFastUDFCalculation

Once that is done, add this to the ThisWorkbook module:

Private Sub Workbook_SheetCalculate(ByVal Sh As Object)
    Debug.Print Sh.Name
End Sub

and then change a blank cell somewhere. The Immediate window (make sure you have it on in the IDE) will print any names from all sheets that calculate. If you find any name other than the one you just updated (where the cell you changed resides) then you must have volatile formulas in those sheets. Avoid volatile formulas entirely if possible, including OFFSET, INDIRECT, TODAY etc. For example you could replace OFFSET with INDEX. See here

Definitely, replace all your VLOOKUPs with INDEX/MATCH. Try reusing the result of the MATCH (via intermediate column or LET function) if you have multiple columns to retrieve (via INDEX). Basically, reuse calculations where possible.

If you have Office 365 then make the switch to spill array functionality. It is orders of magnitude faster. You could also modify your UDFs to return full arrays rather than one value. I've recently redesigned a financial model that was taking 3 minutes to calculate with only 100 rows across 40 sheets and about 300 columns per sheet. I got it down to 0.5 seconds even if now it works with 5000 rows and 1000 columns. Spill array functionality is the future of Excel, no doubt.

Try profiling code and see where your bottlenecks are.

Try replacing Worksheet Functions with your own. For example you could use:

Public Function Radians(ByVal sexaAngle As Double) As Double
    Const PI As Double = 3.14159265358979
    Radians = sexaAngle / 180 * PI
End Function

instead of WorksheetFunction.Radians.

Just to prove the point, I ran this:

Sub Test()
    Const iterations As Long = 1000000
    Dim t As Double
    Dim i As Long
    Dim d As Double
    '
    t = Timer
    For i = 1 To iterations
        d = WorksheetFunction.Radians(i)
    Next i
    Debug.Print Round(Timer - t, 3)
    '
    t = Timer
    For i = 1 To iterations
        d = Radians(i)
    Next i
    Debug.Print Round(Timer - t, 3)
End Sub

and got 3.312 seconds for the WorksheetFunction.Radians and only 0.102 seconds for my own Radians.

Like I said, there are many improvements you could make. If I have time in two weeks then maybe I will address your question on Code Review. Best of luck!

hidp123 commented 2 years ago

I did the search for volatile functions, in the immediate window it only showed the current sheet and one other sheet (Compa.) which takes its values from the current "Master" sheet, but I don't have any actual volatile functions on that sheet.

Definitely, replace all your VLOOKUPs with INDEX/MATCH.

I'll try this, but what I've noticed is that the speed issues I'm facing is mainly (probably 95%+) due to the UDFs and VBA code I've written.

Try profiling code and see where your bottlenecks are.

Any suggestions for a good free tool for this?

Try replacing Worksheet Functions with your own.

I've done this for radians and for degrees based on your code above. Thanks. I'm surprised that the built in functions are worse than custom code.

You could also modify your UDFs to return full arrays rather than one value.

That's something I believe will be very useful and fast, but need to get my head around it. Doing VBA coding for the first time :). I guess I'll need a sub for that instead of a function.

cristianbuse commented 2 years ago

@hidp123

I did the search for volatile functions, in the immediate window it only showed the current sheet and one other sheet (Compa.)

Try changing a cell in the current sheet that is definitely not referenced in the Compa sheet. You can confirm this if the Trace Dependents (Formulas ribbon tab) finds nothing. If the Immediate window still shows Compa as calculating then you surely have something volatile in Compa. If not, then great.

the speed issues I'm facing is mainly (probably 95%+) due to the UDFs and VBA code

Although this repo works around the UDF bug which is a nice speed gain, you can still improve on your UDFs. More on this below.

Any suggestions for a good free tool for this?

I only use VBA. For example you could place this code in your model:

Option Explicit

'*******************************************************************************
'Calculate all worksheets and display execution times
'*******************************************************************************
Public Sub BenchmarkSheets()
    Dim app As New ExcelAppState: app.StoreState: app.Sleep
    Dim ws As Worksheet
    Dim book As Workbook
    Dim t As Double
    Dim arr() As Variant
    Dim i As Long
    Dim cell As Range
    Dim totalTime As Double

    ReDim arr(0 To ThisWorkbook.Worksheets.Count, 1 To 3)
    arr(0, 1) = "Sheet Name"
    arr(0, 3) = "Tab Color"
    i = 0
    For Each ws In ThisWorkbook.Worksheets
        i = i + 1
        arr(i, 1) = ws.Name
        arr(i, 3) = ws.Tab.Color
        t = Timer
        ForceDirty ws
        ws.Calculate
        arr(i, 2) = Round(Timer - t, 3)
        totalTime = totalTime + arr(i, 2)
    Next ws
    arr(0, 2) = "Calc time (s): " & totalTime
    '
    Dim r As Long: r = UBound(arr, 1) - LBound(arr, 1) + 1
    Dim c As Long: c = UBound(arr, 2) - LBound(arr, 2) + 1
    Dim tColor As Variant

    Set book = Workbooks.Add
    With book.Worksheets(1)
        Range(.Cells(1, 1), .Cells(r, c - 1)).Value2 = arr
        For i = 1 To UBound(arr, 1)
            Set cell = .Cells(i + 1, 1)
            tColor = arr(i, 3)
            If VarType(tColor) = vbBoolean Then tColor = 15132390
            cell.Interior.Color = tColor
            cell.Font.Color = TextColorToUse(CLng(tColor))
        Next i
        With Range(.Cells(1, 1), .Cells(1, 2))
            .WrapText = True
            .HorizontalAlignment = xlCenter
            .VerticalAlignment = xlCenter
        End With
    End With

    Dim rng As Range
    With book.Worksheets(1)
        Set rng = .Range(.Cells(2, 2), .Cells(r, 2))
    End With
    With rng.FormatConditions.AddDatabar
        .ShowValue = True
        .SetFirstPriority
        .MinPoint.Modify newtype:=xlConditionValueAutomaticMin
        .MaxPoint.Modify newtype:=xlConditionValueAutomaticMax
        .BarColor.Color = 5920255
        .BarColor.TintAndShade = 0
        .BarFillType = xlDataBarFillGradient
        .Direction = xlContext
        .NegativeBarFormat.ColorType = xlDataBarColor
        .BarBorder.Type = xlDataBarBorderSolid
        .NegativeBarFormat.BorderColorType = xlDataBarColor
        .BarBorder.Color.Color = 5920255
        .BarBorder.Color.TintAndShade = 0
        .AxisPosition = xlDataBarAxisAutomatic
        .AxisColor.Color = 0
        .AxisColor.TintAndShade = 0
        .NegativeBarFormat.Color.Color = 255
        .NegativeBarFormat.BorderColor.Color = 255
        .NegativeBarFormat.BorderColor.TintAndShade = 0
    End With
    book.Worksheets(1).UsedRange.Columns.AutoFit
    app.RestoreState
End Sub

Function TextColorToUse(BackColor As Long) As Long
'  This function returns the color to use for
'  text to make it readable on a dark background
'  Code by of Rick Rothstein
  Dim Luminance As Long
  Luminance = 77 * (BackColor Mod &H100) + _
              151 * ((BackColor \ &H100) Mod &H100) + _
              28 * ((BackColor \ &H10000) Mod &H100)
  '  Default value of TextColorToUse is 0-Black, set
  '  it to White if the Luminance is less than 32640
  If Luminance < 32640 Then TextColorToUse = vbWhite
End Function

'*******************************************************************************
'Force mark all Formulas in worksheet as Dirty (needing recalculation)
'Main use would be to forcefully calculate non-volatile UDFs
'*******************************************************************************
Public Sub ForceDirty(ByVal ws As Worksheet)
    If Not ws.EnableCalculation Then Exit Sub 'When enabled will mark Dirty
    '
    On Error Resume Next
    'Please note that turning .EnableCalculation off and then back on is faster
    '   than: ws.UsedRange.SpecialCells(xlCellTypeFormulas).Dirty
    ws.EnableCalculation = False
    ws.EnableCalculation = True
    On Error GoTo 0
End Sub

and then run the BenchmarkSheets method. For this to work you need the ExcelAppState class which you should already have and you need to make sure that all worksheets are unprotected. This will show you how much each sheet takes to calculate in isolation so that you can start improving with the slowest. The total will of course be distorted because calculating the whole book at once is different. Regardless, it will give you a good idea.

I'm surprised that the built in functions are worse than custom code.

Not always. You must test each one and see if you can actually make improvements. I simply use this approach:

Sub TestSpeed()
    Const iterations As Long = 1000000 'I usually start with 1000 and increase as needed until the difference is visible
    Dim t As Double
    Dim i As Long
    Dim res As Double 'Or whatever data type is needed
    '
    t = Timer
    For i = 1 To iterations
        res = Method1() 'Whatever the name is and arguments are needed
    Next i
    Debug.Print Round(Timer - t, 3)
    '
    t = Timer
    For i = 1 To iterations
        res = Method2() 'Whatever the name is and arguments are needed
    Next i
    Debug.Print Round(Timer - t, 3)
End Sub

which gives a good indication which one is faster between Method1 and Method2.

That's something I believe will be very useful and fast, but need to get my head around it. Doing VBA coding for the first time :). I guess I'll need a sub for that instead of a function.

You still need a Function to return values. Maybe this post that I wrote a while ago will help. Sub is basically DoSomething while Function is like GetSomething.

Working with arrays is not complicated at all. Here is a repo that I wrote a long time ago to deal with array manipulation. Basically, the input and output of your functions could change from simple values to full arrays and that's a lot faster than calling a UDF for each cell.