Loop within a loop causing memory leakage

[dmbull]

The following code results in a huge jump in memory (that isn't released once its run) even though there is only 2 Integer variables and 1 String variable. Its a simplified version of some 'real code' which makes VBA run out of memory and crash. Any ideas what's going on and how to resolve?

Sub test()
Dim Loop1_Count As Integer
Dim Loop2_Count As Integer
Dim A As String

Loop1_Count = 1
Do While Loop1_Count <= 1000
Loop2_Count = 1
Do While Loop2_Count <= 1000
A = Workbooks("Test.xlsm").Worksheets("Test").Cells(Loop2_Count, "A")
Loop2_Count = Loop2_Count + 1
Loop
Loop1_Count = Loop1_Count + 1
Loop

End Sub
<strike></strike>


The above code lives in Test.xlsm.
"Test" is a worksheet in Test.xlsm and contains only blank cells

 

[JackDanIce]

Welcome to the board. The code you've posted works fine for me, without error. You could try declaring Loop1_Count and Loop2_Count as Long types.

 

[dmbull]

Welcome to the board. The code you've posted works fine for me, without error. You could try declaring Loop1_Count and Loop2_Count as Long types.

Thanks for looking.

The code "works" for me in the sense that it comes to an end and doesn't crash. The issue for me is that it eats up way more memory than it should, and even more importantly, it doesn't free up the memory when its done. I can see this by watching Excel on Task Manager.

I can make it crash easy enough by increasing the loops to 2000.

Hope that makes more sense?

Interestingly, I've subsequently solved the memory leak in the "real code" by saving the sheet I'm pulling data from in a 2D array first and then access the array rather than the sheet in the nested loop.

 

[JackDanIce]

You're posting a very simple piece of code, which pre Excel 2003 could handle, when most people were running Windows 98 or other.

In otherwords, I don't think, the code alone you have posted is the problem, regardless of what Task Manager is showing.

a variable is declared as string type. When complied, the code will allocate a specific amount of memory to store a string data type. You are then changing/updating this value 1,000,000 times with your nested loop based on column A (A1:A1000) in worksheet Test, but the amount of memory allocated to variable a is fixed, it's just the contents of a that is replaced on each loop.

Think of a as a blackboard that you are continuously writing over, the size of the board doesn't change, no matter what you write on it or the number of times you do. The only other guess I have is you have a value that exceeds the size a can hold.

If there is "real code", I suspect there are other aspects causing the problem.

 

[RickXL]

Hi,

I am with JackDanIce on this. I can run the code in 64 bit Windows 2010 and Excel 2013 and it runs without problem even if I increase the loops to 2000.

I should point out that Task Manager can be misleading. Just because it leaves the memory allocated does not mean that it is leaking. The real question is when another application needs it does it then make it available. Windows does not "tidy up" unless it feels it needs to.

I did get a "Not Responding" message for part of the "2000 run" though. You need to consider how Excel works. NB This is a "RickXL model" and is not necessarily how it actually works.

1. The worksheet part of Excel was written first by one team of programmers.
2. There were macros prior to VBA but VBA itself only appeared in Excel 5.

The result is that VBA and the worksheet side of Excel are not integrated as well as they could be. When worksheet data is required by VBA I have visions of it sending a truck to make a pick-up. So, for each of your calls to a worksheet cell you are setting up a delivery - 4 million of them, in fact. Each delivery takes time and so can make the system unresponsive. Presumably, those 4 million trucks are causing traffic jams somewhere along the way.

If you get all the data using a VBA array you need only one delivery. You can get 4 million cells in a single truck. When that data is completely in VBA then VBA can process it very quickly.