Memory Leak?

07-18-2006, 01:51 AM

StRoNgFoE_2000

In the below trigger I think I have a memory leak.. but I can't see where. I looked it over 100 times and I just can't find it. The trigger was converted to JASS with a few small changes such as local variables and some BJ's taken out (I'm not yet sure how to eliminate all).

I have 1.5GB of RAM. When the map first loaded my available memory was 1198MB. I tested without using the trigger for 5 minutes, my available memory was 1191MB. I let the map run for 5 minutes only building towers and such, without sending units, and my available memory as 1185MB. Not too bad.. So, for another 5 minutes, I ran the build unit trigger (the one below) around maybe 200 times (which isn't much considering this is a tower wars), and my available memory dropped down to 1108MB, so I ran the trigger about 200 more times and it dropped down to 1026MB. And after another 5 minutes dropped down to 986. This isn't a big issue for me because I have a lot to spare and I never feel the memory drop, but I imagine machines with around 256MB will suffer.

Here is the trigger.. I cannot find the leak, I was hoping someone more experienced then me can.

JASS:

function BuildUnitTopActions takes nothing returns nothing
    local unit UnitBuild1
    local unit UnitBuild2
    local location TempPoint
    if (IsUnitType(GetEnteringUnit(), UNIT_TYPE_PEON)) or (IsUnitType(GetEnteringUnit(), UNIT_TYPE_GIANT)) or (IsUnitType(GetEnteringUnit(), UNIT_TYPE_TOWNHALL)) then
    return
    else
    set udg_Income[GetConvertedPlayerId(GetOwningPlayer(GetEnteringUnit()))] = (udg_Income[GetConvertedPlayerId(GetOwningPlayer(GetEnteringUnit()))] + GetUnitPointValue(GetEnteringUnit()))
    set udg_TotalIncome1 = (udg_Income[1]+(udg_Income[2]+udg_Income[3]))
    if (GetOwningPlayer(GetEnteringUnit())==Player(0)) then
    call MultiboardSetItemValueBJ(GetLastCreatedMultiboard(), 2, 2, I2S(udg_Income[1]))
    elseif (GetOwningPlayer(GetEnteringUnit())==Player(1)) then
    call MultiboardSetItemValueBJ(GetLastCreatedMultiboard(), 2, 3, I2S(udg_Income[2]))
    elseif (GetOwningPlayer(GetEnteringUnit())==Player(2)) then
    call MultiboardSetItemValueBJ(GetLastCreatedMultiboard(), 2, 4, I2S(udg_Income[3]))
    endif
    call MultiboardSetItemValueBJ(GetLastCreatedMultiboard(), 2, 5, I2S(udg_TotalIncome1))
    set TempPoint = GetRectCenter(gg_rct_Middle_Center_Bottom_Left)
    call CreateNUnitsAtLoc(1, GetUnitTypeId(GetEnteringUnit()), Player(6), TempPoint, bj_UNIT_FACING)
    call RemoveLocation(TempPoint)
    call AddSpecialEffectTargetUnitBJ("origin", GetLastCreatedUnit(), "Abilities\\Spells\\Undead\\DarkRitual\\DarkRitualTarget.mdl")
    call DestroyEffect(GetLastCreatedEffectBJ())
    call SetUnitColor(GetLastCreatedUnit(), GetPlayerColor(GetOwningPlayer(GetEnteringUnit())))
    set UnitBuild1 = GetLastCreatedUnit()
    set TempPoint = GetRectCenter(gg_rct_Middle_Center_Top_Right)
    call CreateNUnitsAtLoc(1, GetUnitTypeId(GetEnteringUnit()), Player(6), TempPoint, bj_UNIT_FACING)
    call RemoveLocation(TempPoint)
    call AddSpecialEffectTargetUnitBJ("origin", GetLastCreatedUnit(), "Abilities\\Spells\\Undead\\DarkRitual\\DarkRitualTarget.mdl")
    call DestroyEffect(GetLastCreatedEffectBJ())
    call SetUnitColor(GetLastCreatedUnit(), GetPlayerColor(GetOwningPlayer(GetEnteringUnit())))
    set UnitBuild2 = GetLastCreatedUnit()
    if (udg_UnitPlayerTop==3) then
    set udg_UnitPlayerTop = 0
    endif
    if (udg_UnitPlayerTop==0) then
    call DoNothing( )
    elseif (udg_UnitPlayerTop==1) then
    call SetUnitOwner(UnitBuild1, Player(8), false)
    call SetUnitOwner(UnitBuild2, Player(8), false)
    elseif (udg_UnitPlayerTop==2) then
    call SetUnitOwner(UnitBuild1, Player(10), false)
    call SetUnitOwner(UnitBuild2, Player(10), false)
    endif
    set udg_UnitPlayerTop = (udg_UnitPlayerTop + 1)
    set TempPoint = GetRectCenter(gg_rct_Middle_Left_Bottom)
    call IssuePointOrderLoc(UnitBuild1, "move", TempPoint)
    call RemoveLocation(TempPoint)
    set TempPoint = GetRectCenter(gg_rct_Middle_Right_Bottom)
    call IssuePointOrderLoc(UnitBuild2, "move", TempPoint)
    call RemoveLocation(TempPoint)
    call RemoveUnit(GetEnteringUnit())
    call ConditionalTriggerExecute(gg_trg_UnitKillers)
    if (udg_SuperSpeed) and (GetUnitTypeId(udg_ShrineTracker[GetConvertedPlayerId(GetOwningPlayer(GetEnteringUnit()))])=='h05R') then
    call ConditionalTriggerExecute(gg_trg_AutoUpgradeSuperSpeed)
    elseif (not(udg_SuperSpeed)) and (GetUnitTypeId(udg_ShrineTracker[GetConvertedPlayerId(GetOwningPlayer(GetEnteringUnit()))])=='hC15') then
    call ConditionalTriggerExecute(gg_trg_AutoUpgradeShrine)
    endif
    set UnitBuild1 = null
    set UnitBuild2 = null
    endif
endfunction

function InitTrig_BuildUnitTop takes nothing returns nothing
    set gg_trg_BuildUnitTop = CreateTrigger( )
    call TriggerRegisterEnterRectSimple(gg_trg_BuildUnitTop, gg_rct_Region_Top)
    call TriggerAddAction(gg_trg_BuildUnitTop, function BuildUnitTopActions)
endfunction

07-18-2006, 02:00 AM	#2
Rising_Dusk	You have two leaks. First you don't remove the location, second you dont nullify the variable. Add -- JASS: call RemoveLocation(TempPoint) set TempPoint = null -- onto the end. Also, move your unit nullifications outside of the conditional statement.

07-18-2006, 02:05 AM

StRoNgFoE_2000

I never knew you had to set points to null, thank you. But you say I do not remove it? I checked through and I removed it after every usage of setting it, always after the action. Do I have to remove it again at the end of the trigger? I'm a little confused on that.

For example..

JASS:

 
    set TempPoint = GetRectCenter(gg_rct_Middle_Left_Bottom)
    call IssuePointOrderLoc(UnitBuild1, "move", TempPoint)
    call RemoveLocation(TempPoint)

07-18-2006, 02:09 AM	#4
Rising_Dusk	Sorry, I didn't actually read your code I just skimmed to the end because that's where most people do cleanup. :D

07-18-2006, 02:15 AM

StRoNgFoE_2000

Ok, I thought maybe you have to remove points after everytime using them before setting them again or else the previous point will leak. So this means I can destroy it at the end even though I set it to something different multiple times?

Example at the end of my trigger....

JASS:

    set TempPoint = GetRectCenter(gg_rct_Middle_Left_Bottom)
    call IssuePointOrderLoc(UnitBuild1, "move", TempPoint)
    set TempPoint = GetRectCenter(gg_rct_Middle_Right_Bottom)
    call IssuePointOrderLoc(UnitBuild2, "move", TempPoint)
    call RemoveUnit(GetEnteringUnit())
    endif
    endif
    call RemoveLocation(TempPoint)
    set TempPoint = null
    set UnitBuild1 = null
    set UnitBuild2 = null
endfunction

Instead of..

JASS:

    
    set TempPoint = GetRectCenter(gg_rct_Middle_Left_Bottom)
    call IssuePointOrderLoc(UnitBuild1, "move", TempPoint)
    call RemoveLocation(TempPoint)
    set TempPoint = null
    set TempPoint = GetRectCenter(gg_rct_Middle_Right_Bottom)
    call IssuePointOrderLoc(UnitBuild2, "move", TempPoint)
    call RemoveLocation(TempPoint)
    set TempPoint = null
    call RemoveUnit(GetEnteringUnit())
    endif
    endif
    set UnitBuild1 = null
    set UnitBuild2 = null
endfunction

07-18-2006, 02:20 AM	#6
Rising_Dusk	Nonono, you're correct. You should remove them after everytime you set them, otherwise they will leak. I was saying I didn't actually 100% go through your code, so I didn't know how you were using them. I just assumed you only created one point and not multiple. Sorry for the confusion.

07-18-2006, 04:47 AM

StRoNgFoE_2000

Ugh.. after setting the points to null and even removing the special effects it is still leaking. The map eats up no memory until this trigger is ran.. hmm.. is there a leak I don't know about, like maybe integers even though I could find nothing about integers leaking..

This little section right here changes player every other unit... maybe this could be causing the memory buildup?

JASS:

    if (udg_UnitPlayerTop==3) then
    set udg_UnitPlayerTop = 0
    endif
    if (udg_UnitPlayerTop==0) then
    call DoNothing( )
    elseif (udg_UnitPlayerTop==1) then
    call SetUnitOwner(UnitBuild1, Player(8), false)
    call SetUnitOwner(UnitBuild2, Player(8), false)
    elseif (udg_UnitPlayerTop==2) then
    call SetUnitOwner(UnitBuild1, Player(10), false)
    call SetUnitOwner(UnitBuild2, Player(10), false)
    endif
    set udg_UnitPlayerTop = (udg_UnitPlayerTop + 1)

07-18-2006, 07:21 AM

Rising_Dusk

Theres nothing there that should leak.
Try indenting your text and reposting it.

Also make sure you're properly setting globals.
Also check how many times the trigger gets run.
And to top it off, some BJDebugMsg(...)s around the trigger to see where it ends (If it does prematurely end) or if something happens too many times, etc.

Notably, integers do not leak.
That much I know for a fact -- Also, globals do not leak.
So no worries in those regards.

07-18-2006, 08:24 AM	#9
Captain Griffen	Co-ordinates > Locations almost always in JASS. So find out the co-ordinates (just look in WE, says on the bottom left where the cursor is), and use them. Locations are annoying and slow things down (as you have to use variables and remove them, and null them).

07-18-2006, 08:51 AM

#10

StRoNgFoE_2000

Ok, I tested the trigger again, this time with Rising_Dusk's suggestion of seeing how many times it was fired. It seems like there is no leak after all, and the memory lost may be normal. What I did was check the available memory after every 300 executions of the trigger. Here are the results:

0 Times - 1,028,624
300 Times - 1,009,756
600 Times - 999,428
900 Times - 994,112
1200 Times - 988,740
1500 Times - 986,556
1800 Times - 985,688
2100 Times - 982,864
2400 Times - 981,280
2700 Times - 980,816
3000 Times - 980,165

48MB lost is no big deal. I'm guessing because it stays semi-constant around 2000 executions it does not leak. Here is the trigger with the indents (have not yet used coords over points yet, mainly cause I don't know how to yet). It is modified slightly from my first post.

