wc3campaigns
WC3C Homepage - www.wc3c.netUser Control Panel (Requires Log-In)Engage in discussions with other users and join contests in the WC3C forums!Read one of our many tutorials, ranging in difficulty from beginner to advanced!Show off your artistic talents in the WC3C Gallery!Download quality models, textures, spells (vJASS/JASS), systems, and scripts!Download maps that have passed through our rigorous approval process!

Go Back   Wc3C.net > Warcraft III Modding > Developer's Corner > Triggers & Scripts
User Name
Password
Register Rules Get Hosted! Chat Pastebin FAQ and Rules Members List Calendar



Reply
 
Thread Tools Search this Thread
Old 07-13-2007, 06:58 AM   #1
cohadar
master of fugue
 
cohadar's Avatar
 
Join Date: Jun 2007
Posts: 2,453

Submissions (5)

cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)

Default Return value LEAK

Return values leak sometimes, but only if you return local variables.

For example:

Code:
function CreatePeasantAtCenter takes nothing returns unit
    local unit u = CreateUnit(Player(0), 'phea', 0, 0, 0)
    return u
endfunction

to fix this you would do:
Code:
function CreatePeasantAtCenter takes nothing returns unit
   return CreateUnit(Player(0), 'phea', 0, 0, 0)
endfunction

these examples are fairly simple so people might think ok
I will just never use a local to hold return value,
but it does not work that way.

Sometimes you need a variable to hold your return value before
you return it, (some processing needed before return)

One example is a function that creates a struct and fills it with some data.

Another example is infamous leak in blizzards GetUnitsOfPlayerAll()
Here is what blizzard did:

Code:
function GetUnitsOfPlayerAll takes player whichPlayer returns group
    return GetUnitsOfPlayerMatching(whichPlayer, null)
endfunction

// witch calls this:

function GetUnitsOfPlayerMatching takes player whichPlayer, boolexpr filter returns group
    local group g = CreateGroup()
    call GroupEnumUnitsOfPlayer(g, whichPlayer, filter)
    call DestroyBoolExpr(filter)
    return g                             // <---- oh yeah return LEAK
endfunction

// blizzard could have avoided this by using globals like they usually do

function GetUnitsOfPlayerMatching takes player whichPlayer, boolexpr filter returns group
    set bj_lastCreatedGroup = CreateGroup()
    call GroupEnumUnitsOfPlayer(bj_lastCreatedGroup, whichPlayer, filter)
    call DestroyBoolExpr(filter)
    return bj_lastCreatedGroup      // <---- no more leak, bj_ is a global
endfunction

// but for some reason they forgot to do it here

Bottom line:
if you have a function that creates something,
store the value in your custom global
or use bj_s if you think appropriate
just stay away from locals.
cohadar is offline   Reply With Quote
Sponsored Links - Login to hide this ad!
Old 07-13-2007, 07:16 AM   #2
DioD
obey
 
DioD's Avatar
 
Join Date: Feb 2006
Posts: 1,532

Submissions (4)

DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)

Send a message via ICQ to DioD
Default

Code:
function CreatePeasantAtCenter takes nothing returns unit
    local unit u = CreateUnit(Player(0), 'phea', 0, 0, 0)
    return u
endfunction
leaks local's pointer (minor)

Code:
function CreatePeasantAtCenter takes nothing returns unit
   return CreateUnit(Player(0), 'phea', 0, 0, 0)
endfunction
leak nothing, but there is BJ like function == useless

Code:
function CreatePeasantAtCenter takes nothing returns nohing
   set EX_LAST_UNIT = CreateUnit(Player(0), 'phea', 0, 0, 0)
endfunction
Some times we need to "Execute" other functions, best solution of the problem is temp global, blizzard used it, but there is better way...

Code:
function CreatePeasantAtCenter takes unit Null returns unit
   set Null = CreateUnit(Player(0), 'phea', 0, 0, 0)
   return Null
endfunction
To fix we allocate parameter value from function, soo we can do anything we want with created unit

function do any
//we must pass value, i used pregenerated handles to avoid passing nulls...
call Kill(CreatePeasantAtCenter(NULL))
endfunction
DioD is offline   Reply With Quote
Old 07-13-2007, 07:31 AM   #3
cohadar
master of fugue
 
cohadar's Avatar
 
Join Date: Jun 2007
Posts: 2,453

Submissions (5)

cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)

Default

Quote:
Originally Posted by DioD
[code]
>> leaks local's pointer (minor)

>> leak nothing, but there is BJ like function == useless

>> To fix we allocate parameter value from function, soo we can do anything we want with created unit
endfunction

