Theory - What is faster?

[StRoNgFoE_2000]

For how serious an issue this group leakage could become in some maps, it's surprising how little information I can find on it. I need to get some things cleared up, I'll try to piece together what little bit of the scattered information I found, and hope that someone verifies my questions.

1.) So, creating a group, doing any enum on it (not a null boolexpr), then destroying it, will leak the group?

2.) Enumerating a group that has a null boolexpr, will not leak when destroying the group, but the null boolexpr will leak?

3.) To avoid number 2, the actions for the group can be done in the boolexpr callback, then return false to avoid the leak?

4.) What if it returns true? In the case I need one random unit from a group, it has to return true (I think?). Is true the cause of the leak?

5.) And the final brain stumper, the one that is bugging me the most. Suppose I just want to use a group to get one unit (for finding a unit near a unit), but this group has to be multi-instanceable, and it cannot leak, and doing the actions in the callback fails because I only want one unit, then how would I go about this? The boolexpr has to return true to get the units for the group to pick from.

I'll try to think of an example for this.

JASS:

// An impractical example, but the idea is to understand what I mean. Using a global group for
// a spell can be troublesome if there are waits or some other reason the global group could be
// overwritten. The only thing I can think of is to use a group array.

    // The problem here is, I only want one unit from the group, a random unit to do actions
    // on. I cannot do the actions in the boolexpr callback, because I only want one random
    // unit from the group, and I don't want the actions done to the entire group. If another
    // player calls this trigger, the global group would be overwritten.

globals
    private group    g_group
    private boolexpr g_bool
    private player   g_player
endglobals        
function GroupFilter takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(), g_player) // Boolexpr is returning true, is there a leak here?
endfunction
function DoSomeActions takes nothing returns nothing
    local unit c = GetTriggerUnit()
    local unit u
    local real x = GetUnitX(c)
    local real y = GetUnitY(c)
    set g_player = GetTriggerPlayer()
    call GroupEnumUnitsInRange(g_group, x, y, 500, g_bool)
    if CountUnitsInGroup(g_group) > 0 then
        set u = GroupPickRandomUnit(g_group)              
        call TriggerSleepAction(10)  // g_group cannot be overwritten, so what method do I use? Array?
        call DoSomeActionsToUnit(u)
        call GroupClear(g_group)
    endif
    set c = null
    set u = null
endfunction
function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    set g_group = CreateGroup()
    set g_bool  = Condition(function GroupFilter)
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddAction(t, function DoSomeActions)
endfunction

// Simply put, I just want to find a unit near x and y to do some actions to. I know this cannot
// be done without using a group, so in this case, if I use a global group, do I have to use an
// array per player?

// This seems to be the only logical answer I can come up with.

globals
    private array group    g_group
    private boolexpr       g_bool
    private player         g_player
endglobals        
function GroupFilter takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(), g_player) // Returning true to get the units for the group. Is this bad?
endfunction
function DoSomeActions takes nothing returns nothing
    local unit c = GetTriggerUnit()
    local unit u
    local real x = GetUnitX(c)
    local real y = GetUnitY(c)
    local integer i
    set g_player = GetTriggerPlayer()
    set i = GetPlayerId(g_player)
    call GroupEnumUnitsInRange(g_group[i], x, y, 500, g_bool)
    if CountUnitsInGroup(g_group[i]) > 0 then
        set u = GroupPickRandomUnit(g_group[i])              
        call TriggerSleepAction(10)  // g_group[i] is safe because it's per player.
        call DoSomeActionsToUnit(u)
        call GroupClear(g_group[i])
    endif
    set c = null
    set u = null
endfunction
function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    local integer i = 0
    loop
        exitwhen i > 11
        set g_group[i] = CreateGroup()
        set i = i + 1
    endloop
    set g_bool = Condition(function GroupFilter)
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddAction(t, function DoSomeActions)
endfunction



//==================================================================
//==================================================================
// Now, if I were doing the actions to the entire group, I understand this would
// not have the same issue, since I can use the callback for my actions and return
// false to avoid the leak. I understand this much, it's the above block of code 
// that is stumping me.

