Which could be more efficient/better performance?

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

I'm elaborating an answer for you, hopefully this night I'll have some ideas for you

Thanks admin 

I am desperately need some references to enhance performance of systems in my map
Since ppl with low-spec PC keep struggling with my map

The first thing is to start using some libraries to speed up your code, the first one is an indexing library like ALLOC: https://wc3modding.info/5012/snippet-allocloop-aka-alloc-2/

With this one the allocation of things would be dramatically increased. And it works because it's slower to create/destroy handles than reusing it.

I'll post some examples about what you can do with this baby.

Let's start with timers:

    function RunThis takes nothing returns nothing
        // code here. The timer hancle can be managed in this function to destroy or pause...
    endfunction
     
    function InitTrig_test takes nothing returns nothing
        call TimerStart(CreateTimer(),0.25,true,function RunThis) // with this you save a variable assignation...
    endfunction

Using periodic triggers is a BIG no no. triggers are not recyclable as timers. And for time recycling there's a tool called timerutils.

About Unit grouping, the fastest way is the one you posted:

    function GroupExec takes nothing returns nothing
        // As you can see, I use an already created group, a bj one in order to make it faster in use, and you don't need to destroy it, it's cleaned up by the same loop...
        local unit c = GetTriggerUnit()
        local real x = GetUnitX(c)
        local real y = GetUnitY(c)
        local unit temp
        call GroupEnumUnitsInRange(bj_lastCreatedGroup,x,y,300,null)
        loop
            set temp = FirstOfGroup(bj_lastCreatedGroup)
        exitwhen temp == null
            if IsUnitEnemy(temp,GetOwningPlayer(c)) then
                //execute each valid unit
            endif
            call GroupRemoveUnit(bj_lastCreatedGroup,temp)
        endloop
        set c = null
    endfunction

Answered some parts...

Quote from: moyack on February 21, 2015, 05:14:13 PM
The first thing is to start using some libraries to speed up your code, the first one is an indexing library like ALLOC: https://wc3modding.info/5012/snippet-allocloop-aka-alloc-2/
With this one the allocation of things would be dramatically increased. And it works because it's slower to create/destroy handles than reusing it.

Will alloc's allocate will instantly override current allocate method on struct?

Quote
Let's start with timers:

Code (jass) Select Expand

    function RunThis takes nothing returns nothing
        // code here. The timer hancle can be managed in this function to destroy or pause...
    endfunction
     
    function InitTrig_test takes nothing returns nothing
        call TimerStart(CreateTimer(),0.25,true,function RunThis) // with this you save a variable assignation...
    endfunction

Using periodic triggers is a BIG no no. triggers are not recyclable as timers. And for time recycling there's a tool called timerutils.

I had used recycling system like Vex timerutils and dummy system in my map, but sometimes it is bugged, I don't know whether it is my fault or system's fault.
One of bug is when I use a recycled timer.
Timer that is retrieved for me is still currently being used with other instances (still active). So, I took a safe way to use default create and destroy.

Quote
About Unit grouping, the fastest way is the one you posted:

Code (jass) Select Expand

    function GroupExec takes nothing returns nothing
        // As you can see, I use an already created group, a bj one in order to make it faster in use, and you don't need to destroy it, it's cleaned up by the same loop...
        local unit c = GetTriggerUnit()
        local real x = GetUnitX(c)
        local real y = GetUnitY(c)
        local unit temp
        call GroupEnumUnitsInRange(bj_lastCreatedGroup,x,y,300,null)
        loop
            set temp = FirstOfGroup(bj_lastCreatedGroup)
        exitwhen temp == null
            if IsUnitEnemy(temp,GetOwningPlayer(c)) then
                //execute each valid unit
            endif
            call GroupRemoveUnit(bj_lastCreatedGroup,temp)
        endloop
        set c = null
    endfunction

Aw..
By judging on how you replace my g var, I conclude that it is better to using global variable than local variable, since globals are reusable and creates less handle (locals create one handle per creation)
Am I right?

Edit: http://www.thehelper.net/threads/new-leak-found-keeping-references-to-destroyed-objects.122136/
Got something interesting here, which telling us to null global/local handle var as soon as possible to reset handle ID counter

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

Well looks like I will moving again from vanilla JASS to vJass one
Gracias admin