There is no such thing as i minor leak,
if you put it in a periodic timer it very soon becomes major one.

BJ like functions are not useless,
there is a thing called over-optimization
(garbling of code with no real performance purpose)

Adding return arguments is a good idea. blizzard uses it in natives
example : GroupEnumUnitsOfPlayer

but you must mark them properly and change the function name
so the user knows they are return parameters and not just some rubbish.

Also bj_ like globals are sometimes really necessary so this method is not
"better" it is just different.
cohadar is offline   Reply With Quote
Old 07-13-2007, 08:29 AM   #4
DioD
obey
 
DioD's Avatar
 
Join Date: Feb 2006
Posts: 1,532

Submissions (4)

DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)

Send a message via ICQ to DioD
Default

globals is much more better.

if used correctly there is no need of locals at all.

i used this with SCV, making CreateTimerEx() function that create timer and place it into global.

function
if Ex_Crushed != null then
call Echo("Thread Crush at " + Ex_Crushed)
call Echo(I2S(Ex_Last_Integer))
endif
set Ex_Crushed = "This function name"
call CreateTimerEx() //timer created and stored in global Ex_Last_Handle
call Ha2StEx() //store string and integer value of last handle
//Ha-Handle St-String Ex-Extended eg stores result in globals
call StoreInteger(Ex_Core,Ex_Last_String,"handleid",Ex_Last_Integer)
set Ex_Crushed = null
endfunction

by globals there is no need of locals or nullifications.
also you may easy track thread crushes by globals and track data used by functions cause it is not gone or nulled.

and minor leak is pointer
major is handle, like locations of timers

Last edited by DioD : 07-13-2007 at 08:30 AM.
DioD is offline   Reply With Quote
Old 07-13-2007, 09:10 AM   #5
cohadar
master of fugue
 
cohadar's Avatar
 
Join Date: Jun 2007
Posts: 2,453

Submissions (5)

cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)

Default

Quote:
Originally Posted by DioD
globals is much more better.

if used correctly there is no need of locals at all.

by globals there is no need of locals or nullifications.

also you may easy track thread crushes by globals and track data used by functions cause it is not gone or nulled.

and minor leak is pointer
major is handle, like locations of timers

I agree with you completely,
also would like to add that globals that are used as return values
should be private inside a scope or library to avoid synchronization problems.

(pointers are from systems that don't have garbage collector, like C/C++)

so you were saying:
minor = reference (or handle)
major = in game object that handle is referencing

very interesting notation, I am just not sure of this:

Lets assume timers object takes 30 bytes of memory.
and we have a handle that points to that timer.

If we destroy the timer does the game simply stops the timer from working
(and optionally zeroes those 30 bytes) or it actually frees those 30 bytes.

Or maybe it waits for handle to be free also to free those 30 bytes.

Any comparison of how this is done in C# or Java is invalid here
because JASS has completely berserk implementation.

So basically I would need to make a test map to check this out.
(currently at work, when I get home)

If you have that test already I would like to see it.

PS: it nice talking to someone who is versatile in this stuff.
cohadar is offline   Reply With Quote
Old 07-13-2007, 09:48 AM   #6
Toadcop
BuranX
 
Toadcop's Avatar
 
Join Date: Jul 2006
Posts: 1,886

Submissions (4)

Toadcop is just really nice (299)Toadcop is just really nice (299)

Approved Map: TcXSpell Making Session 10 Winner

Send a message via ICQ to Toadcop
Default

btw... it's bullshit ^^ cause try to do...

function test takes nothing returns nothing
local unit u=some_global_unit
endfunction

this function will absolutely not leak anything. the fault is bad coders.
Quote:
leaks local's pointer
it's a fairy tale for lamers.

your example would simple leak cause the handle id will be "used forever" and war3 would need to get some new one == "leak" but nit really.
it's a amazing old story you would better look what you are writting in your algorithms than to hunt every idiotical leak. <_<

// sorry it must be ^^ i hate disinformations...

Last edited by Toadcop : 07-13-2007 at 09:53 AM.
Toadcop is offline   Reply With Quote
Old 07-13-2007, 10:49 AM   #7
cohadar
master of fugue
 
cohadar's Avatar
 
Join Date: Jun 2007
Posts: 2,453

Submissions (5)

cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)

Default

Quote:
Originally Posted by Toadcop
btw... it's bullshit ^^ cause try to do...

function test takes nothing returns nothing
local unit u=some_global_unit
endfunction

this function will absolutely not leak anything. the fault is bad coders.
it's a fairy tale for lamers.

Actually it will cause a memory loss
(I avoid the term leak here because you seem to think that leak is only non destroying objects)

