Leaking unit groups.

This little problem disturbs me greatly, I decided to measure it and used the following trigger.
Okay, it creats 10 Unit Groups every second, inside the CONDITIONS which is the last place a newbie would except rampant memory leaks.

I placed 100 units on the map, then fired it up and took measurements of WC3 Memory Footprint:
2 Minute: 123K
4 Minute: 160K
6 Minute: 195K

Another trigger confirmed that "Leaky" was running 600 times a minute. For this map, the leak rate was about 245K/minute. Which is exactly what one would expect:
600 Unit Groups created per minute, each contains 100 pointers to units, each pointer being 4 bytes.
600*100*4 / 1024 = 234K
Then ofcourse a little unitgroup infrastructure and "natural" leaking to bring the total up a bit.


My map was a very extreme example, but it also leaked extreme amounts of memory over a very short period of time. Imagine a RPG, creating lots of unit groups, running for 3 hours. I admit that my (and any) system with 512K ram would basically just shrug it off, but an older machine with 128K ram would be begging for mercy (and crippled when quitting), and an average machine with 256K ram would be feeling the pinch.

I will be rewriting much of my Open RPG triggers to destroy the most commonly created unit groups, unfortunate, but it seems like the right thing to do.

This is actually a pretty dumb bug from Blizzard's side. They should have picked up the lose group and destroyed them (aka. Garbage collector).
Good job finding out, this is a great factor in reducing lag, actually.

Well, the thing is, a garbage collector would probably slow performance somewhat, which is one of the reasons C++ is faster than Java(other than the whole virtual machine thing). Java has a collector thread always running in the background, and even though it's low priority, it's still taking processor time. It's up to the programmer to clean up, unfortunately.

I know a more efficient solution.
Basically it has to do with the blizzard.j global bj_wantDestroyGroup. Convert your trigger to Jass and add the following line in front of each condition (in front of the code, that checks whether there is a specific amount of units in the group):

This will remove the leaks if you use the group functions from blizzard.j (the ones the WE uses).

Another solution might be that you store each group to a local var and destroy it afterwards. You'd need only 1 local var (I'm not sure on this).

I think that only worked with ForGroupBJ. ForGroupBJ doesn't return a group so it can safely destroy it after use. The other functions can't both return the group and destroy at the same time.

Well I only knew about the problem thanks to your thread Starcraft Freak 8))

Ah so that "set bj_wantDestroyGroup = true" thing works for conditions too. I understood it working for those "pick every unit" thing, but I thought it would mess up for something like 'Unitgroup is empty" but I see from blizzard.j that the use of local variables makes that a non issue.

Unfortunately converting triggers to custom script is often a no-no for me because the GUI just makes some things so much easier. I'll probably just move such conditions into the actions.

You do not actually need to set that bj variable before every condition. Just set it once and leave it. But then you have to be careful that you don't call ForGroupBJ with a group that you don't actually want destroyed.

Since there are no real pointers in JASS (you cannot have linked lists or similar things) it would have been easy for Blizzard to implement an automatic removal of unaccessable objects. A garbage collection would be overkill without real pointers but reference counting is easy to implement in an interpreted language, very efficient and without real pointers there are no possibilities for it to fail. The cost in both memory and performance would have been negligible. Too bad Blizzard didn't do that.

Actually there are a lot more functions that have that behaviour implemented:
IsUnitGroupDeadBJ
IsUnitGroupEmptyBJ
ForGroupBJ
GroupAddGroup
GroupRemoveGroup
GroupPickRandomUnit
CountUnitsInGroup <- thats the function he uses

Also note you DO have to set it everytime before a forgroup, because it resets it to false :)

Now this is fun.

Interestingly enough this code:
Set GroupA = (Last created unit group)
Set GroupB = GroupA
Is quite different to this code:
Set GroupA = (Last created unit group)
Set GroupB = (Last created unit group)

Because Group variables are pointers and (Last created unit group) returns a copy of the last created unit group. So in the first case, you get two pointers to the same group, and in the second case, you get two pointers to two different groups (with identical contents). Would a novice really expect that behaivour?

It seems to be unreasonably difficult to get a genuine copy of a unit group in the GUI, involving something hairbrained like
Set GroupB = (Random (Number of units in GroupA) units from GroupA)
Ofcourse if you know for sure that GroupB is pointing to an absolutely unique unit group, then you can do something simpler like "Add all units from GroupA to GroupB"

So it seems that if the maper wishes to retain sanity then it would be a good idea to avoid
Set variableB = variableA
at all costs, and when it is useful to do such a thing "variableB" must be treated as const as to avoid changing variableA

I learnt that lesson the painful way
I THOUGHT I was cleaning up a memory leak, but was ACTUALLY filling my array with NULLS. And it took me a shamefully long time to figure out the problem.

edit: Heres more ghastly little leaks!

C ((Entering unit) is in (Last created unit group)) Equal to True
A Unit Group - Pick every unit in (Last created unit group) and do (Do nothing)

All this is exactly why you should code in jass, not the GUI. The GUI hides a lot of memory leaks behind a good looking tree view. Try and learn to code in jass, it will make your mapping life easier in the end.