Why null local variables?

[D4RK_G4ND4LF]

My pc ran the following code from 13:10 to 17:40
Memory load of wc3 at start: 98.200 K
..and at the end: 98.880 K
Number of created local unit variables: 500.000.000
My pc had a cpu load of 20% while running this code btw so it is unlikely that anyone will ever make more local unit variables in any game

I used GetEnumUnit() instead of setting the local variable to a global variable just in case that it would somehow not work correctly
This should make sure the units are different enough though
there were roughly 100 units in the map, 2 of them heros

all in all I don't think that not nulling local variables can cause any trouble except with H2I/I2H which is not used any more

if someone has an idea why to null local variables please tell me

globals
    integer i = 0//count local handles
endglobals

function leak takes nothing returns nothing
    local unit u  = GetEnumUnit()
    if u != null then//to make sure that there is no bug
        set i = i + 1
    endif
    //no "set u = null" :P
endfunction

function leakLoop takes nothing returns nothing
    call SetPlayerState(Player(0), PLAYER_STATE_RESOURCE_GOLD, i)//so we actually see that it works but since goldlimit is at 1 million I made a debug msg which displays I2S(i) when pressing esc
    call ForGroup(udg_g, function leak)
endfunction

//===========================================================================
function InitTrig_Leak takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterTimerEvent(t, 0.0025, true)
    call TriggerAddAction(t, function leakLoop)
endfunction

 

[Pharaoh_]

Local variables must be nulled, when they contain agent-type pointers. All variables reserve memory, be it globals or locals. Globals reserve it in the map initialization though. The nulling of course should happen before the return of the function.

 

[Anachron]

It is really important, as Pharaoh_ already pointed out.
They refer to an index entry and you should destroy those pointers.

 

[Bribe]

There's no handle leak because you were not generating 500,000,000 units, so the pointers were never going out of bounds. That many pointers on one unit just reserves one handle, so no matter how many things are memorizing his index it's still the same infinitesimal leak. A proper test would be

function leak takes nothing returns nothing
    local unit u=CreateUnit(Player(0),'hfoo',0.,0.,0.)
    call RemoveUnit(u)
endfunction

 

[D4RK_G4ND4LF]

Local variables must be nulled, when they contain agent-type pointers. All variables reserve memory, be it globals or locals. Globals reserve it in the map initialization though. The nulling of course should happen before the return of the function.

so why does wc3 not require more memory?

There's no handle leak because you were not generating 500,000,000 units, so the pointers were never going out of bounds. That many pointers on one unit just reserves one handle, so no matter how many things are memorizing his index it's still the same infinitesimal leak. A proper test would be

function leak takes nothing returns nothing
    local unit u=CreateUnit(Player(0),'hfoo',0.,0.,0.)
    call RemoveUnit(u)
endfunction

so every unit can only leak once?
if that was true this would be like nothing
and creating a unit leaks anyway no matter if u use a global or local variable
gonna try this out with locations
maybe it will make a difference
(tried it with unitgroups already and they leak, no matter if destroyed or nulled or both)

edit: seems like the following code really requires "set l = null" to prevent it from leaking which would prove what you said

function leak takes nothing returns nothing
    local location l = Location(0.00, 0.00)
    call RemoveLocation(l)
endfunction

edit2: same with unit leaks in both cases (with and without setting u = null)
also call ShowUnit(u, false) does not make a difference (most likely because it is removed instantly anyway)

edit3: rects require nulling, too, unless one uses globals

but if local unit variables only leak once it would not really make any difference because creating a unit leaks a lot more I think

 

[PurgeandFire]

Yep, it has been a known issue in 1.24. For some reason, RemoveUnit() doesn't completely clear the memory. Even in simple melee matches, people have noticed a difference.

 

[Berb]

(tried it with unitgroups already and they leak, no matter if destroyed or nulled or both)

That's because unit-groups always leak. There's a reason why there is a GroupUtils library for this stuff.

but if local unit variables only leak once it would not really make any difference because creating a unit leaks a lot more I think

Try the following code:

local integer i=0
local unit array u
local unit array u2
local unit array u3
local unit array u4
loop
    exitwhen(i==600)
    set u[i]  = CreateUnit(Player(0), 'hfoo', 0, 0, 0) 
    set u2[i] = CreateUnit(Player(0), 'hfoo', 0, 0, 0)
    set u3[i] = CreateUnit(Player(0), 'hfoo', 0, 0, 0)
    set u4[i] = CreateUnit(Player(0), 'hfoo', 0, 0, 0)
    call RemoveUnit(u[i]) 
    call RemoveUnit(u2[i])
    call RemoveUnit(u3[i])
    call RemoveUnit(u4[i])
    set i=i+1
endloop

Before this code is executed my memory usage in War3 is 91,912 KB.

After this code is executed my memory usage in War3 is 144,156 KB.

I'm running the test again except this time I'm setting the array elements to null.

Start: 92,540 KB
End: 143,144 KB

That's weird. Why is the memory still being leaked?

 

[D4RK_G4ND4LF]

That's because unit-groups always leak. There's a reason why there is a GroupUtils library for this stuff.

yeah I know but it's nice to prove this myself

That's weird. Why is the memory still being leaked?

creating a unit leaks anyway no matter if u use a global or local variable

probably same as wit groups or special effects
too complex for blizzard to destroy every leak

 

[Berb]

Oh, I didn't see that. Thanks.

Yea, other types don't leak.

And how hard could it honestly be... writing clean code.

 

[SlayerII]

wait.. Does this mean,that i am forced to dummy recycling? I use quite alot dumm yunits in my map, and currently im only removing them =/

 

[Anachron]

Yes, recycling is a good thing.

 

[ChaosKnight]

Globals. Dah.

 

[PurgeandFire]

Yes. Unit leaks have become major since 1.24. RemoveUnit() doesn't clear all the memory, so now it is best to recycle. The leak was noticed even when people were playing melee. However, this doesn't mean that an average 3,000 unit map is doomed for life, it just means it is best to recycle dummies rather than kill/remove them. Some spells use like 20+ dummies per cast, and that adds up to 2,000+ after 100 casts. With recycling, you can keep it at around 60 dummy units (depending on how many casts were made at a time).

 

[Berb]

In most cases you only need 1 dummy for many dummy abilities. The biggest problem arises when you're planning on not having instant effects with your dummies. For example, if your dummy is supposed to walk somewhere before casting an ability then you're going to need multiple units, all of which should be recycled to ensure that memory is not unnecessarily leaked.

 

[Berb]

In most cases you only need 1 dummy for many dummy abilities. The biggest problem arises when you're planning on not having instant effects with your dummies. For example, if your dummy is supposed to walk somewhere before casting an ability then you're going to need multiple units, all of which should be recycled to ensure that memory is not unnecessarily leaked.

 

[Deaod]

Why null them? Because otherwise, youll be leaking handle IDs, which could be deadly to a bunch of libraries (such as TimerUtils, under certain conditions).