Native hashtable leaks? + Unit Attachment

[Magentix]

Just a question that popped in my mind:
If you can store a unit's HandleId in a hashtable and you can also store a unit in a hashtable... what happens when the unit dies and you didn't have a trigger to "clean up" after unit death?

Does the hashtable get eventually cluttered like that with dead integer/unit references?

Also, wat if I use hashtables to replace an outdated (according to WC3C) system like PUI?
[ljass]call SaveInteger(ht,StringHash("UnitStruct"),GetHandleId(someUnit),structPointer)[/ljass]
[ljass]set someStruct = LoadInteger(ht,StringHash("UnitStruct"),GetHandleId(someUnit))[/ljass]

Would I risk getting back the wrong stuff or null structs?

 

[kingkingyyk3]

PUI is not outdated. It is still a usable and decent library.

 

[Magentix]

WC3C graveyarded it because Vex seems to have made something better...

My question remains though: Why not just use hashtables instead of PUI from now on? If the leaks I ask about don't exist, it could be a valid alternative, no?

 

[Trollvottel]

There should not be any problem with that. At least i cant see one. The things just dont get removed which shouldnt be that bad.

 

[SerraAvenger]

Since you don't use a handle, ids will be recycled.
Hence you'll get problems with old data.
I see one way around this, but it is awkward. Store the unit in the hashtable.
ids will no longer be recycled, and you're safe.

Using death detection is utter fake.
a) you can demolish other onDeath triggers
b) you can mess up heroes/revived units
c) you don't get removed units (this is what PUI was about).

EDIT: Vex didn't make any indexing system? grim001 made one some time ago, AutoIndex; Vex has made a standardized interface for all of them to use...

 

[kingkingyyk3]

My question remains though: Why not just use hashtables instead of PUI from now on? If the leaks I ask about don't exist, it could be a valid alternative, no?
You will be doomed :
Example ->
Unit A has handle id of 1. Struct is attached to it.
Unit A died.
Handle id is recycled.
Unit B is created and it has the handle id of 1.
Another struct is attached to it.
GG, the struct on Unit A is leaked.
Slowly, the struct will hitting it's limit.

 

[SerraAvenger]

Another struct is attached to it.

Nonsense.
Perhaps you code so faulty that stuff like that can happen.
Yeah, if unit id recycling cannot happen, then structs may leak - assuming you use structs, of course.

Gladly, there's actually a way around that:
Store the unit in the struct. Theres x posibilities to actually achieve that...

 

[Magentix]

So in short: steer away from hashtables for unit attachment and still use PUI or AutoIndex?

 

[Laiev]

or AIDS

 

[SerraAvenger]

So in short: steer away from hashtables for unit attachment and still use PUI or AutoIndex?

Yeah, PUI and AutoIndex are the way to go.

 

[kingkingyyk3]

In case if you want to use hashtable...

library UnitAttaching
    
    globals
        private hashtable hasht = InitHashtable()
        private integer Keyz = 0
    endglobals
    
    /*
        private struct Data
            implement UnitAttaching
        endstruct
        
        function test takes nothing returns nothing
            local Data d = Data.create()
            set Data[someunit] = d
            set d = Data[someunit]
            call d.release()
        endfunction
        
    */
    module UnitAttaching
        private static integer array ua_id
        private static integer unitKey = 0
        private static integer dataKey = 0
        
        static method operator [] takes unit whichUnit returns thistype
            local integer id = GetHandleId(whichUnit)
            if LoadUnitHandle(hasht,id,.unitKey) != whichUnit and LoadInteger(hasht,id,.dataKey) != 0 then
                call thistype(LoadInteger(hasht,id,.dataKey)).destroy()
                call RemoveSavedHandle(hasht,id,.unitKey)
                call RemoveSavedInteger(hasht,id,.dataKey)            
            endif
            return LoadInteger(hasht,id,.dataKey)
        endmethod

        static method operator []= takes unit whichUnit, thistype whichData returns nothing
            local integer id = GetHandleId(whichUnit)
            if LoadInteger(hasht,id,.dataKey) != 0 then
                call thistype(LoadInteger(hasht,id,.dataKey)).destroy()
            endif
            call SaveUnitHandle(hasht,id,.unitKey,whichUnit)
            call SaveInteger(hasht,id,.dataKey,whichData)
            set .ua_id[whichData] = id
        endmethod

        method release takes nothing returns nothing
            local integer id = .ua_id[this]
            call .destroy()
            call RemoveSavedHandle(hasht,id,.unitKey)
            call RemoveSavedInteger(hasht,id,.dataKey)
        endmethod
        
        private static method onInit takes nothing returns nothing
            set Keyz = Keyz + 1
            set .unitKey = Keyz
            
            set Keyz = Keyz + 1
            set .dataKey = Keyz
        endmethod
    endmodule
endlibrary

Slower than array.

 

[SerraAvenger]

Note that using this might leak structs.

"Yeah, if unit id recycling cannot happen, then structs may leak - assuming you use structs, of course."

I can't really think of a way to stop both struct transporting and 'leakage' at the same time (without using a periodic check...)
Seems you have to choose -.-

