[JASS] Which could be more efficient/better performance?

[8uY_YoU]

Hello guys,
I have problems when people say my map is kinda laggy
As far as I know my code is leak free. However, I noticed that I put too much system inside which gives a lot pressure to the CPU (especially periodic calls + groupenums calls).
So, before I refine my codes, I need some suggestion

I will give examples from each category and you guys just point out, which is more efficient method to use and give better performance.
If there is another better method that I didn't know or some of my methods are wrong, tell me!

1. Periodic Calls

a. Using Timer

function RunThis takes nothing returns nothing
    // code here
endfunction

function InitTrig_test takes nothing returns nothing
    local timer t = CreateTimer(  )
    call TimerStart(t,0.25,true,function RunThis)
endfunction

b. Using Trigger

function RunThis takes nothing returns boolean
    // code here
    return false
endfunction

function InitTrig_test takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerRegisterTimerEvent(t,0.25,true)
    call TriggerAddCondition(t,Condition(function RunThis))
endfunction

2. Group Execution

a. Blizz-way (Filtering then Execute)

function GroupExecuteUnits takes nothing returns nothing
    //execute each valid unit
endfunction

function GroupFilter takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(GetTriggerUnit()))
endfunction

function GroupExec takes nothing returns nothing
    local group g = CreateGroup()
    local unit c = GetTriggerUnit()
    local real x = GetUnitX(c)
    local real y = GetUnitY(c)
    call GroupEnumUnitsInRange(g,x,y,300,Filter(function GroupFilter))
    call ForGroup(g, function GroupExecuteUnits)
    call DestroyGroup(g)
    set g = null
    set c = null
endfunction

b. In one function way

function GroupExec takes nothing returns nothing
    local group g = CreateGroup()
    local unit c = GetTriggerUnit()
    local real x = GetUnitX(c)
    local real y = GetUnitY(c)
    local unit temp
    call GroupEnumUnitsInRange(g,x,y,300,null)
    loop
        set temp = FirstOfGroup(g)
    exitwhen temp == null
        if IsUnitEnemy(temp,GetOwningPlayer(c)) then
            //execute each valid unit
        endif
        call GroupRemoveUnit(g,temp)
    endloop
    call DestroyGroup(g)
    set g = null
    set c = null
endfunction

c. What-i-am-currently-using way

function GroupFilter takes nothing returns boolean
    if IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(GetTriggerUnit())) then
        //execute each valid unit
    endif
    return false
endfunction

function GroupExec takes nothing returns nothing
    local group g = CreateGroup()
    local unit c = GetTriggerUnit()
    local real x = GetUnitX(c)
    local real y = GetUnitY(c)
    local filterfunc ff = Filter(function GroupFilter)
    call GroupEnumUnitsInRange(g,x,y,300,ff)
    call DestroyGroup(g)
    call DestroyFilter(ff)
    set ff = null
    set g = null
    set c = null
endfunction

3. Multi-Instancing

a. Indexing (in this case, one timer for all instances)

function Execute takes nothing returns nothing
    local integer i = 1
    loop
    exitwhen i > udg_INDEX
        set udg_Durations[i] = udg_Durations[i] - TimerGetTimeout(udg_Time)
        if udg_Durations[i] > 0 then
            //do a heal/damage to udg_Units[i]
        else
            if i != udg_INDEX then
                set udg_Durations[i] = udg_Durations[udg_INDEX]
                set udg_Units[i] = udg_Units[udg_INDEX]
            endif
            set udg_INDEX = udg_INDEX - 1
            set i = i - 1
        endif
        set i = i + 1
    endloop

    if udg_INDEX == 0 then
        call PauseTimer(udg_Time)
    endif
endfunction

function Register takes unit u, real duration returns nothing
    set udg_INDEX = udg_INDEX + 1
    set udg_Units[udg_INDEX] = u
    set udg_Durations[udg_INDEX] = duration
    if udg_INDEX == 1 then
        call TimerStart(udg_Time,0.5,true,function Execute)
    endif
endfunction

b. Keying To Handle (in this case, one timer for one instance)