If you make a periodic event in witch you do this:

Code:
// periodic or a loop

set some_global_unit = CreateUnit()
call test()
call RemoveUnit(some_global_unit)

// and modify test function this way
function test takes nothing returns unit
    local unit u=some_global_unit
    return u
endfunction

// we are talking about return leaks so don't plant bad examples please

It will leak - big time.

So I think you just stepped into some bullshit,
better clean your shoes.
cohadar is offline   Reply With Quote
Old 07-13-2007, 11:25 AM   #8
Rising_Dusk
Obscurity, the Art


Projects Director
Project Leader: OD
 
Join Date: Feb 2006
Posts: 9,729

Submissions (27)

Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)

Hero Contest #3 - 1st PlaceApproved Map: Desert of ExileApproved Map: Advent of the ZenithHero Contest #2 - 1st PlaceHero Contest - Third place>

Send a message via AIM to Rising_Dusk Send a message via MSN to Rising_Dusk
Default

Pointer leaks are 4 byte references.
Leaking these are as "minor" as it comes with leaks.

Collapse JASS:
function Rawr_1 takes nothing returns location
    local location l = Location(0,0)
    return Location(0,0)
endfunction
Which is the worse leak? The pointer? Or the location?
An entire map can go through 100% of its code and NEVER reset a pointer and it will not lag (I've tried it with both of my maps).
However, leaking a couple dozen locations every minute or so really adds up and can cause huge buildup of fps loss.

Location leaks are just my example, leaking groups, rects, etc. yields the same result.
The pointers really don't matter, so yes they are a very "minor" leak.
__________________

Last edited by Rising_Dusk : 07-13-2007 at 11:27 AM.
Rising_Dusk is offline   Reply With Quote
Old 07-13-2007, 12:18 PM   #9
Toadcop
BuranX
 
Toadcop's Avatar
 
Join Date: Jul 2006
Posts: 1,886

Submissions (4)

Toadcop is just really nice (299)Toadcop is just really nice (299)

Approved Map: TcXSpell Making Session 10 Winner

Send a message via ICQ to Toadcop
Default

cohadar <_< in your case units are leaking.

omg... ok i will test it AGAIN...

well im was alomst correct ! the "new" feature is what if some function returns value and you don't use it (for example call SomeWhatReturnsUnit() will leak and set xtmp=SomeWhatReturnsUnit() will NOT leak maybe there some stack cleaning problem i don't know but it's a fact ! + the leaks are VERY minor ! )

i have called this code in a 0.001 period and NO leaks ! but if i was calling simple this functions (not using them as a value) i get some minor but constant leaks. the problem is return with out return it works flawless !

Code:
function XXXXX takes nothing returns unit
    local unit u=gg_unit_Hblm_0001 // it's a global unit
    return u
endfunction

globals
    unit uuuxxx=null
endglobals

function Trig_per_Actions takes nothing returns nothing
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()

    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()

    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()

    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()

    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
    set uuuxxx=XXXXX()
endfunction

well but i think also this leaks are not worth to be mentioned cause ~50K calls and you will get 4 KB less memory.

conclusion to much panic =\
// btw i have thinked about this long ago what some func returns value and what happens if you don't "use" it... so the answer: it will leak. but extreme minor.
Toadcop is offline   Reply With Quote
Old 07-13-2007, 12:34 PM   #10
DioD
obey
 
DioD's Avatar
 
Join Date: Feb 2006
Posts: 1,532

Submissions (4)

DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)

Send a message via ICQ to DioD
Default

reference to static unit is always same, we talk about dinamic triggers\actions with reference to different handles.
DioD is offline   Reply With Quote
Old 07-13-2007, 12:48 PM
Toink
This message has been deleted by Toink.
Old 07-13-2007, 12:48 PM   #11
grim001
requires vJass
 
grim001's Avatar


Code Moderator
 
Join Date: Nov 2006
Posts: 1,540

Submissions (10)

grim001 is just really nice (277)grim001 is just really nice (277)

Send a message via AIM to grim001
Default

Leaking a 4 byte pointer is "not a big deal"

but, the problem is that unless you null all references to a handle its handle ID will never be recycled

that causes the handle ID to continuously go up, and WC3 seems to slow down the higher the handle ID gets, whether you are leaking anything else or not. It seems that a high handle ID inherently slows down WC3.
grim001 is offline   Reply With Quote
Old 07-13-2007, 12:49 PM   #12
Vexorian
Free Software Terrorist
 
Vexorian's Avatar


Technical Director
 
Join Date: Apr 2003
Posts: 14,898

Submissions (37)

Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)

Hero Contest #3 - 2nd Place

Default

Quote:

