This is so slow I have to run it overnight

[TheWennerWoman]

Hello,

I have inherited a file that needs to be run overnight as it ties up Excel and takes hours. The relevant piece of code is below, is there anything obvious that leaps out at you gurus as being bad practice and that can be replaced.......anything that gives a speed improvement would be really appreciated.

lrow = Sheet1.Cells(Sheet1.Rows.Count, "B").End(xlUp).Row
x = Sheet1.Range("N" & lrow).Value

For y = x To 1 Step -1

lrow = Sheet1.Cells(Sheet1.Rows.Count, "B").End(xlUp).Row

    Do Until y <> Sheet1.Range("N" & lrow).Value
    Sheet1.Rows(lrow).Copy
    Sheet2.Cells(Sheet2.Rows.Count, "A").End(xlUp).Offset(1, 0).PasteSpecial xlPasteValues
    Sheet1.Rows(lrow).EntireRow.Delete
    lrow = Sheet1.Cells(Sheet1.Rows.Count, "B").End(xlUp).Row
    Loop

    lrow1 = Sheet2.Cells(Sheet2.Rows.Count, "B").End(xlUp).Row
    Sheet2.Range("O1:T1").Copy Sheet2.Range("O3:T" & lrow1)
    Sheet2.Range("O3:T" & lrow1).Copy
    Sheet2.Range("O3:T" & lrow1).PasteSpecial xlPasteValues
    z = Application.WorksheetFunction.Sum(Sheet2.Range("Q:Q"))
    If z = 1 Then
        For i = 3 To lrow1
        If Sheet2.Range("Q" & i).Value = 0 Then
        Sheet2.Rows(i).Cut Sheet3.Cells(Sheet3.Rows.Count, "A").End(xlUp).Offset(1, 0)
        Else
        Sheet2.Rows(i).Cut Sheet5.Cells(Sheet5.Rows.Count, "A").End(xlUp).Offset(1, 0)
        End If
        Next i
    Else
    Sheet2.Rows("3:" & lrow1).Cut Sheet4.Cells(Sheet4.Rows.Count, "A").End(xlUp).Offset(1, 0)
    End If
        
        
Application.StatusBar = "Progress: " & y & " remaining. " & Format(y / x, "0%")

Next y

Thanks in advance.

 

[RoryA]

The whole thing looks painful to me. Lots of looping and copying/cutting single rows at a time. Plus updating the status bar adds to the runtime. Hard to tell in isolation why it's written how it is, or whether there is any optimization in place at all, such as turning off screenupdating and events.

 

[TheWennerWoman]

The whole thing looks painful to me. Lots of looping and copying/cutting single rows at a time. Plus updating the status bar adds to the runtime. Hard to tell in isolation why it's written how it is, or whether there is any optimization in place at all, such as turning off screenupdating and events.

Anyone else? Otherwise I'll concede defeat.

 

[Michael M]

How many rows are you looking at processing ?
Try using

Application.screenupdating = false

at the start of the code and

Application.screenupdating =True

at the end of the code
Get rid of the status bar as a test for time usage.
Also look at using Autofilter to copy data in a block rather than line by line.
And this section, probably going to be a killer.

 If z = 1 Then
        For i = 3 To lrow1

By cutting rows from the top down rather than the bottom up, means every time you cut a row, the entire worksheet has to move up 1 row, no big deal if there are only a few rows, but a real problem if there are thousands.
Also look at where lrow takes you to....if it's waaay down the worksheet, that could also extend the process greatly

 

[kevin9999]

It would help to see the full code, as well as a copy of your data using the XL2BB add in, or share a desensitised version of your file via Google Drive, Dropbox or similar file sharing platform.