function Execute takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer key = GetHandleId(t)
    local unit u = LoadUnitHandle(udg_HashTable,key,0)
    local real duration = LoadReal(udg_HashTable,key,1)
    
    ////do a heal/damage to unit "u"
    
    set duration = duration - TimerGetTimeout(t)
    call SaveReal(udg_HashTable,key,0,duration)
    if duration <= 0 then
        call FlushChildHashtable(udg_HashTable,key)
        call PauseTimer(t)
        call DestroyTimer(t)
    endif
    set u = null
    set t = null
endfunction

function Register takes unit u, real duration returns nothing
    local timer t = CreateTimer()
    local integer key = GetHandleId(t)
    call SaveUnitHandle(udg_HashTable,key,0,u)
    call SaveReal(udg_HashTable,key,1,duration)
    call TimerStart(t,0.5,true,function Execute)
endfunction

Note: the periodic method can be replaced by trigger periodic method

==============================================

THANKYOU
PS: I haven't run these codes

 

[Wietlol]

1. Periodic calls:
A timer is way more efficiënt because a timer directly calls a function.
A trigger calls a list of conditions , saves the returned value if it is false and then checks if the value is true to run a list of actions.
Calling a function directly is... 80% faster I thought but could be wrong.
A timer can also be more efficiëntly turned on and off.

2. Group Execution:
First of all don't filter. Make the filter inside the foreach unit call (if possible).
In that case doing the ForGroup is the most efficiënt way... I suppose.

3. Multi-Instancing:
a. one timer and save in array.
However you should have a function that catches all removes of the objects.
For example a missile system. When the missile is destroyed, a function is called that removes the missile and saves the data of the last one in the array to the one of the removed missile.
There you should have a check to pause the timer.

 

[8uY_YoU]

Seems like quiet here

1. Periodic calls:
A timer is way more efficiënt because a timer directly calls a function.
A trigger calls a list of conditions , saves the returned value if it is false and then checks if the value is true to run a list of actions.
Calling a function directly is... 80% faster I thought but could be wrong.
A timer can also be more efficiëntly turned on and off.

2. Group Execution:
First of all don't filter. Make the filter inside the foreach unit call (if possible).
In that case doing the ForGroup is the most efficiënt way... I suppose.

3. Multi-Instancing:
a. one timer and save in array.
However you should have a function that catches all removes of the objects.
For example a missile system. When the missile is destroyed, a function is called that removes the missile and saves the data of the last one in the array to the one of the removed missile.
There you should have a check to pause the timer.

1) Thats my current method of periodic calls in my map, however I did saw Dota's JASS which is using trigger as periodic calls. Almost no timer is used in Dota (I only saw it in j file on playdota forum not hacking it). That's why I brought it for comparation

2) In other words, null in filter right?
Honestly some of nice spell map that I have seen is using the first method (one function), but I found it takes more line than what you said.

3) Indexing, ok
so less handle is still better than less process?

 

[Wietlol]

1) I don't know about DotA code but this is basic logic.
You create a timer with ... as duration and make it looping.
When the timer ends you -
A. Call a function.
B. Do nothing
In B you have a trigger that has an event ("timer" expires)
Then it runs it runs a ConditionalTriggerExecute... which lists the conditions of the trigger, then runs all while keeping track of a boolean.
Then it checks if that boolean is true (to run the actions but it will always be false... optimization Conditions vs Actions)
I mean... you tell the logic.

2) null in filter indeed.
Filtering is not bad itself.
It is just that you do an extra action based on the returned value (GroupRemoveUnit(this, GetEnumUnit()))
You can use a filter if you want to use that same group later again.
Then you use a filter the first time to remove unwanted units and then you can do a ForGroup() whenever you want to use that same group again.
If you will only use it once, then use ForGroup().

3) not only less handle but also increased possibilities.
process could still be called once in a while. Take a look inside the EOT_Interval function in my Effect Over Time System. You can see an interval execution that is dynamic instead of every 0.5 seconds etc.
... that indexing is pretty interesting though.

 

[BPower]

ForGroup is slower compared to FirstOfGroup depending on the unit group size. The more units the slower

arrays are faster than hashtables. However this is not about speed, but more about the amount of data you want to store.