Collapse JASS:
function test takes nothing returns nothing
local unit u=some_global_unit
endfunction
this function will absolutely not leak anything. the fault is bad coders.
The fault here is bad testers.

If you later removed the unit pointed by the global and set the global to null or assigned something else to it, the local instances in that function would still point towards the unit thus there will be a leak of 4 bytes.


Quote:
One example is a function that creates a struct and fills it with some data.
These leaks are only caused by the faulty ref counter from blizzard, and thus they only affect handles, not integers, so structs are "safe"
__________________
Zoom (requires log in)Wc3 map optimizer 5.0
Someone should fix .wav sound in this thing.
Zoom (requires log in)JassHelper 0.A.2.A
Turns your simple code into something that is complicated enough to work.
Faster != more useful
Vexorian is offline   Reply With Quote
Old 07-13-2007, 01:39 PM   #13
cohadar
master of fugue
 
cohadar's Avatar
 
Join Date: Jun 2007
Posts: 2,453

Submissions (5)

cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)

Default

Since we are talking about structs I was thinking something like this:

If you have some handles in a struct, for example

Code:
struct data
    unit u
    timer t
    location l
endstruct
is there any point in nulling those handles in onDestroy method?
(I saw someone somewhere recommending it)

I believe not because number of handles in struct is limited to 8192
so even if it leaks them all it is still only 32kb of memory = unnoticeable.

Last edited by cohadar : 07-13-2007 at 01:39 PM.
cohadar is offline   Reply With Quote
Old 07-13-2007, 03:44 PM   #14
Vexorian
Free Software Terrorist
 
Vexorian's Avatar


Technical Director
 
Join Date: Apr 2003
Posts: 14,898

Submissions (37)

Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)

Hero Contest #3 - 2nd Place

Default

yes and no.
Collapse JASS:
struct data
    unit u
    timer t
    location l
endstruct
If it was
Collapse JASS:
struct data
    unit u=null
    timer t=null
    location l=null
endstruct

which is logical since it is better to have stuff initialized correctly.

then .allocate() (or .create()) would set stuff to null automatically everytime it is called.

If you did d.u=GetTriggerUnit() and later d.destroy() and didn't set d.u to null, since d was destroyed .create will eventually recycle the id and assign null again to d.u , so there isn't really any problem there as long as you do use .destroy and .create
__________________
Zoom (requires log in)Wc3 map optimizer 5.0
Someone should fix .wav sound in this thing.
Zoom (requires log in)JassHelper 0.A.2.A
Turns your simple code into something that is complicated enough to work.
Faster != more useful
Vexorian is offline   Reply With Quote
Old 07-13-2007, 04:03 PM   #15
DioD
obey
 
DioD's Avatar
 
Join Date: Feb 2006
Posts: 1,532

Submissions (4)

DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)DioD is a jewel in the rough (220)

Send a message via ICQ to DioD
Default

i posted SDARS somewhere, this system uses slot table to generate ID's for structs\units and other staff, im not finished it but currently it allocate "used" and "free" sector in some array, there is no used slots inside free sector and if slot marked free, 2 cycles later it will be reused.
reuse will replace values inside arrays and no leaks will happen.

Basicaly it was created for "return bug free" attach to minimize loop seek, later i used it to generate "second handle" for units.
Currently it allocate slots stable and can be used to generate clusters, soon arrays.

Vjass structs is no perfect, but fast and simple.
Id generation uses single linked array list and 2 globals.

I hate vjass and structs only for one thing - its hide real code from me and generate its own staff.
this is problem of any high level programming, and its much more harder do debug\improve.

Every struct generate parralel arrays and use them in direct order if no arrays free, reverse order if any struct were destroyed.

Linked list array returns slots in reverse order eg (3.2.1) from last free to first free, then will jump to use "new" slots.

i tested algoritm hard.
if slot 6 free,
zero will be pushed to array slot 6 and external handle will got 6 as its value.
if slot 99 free
6 will be pushed to array slot 99 and external handle will got 99 as its value.

next use will return 99 and 6 will pop to external handle

if any other slot free if will stay before 6 and this id will stuck for a long time.
dinamic usage may cause some slots to stuck permanently if spell or system used much and is not instant, eg buffs auras and other staff.
DioD is offline   Reply With Quote
Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off


All times are GMT. The time now is 02:33 AM.


Affiliates
The Hubb The JASS Vault Clan WEnW Campaign Creations Clan CBS GamesModding Flixreel Videos

Powered by vBulletin (Copyright ©2000 - 2020, Jelsoft Enterprises Ltd).
Hosted by www.OICcam.com
IT Support and Services provided by Executive IT Services