JASS:

function BuildUnitTopActions takes nothing returns nothing
    local unit UnitBuild1
    local unit UnitBuild2
    local unit EnterUnit
    local location TempPoint
    set EnterUnit = GetEnteringUnit()
    if (IsUnitType(EnterUnit, UNIT_TYPE_PEON)) or (IsUnitType(EnterUnit, UNIT_TYPE_GIANT)) or (IsUnitType(EnterUnit, UNIT_TYPE_TOWNHALL)) then
        call DoNothing( )
    else
        set udg_Income[GetConvertedPlayerId(GetOwningPlayer(EnterUnit))] = (udg_Income[GetConvertedPlayerId(GetOwningPlayer(EnterUnit))] + GetUnitPointValue(EnterUnit))
        set udg_TotalIncome1 = (udg_Income[1]+(udg_Income[2]+udg_Income[3]))
        if (GetOwningPlayer(EnterUnit)==Player(0)) then
            call MultiboardSetItemValueBJ(GetLastCreatedMultiboard(), 2, 2, I2S(udg_Income[1]))
        elseif (GetOwningPlayer(EnterUnit)==Player(1)) then
            call MultiboardSetItemValueBJ(GetLastCreatedMultiboard(), 2, 3, I2S(udg_Income[2]))
        elseif (GetOwningPlayer(EnterUnit)==Player(2)) then
            call MultiboardSetItemValueBJ(GetLastCreatedMultiboard(), 2, 4, I2S(udg_Income[3]))
        endif
        call MultiboardSetItemValueBJ(GetLastCreatedMultiboard(), 2, 5, I2S(udg_TotalIncome1))
        set TempPoint = GetRectCenter(gg_rct_Middle_Center_Bottom_Left)
        call CreateNUnitsAtLoc(1, GetUnitTypeId(EnterUnit), Player(6), TempPoint, bj_UNIT_FACING)
        call RemoveLocation(TempPoint)
        set TempPoint = null
        call SetUnitColor(GetLastCreatedUnit(), GetPlayerColor(GetOwningPlayer(EnterUnit)))
        set UnitBuild1 = GetLastCreatedUnit()
        set TempPoint = GetRectCenter(gg_rct_Middle_Center_Top_Right)
        call CreateNUnitsAtLoc(1, GetUnitTypeId(EnterUnit), Player(6), TempPoint, bj_UNIT_FACING)
        call RemoveLocation(TempPoint)
        set TempPoint = null
        call SetUnitColor(GetLastCreatedUnit(), GetPlayerColor(GetOwningPlayer(EnterUnit)))
        set UnitBuild2 = GetLastCreatedUnit()
        if (udg_UnitPlayerTop==3) then
            set udg_UnitPlayerTop = 0
        endif
        if (udg_UnitPlayerTop==0) then
            call DoNothing( )
        elseif (udg_UnitPlayerTop==1) then
            call SetUnitOwner(UnitBuild1, Player(8), false)
            call SetUnitOwner(UnitBuild2, Player(8), false)
        elseif (udg_UnitPlayerTop==2) then
            call SetUnitOwner(UnitBuild1, Player(10), false)
            call SetUnitOwner(UnitBuild2, Player(10), false)
        endif
        set udg_UnitPlayerTop = (udg_UnitPlayerTop + 1)
        set TempPoint = GetRectCenter(gg_rct_Middle_Left_Bottom)
        call IssuePointOrderLoc(UnitBuild1, "move", TempPoint)
        call RemoveLocation(TempPoint)
        set TempPoint = null
        set TempPoint = GetRectCenter(gg_rct_Middle_Right_Bottom)
        call IssuePointOrderLoc(UnitBuild2, "move", TempPoint)
        call RemoveLocation(TempPoint)
        set TempPoint = null
        call RemoveUnit(EnterUnit)
        call ConditionalTriggerExecute(gg_trg_UnitKillers)
        if (udg_SuperSpeed) and (GetUnitTypeId(udg_ShrineTracker[GetConvertedPlayerId(GetOwningPlayer(EnterUnit))])=='h05R') then
            call ConditionalTriggerExecute(gg_trg_AutoUpgradeSuperSpeed)
        elseif (not(udg_SuperSpeed)) and (GetUnitTypeId(udg_ShrineTracker[GetConvertedPlayerId(GetOwningPlayer(EnterUnit))])=='hC15') then
            call ConditionalTriggerExecute(gg_trg_AutoUpgradeShrine)
        endif
    endif
    set EnterUnit = null
    set UnitBuild1 = null
    set UnitBuild2 = null