globals
    private group    g_group   // Never in a wait so overwrite doesn't matter.
    private boolexpr g_bool
    private player   g_player
endglobals    
function GroupFilter takes nothing returns boolean
    local unit u = GetFilterUnit()
    if IsUnitEnemy(u, g_player) then
        call DoSomeActionsToUnit(u)
    endif
    set u = null
    return false    
endfunction
function DoSomeActions takes nothing returns nothing
    local unit c = GetTriggerUnit()
    local real x = GetUnitX(c)
    local real y = GetUnitY(c)
    set g_player = GetTriggerPlayer()
    call GroupEnumUnitsInRange(g_group, x, y, 500, g_bool)
    call GroupClear(g_group)
    set c = null
endfunction
function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    set g_group = CreateGroup()
    set g_bool  = Condition(function GroupFilter)
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddAction(t, function DoSomeActions)
endfunction

I'm sorry for all the questions but I'm really stumped on this. I'm not one to easily understand some things, unless the description is made very clear.

[Captain Griffen]

1.) So, creating a group, doing any enum on it (not a null boolexpr), then destroying it, will leak the group?

No. Create group - enum it at all - destroy it will cause a memory leak - but not the group itself.

The leak size is related only to the number of groups you do this to, not to the number of times you enum the group before destroying it.

2.) Enumerating a group that has a null boolexpr, will not leak when destroying the group, but the null boolexpr will leak?

No, it will still leak when destroying the group, but will have an additional, larger leak, each and EVERY time you enum with a null boolexpr. If you are using a physics system, this can very rapidly make the map unplayable.

3.) To avoid number 2, the actions for the group can be done in the boolexpr callback, then return false to avoid the leak?

No, that's just a method of boosting efficiency and using just one group for all code that can be done like that, which avoids 1, as you never destroy the group.

4.) What if it returns true? In the case I need one random unit from a group, it has to return true (I think?). Is true the cause of the leak?

Then it can return true and then do a ForGroup (unless you know before hand the number of units you're enuming). But then you can clear the group or whatever, just don't destroy the group.

5.) And the final brain stumper, the one that is bugging me the most. Suppose I just want to use a group to get one unit (for finding a unit near a unit), but this group has to be multi-instanceable, and it cannot leak, and doing the actions in the callback fails because I only want one unit, then how would I go about this? The boolexpr has to return true to get the units for the group to pick from.

See above, returning true in and of itself does not leak...

Oh, and plus, if a unit decays/is removed while it's part of a group, not only does it cause a slight leak, it also polutes the hash table with shadows, which can cause massive degredation to the performance of most group functions (like, few thousand shadows, which is quite doable in some usages, lead to ~50 times reduction in speed if I remember correctly).

GroupClear solves it, though, which is fortunate - something to bear in mind. (I made a GroupRefresh script, go find it for more details on this problem.)

[StRoNgFoE_2000]

Thank you, that last post was VERY helpful. I can honestly say I fully understand the leaks now and what to do (and what not to do) from now on. The only problem is going back and recoding everything heh.

[Anitarf]

Getting a random unit from an area is fairly easy with just a single enum, without actually adding the units to the group; not that it matters leak-wise since as Griffen pointed out returning true or false has nothing to do with it, the only reason we avoid returning true is because this way the code is faster than if we first populated the group with units only to then immediately loop through it again and remove those units from it.

[StRoNgFoE_2000]

Just want to make sure I'm on the right track here. I dug out a spell and decided to recode it with everything that I learned so far. The spell simply shackles all air units for 15 seconds, at a rate of one unit every .035 seconds, to avoid the instant cast on every unit lag.

The only thing that bugs me is I use a group array, because if another player casted while the timer is repeating it would overwrite the group if it wasn't an array. If array is avoidable I cannot think of a method myself. The benefit is, the boolexprs and groups are global and never destroyed so they are being recycled and groups being cleared when struct is destroyed. Just need an evaluation here, is it possible to improve this code, or is this ok as is?

And to avoid question, the rect 'EntireMap' is a global I use that is simply 'bj_mapInitialPlayableArea'. It's a lot easier to remember and to type.