//! zinc
library TableUnitIndex {    

    public struct UnitIndex{ 
        static hashtable datatable = InitHashtable();
        unit old;
       
        static method operator [] ( unit u ) -> thistype 
        {
            thistype this = LoadInteger( datatable, 0, GetHandleId( u ) );
            integer old_data;
    
            if (this == 0) {
                this = allocate();
                SaveInteger( datatable, 0, GetHandleId( u ), this );
                old = u;
            }
            else if (old != u ){
                old_data = GetUnitUserData(old);

                SetUnitUserData( old, 1 );
                if ( GetUnitUserData( old ) == 0 ) {
                    FlushParentHashtable( datatable, this );
                }
                SetUnitUserData( old, old_data );
            }
            
            return this;
        }
        
        method operator [] ( integer field ) -> integer 
        {
            return LoadInteger( datatable, field, this );
        }
        method operator []= ( integer field, integer value )
        {
            StoreInteger( datatable, field, this, value );
        }
    }
}
//! endzinc

now this is the closest I can get, but I don't know if it works. The problem is that if "old" is actually acessing the handle id status, we will never see the handle id getting refreshed and this is as bad leakage wise as the one by yyk (old == u in all cases). I haven't tested it and it's just a concept (coded in SciTe, so no syntax checks)

 

[kingkingyyk3]

The problem is that if "old" is actually acessing the handle id status, we will never see the handle id getting refreshed and this is as bad leakage wise as the one by yyk (old == u in all cases)
No... mine does not leak.

 

[Magentix]

Hmm, I get the gist of your system kingkingyyk3... run with me for a second...

So you allow your system to be implemented in structs I intend to attach to units.
Every struct that implements your module gets a unique unit and data key.
That way a unit can have several structs attached to it...

But here's a few questions:

• Do hashtables for Integers and UnitHandles share the same space? If not, why not just use one static key?
• Why not use an onDestroy or destroy method in which you deallocate? That way your users don't have to call release manually
• You use operator overloading to set the unit struct, but from what I can tell, an unwary user could try to attach a different type of struct to Data (in other words, your system syntax seems a bit confusing)

        private struct Data
            implement UnitAttaching
        endstruct

        private struct UnitData
            integer someInt
            // etc
        endstruct
        
        function test takes nothing returns nothing
            local UnitData ud = UnitData.create()
            set Data[someunit] = ud
            set ud = Data[someunit]
            call ud.destroy()
        endfunction

• And last, but not least: Isn't there a major flaw in your system?

// UnitKey = 1
// DataKey = 2
        private struct Data
            implement UnitAttaching
        endstruct

// UnitKey = 3
// DataKey = 4
        private struct Data2
            implement UnitAttaching
        endstruct
        
        function test takes nothing returns nothing
            local Data d = Data.create()
            local Data d2 = Data2.create()
            set Data[someunit] =  d
            set Data2[someunit] =  d2

            // ERROR?: Both Data structs set the data on pointers 3 and 4
            // Your UnitAttaching struct has the pointers staticly,
            // hence the last 2 created pointers will be returned to every struct extending the system?
        endfunction

Or.. supposing all structs get their own copy of the static members:

// UnitKey = 1
// DataKey = 2
        private struct Data
            implement UnitAttaching
        endstruct

// UnitKey = 1
// DataKey = 2
        private struct Data2
            implement UnitAttaching
        endstruct
        
        function test takes nothing returns nothing
            local Data d = Data.create()
            local Data d2 = Data2.create()
            set Data[someunit] =  d
            set Data2[someunit] =  d2

            // ERROR?: Both Data structs set the data on pointers 1 and 2
            // Your UnitAttaching extending structs have the pointers staticly,
            // starting at 1.
            // So every struct extending the system will actually have pointers 1 and 2
        endfunction

Either way, it seems to me that structs using your module will end up saving data to the same hashtable location

 

[kingkingyyk3]

They have unique child key.

       private static method onInit takes nothing returns nothing
            set Keyz = Keyz + 1
            set .unitKey = Keyz
            
            set Keyz = Keyz + 1
            set .dataKey = Keyz
        endmethod

Why not use an onDestroy or destroy method in which you deallocate? That way your users don't have to call release manually.
Else, the struct will leak.

 

[Magentix]

They have unique child key.

Yup, you're right.. Keyz is global and thus the initiation process has incrementing values.

Comments about syntax (onDestroy/destroy and the attaching manner) and shared hashtable space (Integer/UnitHandle) remains though.

Syntax could be a bit more user friendly (perhaps with a wrapper method?)
If [LJASS]SaveInteger(ht,id,key,0)[/LJASS] doesn't share the same space as [LJASS]SaveUnitHandle(ht,id,key,u)[/LJASS], then you only need one key, right?
(Assuming they don't share space, if they do: Disregard this question)

Oh and P.S.: Not giving criticism for no reason, consider me a scrutinizing customer

 

[SerraAvenger]

No... mine does not leak.

      call SaveUnitHandle(hasht,id,.unitKey,whichUnit)

Magentix, you can't solve that.
You have to choose between struct leakage, struct heritage or periodic recycling. FYI: The last solution is the best. It's the one PUI uses.

 

[Magentix]

Yeah, I'm going with AutoIndex...
I like the way it uses event-based recycling