endfunction

function InitTrig_BuildUnitTop takes nothing returns nothing
    set gg_trg_BuildUnitTop = CreateTrigger( )
    call TriggerRegisterEnterRectSimple(gg_trg_BuildUnitTop, gg_rct_Region_Top)
    call TriggerAddAction(gg_trg_BuildUnitTop, function BuildUnitTopActions)
endfunction

Edit: Maybe setting the entering unit to a variable and using it instead of "GetEnteringUnit()" could have helped? Because before memory lost was over 200MB in around maybe 600-800 executions (not entirely sure), now it's just a mere 48MB after 3000. Oh well I'm just happy it works now.

07-18-2006, 09:12 AM

#11

Captain Griffen

JASS:

function MultiboardSetItemValueBJ takes multiboard mb, integer col, integer row, string val returns nothing
    local integer curRow = 0
    local integer curCol = 0
    local integer numRows = MultiboardGetRowCount(mb)
    local integer numCols = MultiboardGetColumnCount(mb)
    local multiboarditem mbitem = null

    // Loop over rows, using 1-based index
    loop
        set curRow = curRow + 1
        exitwhen curRow > numRows

        // Apply setting to the requested row, or all rows (if row is 0)
        if (row == 0 or row == curRow) then
            // Loop over columns, using 1-based index
            set curCol = 0
            loop
                set curCol = curCol + 1
                exitwhen curCol > numCols

                // Apply setting to the requested column, or all columns (if col is 0)
                if (col == 0 or col == curCol) then
                    set mbitem = MultiboardGetItem(mb, curRow - 1, curCol - 1)
                    call MultiboardSetItemValue(mbitem, val)
                    call MultiboardReleaseItem(mbitem)
                endif
            endloop
        endif
    endloop