JASS:

scope AirShackle initializer Init

globals
    private boolexpr ShackleTeam1
    private boolexpr ShackleTeam2
    private group array ShackleGroup
endglobals

private struct shackle
    real     x  = 0
    real     y  = 0
    integer  i  = 0
    unit     u  = null
    unit     v  = null
    timer    t  = null
    player   p  = null
    static method create takes nothing returns shackle
        local shackle data = shackle.allocate()
        set data.t = NewTimer()
        set data.u = GetTriggerUnit()
        set data.p = GetTriggerPlayer()
        set data.x = GetUnitX(data.u)
        set data.y = GetUnitY(data.u)
        set data.i = GetPlayerId(data.p)
        return data
    endmethod    
    method onDestroy takes nothing returns nothing
        call ReleaseTimer(.t)
        call GroupClear(ShackleGroup[.i])
    endmethod
endstruct

private function ShackleCond takes nothing returns boolean
    return GetSpellAbilityId() == 'A030'
endfunction

private function FilterTop takes nothing returns boolean
    return IsUnitType(GetFilterUnit(), UNIT_TYPE_FLYING) == true and IsUnitInForce(GetFilterUnit(), EnemiesTop) and IsUnitType(GetFilterUnit(), UNIT_TYPE_DEAD) == false
endfunction
private function FilterBot takes nothing returns boolean
    return IsUnitType(GetFilterUnit(), UNIT_TYPE_FLYING) == true and IsUnitInForce(GetFilterUnit(), EnemiesBottom) and IsUnitType(GetFilterUnit(), UNIT_TYPE_DEAD) == false
endfunction

private function AirShackleExecute takes nothing returns nothing
    local timer t      = GetExpiredTimer()
    local shackle data = GetTimerData(t)
    if CountUnitsInGroup(ShackleGroup[data.i]) > 0 then
        set data.u = FirstOfGroup(ShackleGroup[data.i])
        set data.v = CreateUnit(data.p, 'h062', data.x, data.y, 270)
        call UnitAddAbility(data.v, 'A031')
        call IssueTargetOrder(data.v, "magicleash", data.u)
        call UnitApplyTimedLife(data.v, 'BTLF', 15)
        call GroupRemoveUnit(ShackleGroup[data.i], data.u)
    else
        call data.destroy()
    endif
    set t = null
endfunction
private function ShackleStart takes nothing returns nothing
    local shackle data = shackle.create()
    if IsPlayerInForce(data.p, Force1) then
        call GroupEnumUnitsInRect(ShackleGroup[data.i], EntireMap, ShackleTeam1)
    else
        call GroupEnumUnitsInRect(ShackleGroup[data.i], EntireMap, ShackleTeam2)
    endif
    call SetTimerData(data.t, data)
    call TimerStart(data.t, .035, true, function AirShackleExecute)
endfunction

private function Init takes nothing returns nothing
    local integer i = 0
    local trigger t = CreateTrigger()
    set ShackleTeam1 = Condition(function FilterTop)
    set ShackleTeam2 = Condition(function FilterBot)
    loop
        exitwhen i > 5
        set ShackleGroup[i] = CreateGroup()
        call TriggerRegisterPlayerUnitEvent(t, Player(i), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
        set i = i + 1
    endloop
    call TriggerAddCondition(t, Condition(function ShackleCond))
    call TriggerAddAction(t, function ShackleStart)
    set t = null
endfunction
endscope

[Anitarf]

You're doing it wrong, you're limiting a perfectly MUI spell code to one instance per player. Since structs are simply parallel global arrays with indexes getting reused all the time you can just have one group per struct instance without needing to destroy them.

JASS:

struct foo
    group bar

    static method create takes nothing returns foo
        local foo f = foo.allocate()
        if f.bar == null then
            set f.bar = CreateGroup()
        endif
        return f
    endmethod

    method onDestroy takes nothing returns nothing
        call GroupClear( .bar )
    endmethod
endstruct

[StRoNgFoE_2000]

Ah that makes total sense to me now, and cleared up mostly all confusion I had on structs. I knew how to type em up slightly based on examples and readings, but never really knew how they 100% worked. So I'm assuming this also goes for boolexprs, meaning they don't have to be destroyed since they are going to be reused eventually and overwritten with a new value? I could be wrong I've been many times so far, but I am determined to get all this down.


Revamped it.

JASS:

scope AirShackle initializer Init
private struct shackle
    timer    t
    unit     u
    unit     v
    player   p
    real     x
    real     y
    boolexpr b
    group    g
    private static method filtertop takes nothing returns boolean
        return IsUnitType(GetFilterUnit(), UNIT_TYPE_FLYING) == true and IsUnitInForce(GetFilterUnit(), EnemiesTop) and IsUnitType(GetFilterUnit(), UNIT_TYPE_DEAD) == false
    endmethod
    private static method filterbot takes nothing returns boolean
        return IsUnitType(GetFilterUnit(), UNIT_TYPE_FLYING) == true and IsUnitInForce(GetFilterUnit(), EnemiesBottom) and IsUnitType(GetFilterUnit(), UNIT_TYPE_DEAD) == false
    endmethod
    static method create takes nothing returns shackle
        local shackle data = shackle.allocate()
        set data.t = NewTimer()
        set data.u = GetTriggerUnit()
        set data.p = GetTriggerPlayer()
        set data.x = GetUnitX(data.u)
        set data.y = GetUnitY(data.u)
        if data.g == null then
            set data.g = CreateGroup()
        endif
        if IsPlayerInForce(data.p, Force1) then
            set data.b = Condition(function shackle.filtertop)
        else
            set data.b = Condition(function shackle.filterbot)
        endif
        call GroupEnumUnitsInRect(data.g, EntireMap, data.b)
        return data
    endmethod    
    method onDestroy takes nothing returns nothing
        call ReleaseTimer(.t)
        call GroupClear(.g)
    endmethod
endstruct
private function ShackleCond takes nothing returns boolean
    return GetSpellAbilityId() == 'A030'
endfunction
private function AirShackleExecute takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local shackle data = GetTimerData(t)
    if CountUnitsInGroup(data.g) > 0 then
        set data.u = FirstOfGroup(data.g)
        set data.v = CreateUnit(data.p, 'h062', data.x, data.y, 270)
        call UnitAddAbility(data.v, 'A031')
        call IssueTargetOrder(data.v, "magicleash", data.u)
        call UnitApplyTimedLife(data.v, 'BTLF', 15)
        call GroupRemoveUnit(data.g, data.u)
    else
        call data.destroy()
    endif
    set t = null
endfunction
private function ShackleStart takes nothing returns nothing
    local shackle data = shackle.create()
    call SetTimerData(data.t, data)
    call TimerStart(data.t, .035, true, function AirShackleExecute)
endfunction
private function Init takes nothing returns nothing
    local integer i = 0
    local trigger t = CreateTrigger()
    loop
        exitwhen i > 5
        call TriggerRegisterPlayerUnitEvent(t, Player(i), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
        set i = i + 1
    endloop
    call TriggerAddCondition(t, Condition(function ShackleCond))
    call TriggerAddAction(t, function ShackleStart)
    set t = null
endfunction
endscope

[xombie]

So, I know this is a little bit old, but I was just looking through some group behavior. Anyways... what I got from this is that this following code would leak:

JASS:

local group someGroup = CreateGroup()
call GroupEnumUnitsInRange(someGroup, ....)
call DestroyGroup(someGroup)
set someGroup = null

Would this leak?

[Captain Griffen]

Yes.

[MaD[Lion]]

id go with detect attack ^^

[xombie]

So then is there a way to avoid leaks with groups or they always leak?

[Captain Griffen]

...

Read the thread.

[xombie]

Thanks Captain, because I didn't already read the thread. You're a life-saver. /sarcasm off

[Bobo_The_Kodo]

Xombie, get GroupUtils from the resource section or make up your own thing for group recycling.

[Vexorian]

All right I deleted some worthless posts, try sticking to the topic, this group giberish is important, I don't pay you for fighting, k? Edit: Unless the fight is actually about whose system is best, then please fight. Hmnn.