endfunction

Culprite = Blizzard.

May be fixed by Vexorian's optimiser.

07-18-2006, 09:50 AM

#12

PipeDream

Quote:

Locations are annoying and slow things down (as you have to use variables and remove them, and null them).

No, they don't in general slow things down. In fact sometimes they are faster. This shouldn't be surprising; the VM has a lot less work to do passing function arguments when you use locations. Frequently they are also more convenient, namely when you need to return something location like.

07-18-2006, 10:01 AM

#13

oNdizZ

JASS:

        set TempPoint = GetRectCenter(gg_rct_Middle_Center_Bottom_Left)
        call CreateNUnitsAtLoc(1, GetUnitTypeId(EnterUnit), Player(6), TempPoint, bj_UNIT_FACING)
        call RemoveLocation(TempPoint)
        set TempPoint = null
        call SetUnitColor(GetLastCreatedUnit(), GetPlayerColor(GetOwningPlayer(EnterUnit)))
        set UnitBuild1 = GetLastCreatedUnit()
        set TempPoint = GetRectCenter(gg_rct_Middle_Center_Top_Right)
        call CreateNUnitsAtLoc(1, GetUnitTypeId(EnterUnit), Player(6), TempPoint, bj_UNIT_FACING)
        call RemoveLocation(TempPoint)
        set TempPoint = null

you should only nullify that local location when you arent going to use it anymore, just put one "set TempPoint = null" at the end of the script and remove the others.