[Lua]Obliterate all GUI leaks with 1 trigger!

[Uncle]

Does this still work with the current patch? I've been suggesting using this to new users and I'm worried that it might cause them problems.

 

[Dr Super Good]

Does this still work with the current patch? I've been suggesting using this to new users and I'm worried that it might cause them problems.

Lua garbage collector synchronization should no longer work but also not be required.

The automatic garbage collection of locations, groups and forces should still work. As long as hashtables are not involved with the objects.

 

[Uncle]

Do I have to run this code in anyway? And thanks for the quick response.

 

[Dr Super Good]

Do I have to run this code in anyway? And thanks for the quick response.

Yes. It runs itself if the map is in Lua mode.

I have not tested to confirm that it still works. However I do not see why it should no longer work.

 

[Uncle]

Hmm, so I threw in some: print("Removed Leak") throughout the code and it doesn't appear to be displaying this message.

Edit: Nevermind, it appears to be working.

 

[LtCabble]

Hey Dr Super,

i'm using this System since you released it and it worked fine until reforged. The Debug Print pressing ESC does not work anymore since then (Debug is set to True).

If i put a normal Print("Hello") in function Lua_Test_Action() it works, but nothing else is getting printed anymore. The thing is, i dont even know if the system is working at all because of that.

At the moment i'm trying to get my map working again, because it desyncs at completely random times since reforged (At least i think that's the problem. There are no error messages at all, it just puts you back in the menu).

 

[Uncle]

So after importing this script and Bribe's [Lua] Perfect PolledWait (GUI-friendly) into my map I started getting consistent desyncs when playing online.

I disabled both of these scripts and the desyncs stopped. So I can't say for certain which script was causing it since I disabled both at the same time, but I figure it's worth looking into. I can test just this when I get a chance.

 

[Dr Super Good]

i'm using this System since you released it and it worked fine until reforged. The Debug Print pressing ESC does not work anymore since then (Debug is set to True).

Access to the functions that were used to track garbage collector stats is no longer possible since 1.32 since all the garbage collector stuff is now handled internally and is not a concern for the user.

The actual mechanics of the system should still operate correctly and clean up leaks.

 

[LtCabble]

Luckily i got some testing in and i'm 99% sure now, that the desyncing is caused by the gc script. I tried many things before doing that but nothing helped. After deactivating it we could play some games without problems, one even lasting an hour (Game was over then). We played with 3 and 4 Players.

Interestingly my map did not run into problems at all, even though i removed all custom code to activate lua. My map is extremely leaky now, in old wc3 it would have survived a maximum of maybe 20 minutes.

@Uncle I have Bribe's [Lua] Perfect PolledWait (GUI-friendly) script aswell. So far my map seems to be working again so i asume it's save.

 

[aka Orion]

Hey Super good,
Thank you for the hard work you are doing!

Does this still work? The test map doesnt even load with the "network test" trigger . it runs after i delete it , but i see no signs of it working. Also i created lots and lots of leaky triggers in my map and tested it with and without the garbage collector and my memory usage stayed the same .

Does this work with the same variable changed multiple times in the same trigger(example below)?

Set VariableSet TempGroup = (Units in (Region centered at (Position of (Attacked unit)) with size (00.00, 700.00)) owned by (Owner of (Attacked unit)))

(some trigger funcions with TempGroup)

Set VariableSet TempGroup = (Units in (Region centered at (Position of (Attacking unit)) with size (700.00, 700.00)) owned by (Owner of (Attacking unit)))

(more trigger funcions with the other TempGroup)

@Dr Super Good

 

[Dr Super Good]

Does this still work? The test map doesnt even load with the "network test" trigger . it runs after i delete it , but i see no signs of it working. Also i created lots and lots of leaky triggers in my map and tested it with and without the garbage collector and my memory usage stayed the same .

Yes it still works but due to changes with Lua in Warcraft III I can no longer measure that it is working. Also the Network garbage collector is no longer needed.

If it remains in sync is another question. Some people say it causes desyncs now.

Does this work with the same variable changed multiple times in the same trigger(example below)?

Yes it does work with that situation since it uses Lua's garbage collector to detect when all references to one of the objects are lost.

 

[aka Orion]

Thanks for the quick answer

I will apply it to my new version of my map, and give credit
I wanted to change to this, and lua in general, because my map started giving desync problems after i added simple custom scripts as " call DestroyGroup (udg_group) "

Also lately there seems to be more desyncs in general, so its probably on blizzards side

 

[Dr Super Good]

I wanted to change to this, and lua in general, because my map started giving desync problems after i added simple custom scripts as " call DestroyGroup (udg_group) "

This still calls DestroyGroup and such functions internally. That is the only way to clean up the leak. What this system does is automate the calls rather than requiring they be called explicitly.

 

[Kurodark]

If you don't use GUI triggers at all, do Locations, Groups, and Forces still leak? I've been wondering since this is specifically designed for eliminating GUI created triggers but I'm leaning towards almost 100% custom code.

 

[Dr Super Good]

If you don't use GUI triggers at all, do Locations, Groups, and Forces still leak? I've been wondering since this is specifically designed for eliminating GUI created triggers but I'm leaning towards almost 100% custom code.

It should work for all code. Just if one is using custom script one can explicitly remove the leaks easily anyway making this not so useful.

 

[Francoiglesias]

Hey , can you help me?, i have lots of leaks in my map and i would like to use your sistem, but i cant put lua script lenguage , just JASS , why??

 

[Uncle]

View attachment 354703
Hey , can you help me?, i have lots of leaks in my map and i would like to use your sistem, but i cant put lua script lenguage , just JASS , why??

Warning: This caused Desyncs for me. Last I tried it was about 2 months ago.

Anyway, you can't use Lua and JASS together, that is why Lua is disabled for you.

So if you have any triggers that use JASS and this includes things like Custom script:

• Custom script: call RemoveLocation (udg_Point)

Then you won't be able to make the switch until you do the following:

If you disable ALL of the triggers that use JASS then Lua will become available to you. But you won't be able to Enable them as long as you're using Lua. However, you can delete the JASS stuff and begin using Lua. I would advise against this until this leak removing system is confirmed to be desync free.

 

[Macadamia]

@Dr Super Good :
Although I find your code quite advanced and elegant, I was wondering (correct me please if I mislead myself there) if it wouldn't be easier and safer (I didn't mean better or shorter) to just hook all agent destructors to add a simple "nilling" line ?

I mean if we add that nilling, GC wouldn't need to worry about references to non existing wc3 agents, or did I fail to understand the problem properly ?

I say this because in my map I use simple things like this :

function ReplaceNatives()
    local oldRemoveUnit=RemoveUnit
    RemoveUnit=function(u)
        for i=1,4 do
            if (u==Select[i]) then
                BlzFrameSetValue(BlzGetFrameByName("HPBarEx", i), 0)
                BlzFrameSetText(BlzGetFrameByName("HPBarExText", i), " ")
                Select[i]=nil
            end
        end
        oldRemoveUnit(u)
        u=nil
    end
    local oldRemoveDestructable=RemoveDestructable
    RemoveDestructable=function(d)
        oldRemoveDestructable(d)
        d=nil
    end
    local oldDestroyTrigger=DestroyTrigger
    DestroyTrigger=function(tr)
        DisableTrigger(tr)
        oldDestroyTrigger(tr)
        tr=nil
    end
    local oldDestroyTimer=DestroyTimer
    DestroyTimer=function(t)
        PauseTimer(t)
        oldDestroyTimer(t)
        t=nil
    end
end

I know it's not as elegant, and I also don't use a wide range of agents in my map, but any chance this would work in a map with GUI triggers ?

EDIT :
Ignore the Frame part of my RemoveUnit, it was made to hide dead units custom HP Bars

 

[Dr Super Good]

I mean if we add that nilling, GC wouldn't need to worry about references to non existing wc3 agents, or did I fail to understand the problem properly ?

Local variables do not have to be niled in Lua.

Only reason they had to be with JASS was due to a JASS virtual machine bug where function return did not implicitly decrement reference counters for local declared local variables. Lua does not use reference counters so cannot have such bug to begin with.

 

[Macadamia]

OK, indeed I lost view of the absence of reference counters. I should have thought so, still need to get rid of old bad habits when thinking Lua.

This said I still need to nil removed units to prevent failing IsUnitAlive tests like demonstrated in this thread :
[General] - How to declare UnitAlive native from common.ai in Lua ?
I guess I will remove the other hooks though.

Also I understand the GUI problem is not a variable reference issue but an agent destruction that is never done in GUI, and that answers my question then.

Thanks for the clarifications !

 

[Dr Super Good]

I do not see how niling a local variable will help make a native work.

 

[Macadamia]

It won't change the native's behavior. But it will fix the fact that, after removing the unit, the reference still remains for a short amount of time (a 0.01s timer solves this for sure, but there can be circumstances where you wouldn't want to add a delay before checking if a unit is alive.

OK I admit this is very situational though.

 

[Dr Super Good]

But it will fix the fact that, after removing the unit, the reference still remains for a short amount of time (a 0.01s timer solves this for sure, but there can be circumstances where you wouldn't want to add a delay before checking if a unit is alive.

I still do not understand. Niling a local variable will not change all other references to the unit such as from global variables. One would have to nil all variables that referenced the unit, in which case I do not see how this is a check for a unit being alive anymore...

 

[Macadamia]

Yes that is right, nilling local variables will not remove the other references...
I guess the safest way would then be to always check if the variable is nil before testing if it is alive.

Sorry, I realize I have been away from all this for too long, and I still sometimes get confused for some reason. I suppose it takes more than a few months to alter one's perception to fit a new environment.

 

[Jazztastic]

Excellent find and excellent work as always Dr. SG

My question for you, would a GUI trigger with leaks automatically removed by this script see performance improvements over the same GUI trigger with manual leak removal? For ease of access the automatic leak removal sounds fantastic, but is it worthwhile to adopt this system to retrofit existing triggers?

 

[Macadamia]

What this script does really is automatically add "destructors" for the non-primitive natives as far as I understand.

It was designed to replace the hard work of adding these destructors manually.
So it won't magically make maps more efficient, except when destructors were missing.

Now it seems there are (or were) desync issues with that script. If these desyncs would not happen, this script would simply be godsend for all GUI users as it would let them keep using GUI without having to handle all the necessary custom script lines to remove/destroy unused agents. Of course this would mean change the map scripting language to Lua, but in all honesty that is a good thing considering Lua's Virtual Machine efficiency.

 

[Dr Super Good]

My question for you, would a GUI trigger with leaks automatically removed by this script see performance improvements over the same GUI trigger with manual leak removal? For ease of access the automatic leak removal sounds fantastic, but is it worthwhile to adopt this system to retrofit existing triggers?

It potentially improves performance in the form of reducing leaks if there were any before. It does not improve performance directly. That said it was designed to be trivial to use since it works entirely transparently as long as Lua is the virtual machine.

Of course freshly made maps should still explicitly destroy objects since that would be slightly more efficient than garbage collection.

 

[Prometheus3375]

I have heard that collectgarbage is nil now. If it is true, then the first post should be edited.

Lua garbage collector synchronization should no longer work but also not be required.

But why it is no longer required for online games? Can people use this anti-leak system without stopping GC and running it every few minutes?


If GC syncronization is really no longer required, then this bold warning must be deleted.

WARNING: Garbage collection is run locally for each client. Local object destruction will cause clients to go out of sync. The snippet below can be used to synchronize garbage collection at the cost of disabling incremental collection and potentially causing stutter with complex maps

 

[Prometheus3375]

@Dr Super Good , hey, answer on the previous message, please.

 

[Dr Super Good]

I have heard that collectgarbage is nil now. If it is true, then the first post should be edited.

Yes the approach does not work.

But why it is no longer required for online games?

Blizzard should have added something like it.

Can people use this anti-leak system without stopping GC and running it every few minutes?

I do not know how effective it is or if it remains in sync anymore.

 

[SebioL]

Wc3 is not only famous for the quality of its micro RTS style, but mainly for having a powerful and easy modification tool, capable of creating totally new games and differences in relation to Wc3.
I always say out there, the W.E. is one of the main reasons to attract players of all styles to Wc3.
And it's jobs like this one, which make Wc3 to remain one of the greatest games in history and being played a lot today. Congrats DrSG.

I see a lot of importance in a job like this, because for the most basic users who use the GUI (the vast majority), it creates a safer, easier and more comfortable environment to produce (always hoping, of course, that the tool doesn't clog or break systems, tables, hashtables or any other generic functions that might exist inside the triggers, which makes the task even more challenging for the developer this's system.). No one will be required to be a supergifted in vJASS encoding to make more complex maps, if you have a tool like this, the ease with the GUI opens up a wider range of map creators.
Many associate GUI users as lazy people, because no migrate to vJASS, but many find it really difficult to give it with programming advanced or there is a lack of time to study it further, some can hardly find time useful available for even just playing WC3.
I don't think GUI users should totally give up minimal knowledge of manual leak removal, it's best to know the basics and reduce the main issues to a minimum, but a powerful leak collector can help revive maps that many users have given up on find a solution or eliminate leaks that may have been overlooked because they are small, but that may still pose risks.

Of course this should be a service offered by Blizz, but once again the community takes care of it, as there was in other older work done here,
either for automatic removal or for checking leaks (of course none are as significant and highly efficient). I believe that this one, made by one of Hive's leading experts, could one day become one of the most complete garbage collectors. Apparently we have something more upscale, made by someone with a lot of mastery, among all the tools and systems ever made, this's would be icing on the cake for GUI users.
There are still many obstacles to overcome to make it even more stable and comprehensive in the correction of leaks, I fully support that updates continue to make it even more refined and developed.

 

[Dr Super Good]

From what I could gather automatic garbage collection of objects such as locations and groups was intended to have become a native feature. However due to issues with Warcraft III Reforged's development that never happened.

StarCraft II did include functionality like this natively. Point and Unit Group leaks were never a problem there.

 

[Kyrbi0]

From what I could gather automatic garbage collection of objects such as locations and groups was intended to have become a native feature. However due to issues with Warcraft III Reforged's development that never happened.

And here I thought the wounds could be cut no deeper.

 

[Prometheus3375]

Well, it is possible to rewrite all natives that returns and accepts locations/groups/forces to use Lua tables instead.
For example:

do
  local loc = Location(0, 0)
  local native_GetZ = GetLocationZ
  local native_Move = MoveLocation
  local function GetZ(x, y)
    native_Move(loc)
    return native_GetZ(loc)
  end
  function Location(x, y) return {x = x, y = y} end
  function RemoveLocation() end
  function MoveLocation(l, x, y)
    l.x = x
    l.y = y
  end
  function GetLocationX(l) return l.x end
  function GetLocationY(l) return l.y end
  function GetLocationZ(l) return GetZ(l.x, l.y) end
end

Of course, any native which accepts/returns location must be rewritten.

 

[Dr Super Good]

Well, it is possible to rewrite all natives that returns and accepts locations/groups/forces to use Lua tables instead.

That is a pretty clever/good idea. There might be performance implications due to the required implicit packing/unpacking but I am sure a lot of GUI users would be happy to take it just so their map does not leak.

Assuming the Lua garbage collection is performant enough to deal with all the allocated objects...

There is also the issue of GUI global variables and if they will be happy accepting the tables as a value. Array possibly might not.

 

[Verdun]

So - unless this is something so basic it's enough for even a layman to just study the code a bit to figure it out - is there possibly a "slimmed down" version for this that just detects the leaks in a map?

The threads I found on the Hive so far that talked about detection methods were very old ones that said "the only tool made for this was broken, just use your brain" ...
(just to make sure, I did download and use that tool, but while it looks nice and spits out some data about your triggers, the Leaks detected are always "0" even though I created some leaky triggers on purpose for testing...)

Do we really have no more support tool just for leak check in 2022?
And if not, what would it take to make one...?

 

[Bribe]

So - unless this is something so basic it's enough for even a layman to just study the code a bit to figure it out - is there possibly a "slimmed down" version for this that just detects the leaks in a map?

The threads I found on the Hive so far that talked about detection methods were very old ones that said "the only tool made for this was broken, just use your brain" ...
(just to make sure, I did download and use that tool, but while it looks nice and spits out some data about your triggers, the Leaks detected are always "0" even though I created some leaky triggers on purpose for testing...)

Do we really have no more support tool just for leak check in 2022?
And if not, what would it take to make one...?

[Lua] - Leak Preventer is a better version of this. It replaces some natives with ones that don't generate new handles, and recycles others rather than destroying them. It also only picks up on Group (Unit group), Force (player group), Location (Position) and Rect (Region) leaks, so will still fail if you are leaking other kinds of things.

 

[Dr Super Good]

[Lua] - Leak Preventer is a better version of this.

Technically it is worse. This, when it used to work, had no limits at all and just worked in most cases.

Alas that was not meant to be...

So - unless this is something so basic it's enough for even a layman to just study the code a bit to figure it out - is there possibly a "slimmed down" version for this that just detects the leaks in a map?

Such a thing should be possible in theory. However it might only work reliably in single player.

 

[Bribe]

Well, it is possible to rewrite all natives that returns and accepts locations/groups/forces to use Lua tables instead.
For example:

do
  local loc = Location(0, 0)
  local native_GetZ = GetLocationZ
  local native_Move = MoveLocation
  local function GetZ(x, y)
    native_Move(loc)
    return native_GetZ(loc)
  end
  function Location(x, y) return {x = x, y = y} end
  function RemoveLocation() end
  function MoveLocation(l, x, y)
    l.x = x
    l.y = y
  end
  function GetLocationX(l) return l.x end
  function GetLocationY(l) return l.y end
  function GetLocationZ(l) return GetZ(l.x, l.y) end
end

Of course, any native which accepts/returns location must be rewritten.

I actually just did this in [Lua] Automatic Memory Leak Fixer for Locations , however I haven't moved onto Groups yet. Treating locations as tables is extremely straightforward.

For Unit Groups, I would have to do a fair bit more, such as replace natives like GetEnumUnit. I am not sure about the whole "ghost unit" thing, as ForGroup doesn't iterate over removed units. It may be possible to get away with a table with __mode="v" to allow units to drop off when they no longer exist, but I'm actually unsure if WarCraft 3 objects are collected with the "v" flag (as it might only work with Lua objects). Definitely needs more looking into.

If only the multiplayer sync issue wasn't causing problems with this resource, as hooking onto __gc is a pretty novel way to do this.

 

[Dr Super Good]

If only the multiplayer sync issue wasn't causing problems with this resource, as hooking onto __gc is a pretty novel way to do this.

I am not sure if multiplayer sync is an issue anymore. Just it may not work reliably as the GC sweep speed is fixed. If you create locations or groups faster than the GC can sweep the Lua heap then a GC cycle will never complete and so no garbage collection will occur. A very leaky map would still be very leaky because the garbage collector cannot clean it up fast enough.

 

[Bribe]

I've been doing a number of stress tests comparing this automatic remover with the Location script which replaces locations with Tables, to check the performance delta.

Below is a heavily modified version of your code, which aims to reduce overhead as much as possible (within reason). I also added rects to the automatic cleanup.

do
    GC={}
    local debug = true
    local objRef = setmetatable({}, {__mode = "k"})
    local function registerType(name, remove, constructors)
        local stats = debug and {
            collected = 0,
            manually = 0,
            leaking = 0,
            badfree = 0,
            badconstruct = 0
        }
        local removefunc = _ENV[remove]
        local collecting
        _ENV[remove] = function(obj)
            local table = objRef[obj]
            if table then
                removefunc(obj)
                setmetatable(table, nil)
                objRef[obj] = nil
                if debug then
                    stats.manually = stats.manually + 1
                    stats.leaking = stats.leaking - 1
                end
            elseif debug then
                if obj then
                    print("Tried to free an already freed object: " .. obj)
                else
                    print("Tried to free nil of type: " .. name)
                end
                stats.badfree = stats.badfree + 1
            end
        end
        local collectmetatable = {__gc = function(obj)
            removefunc(obj[1])
            if debug then
                if not collecting then
                    collecting=true
                    print("Automatically garbage collecting up to " .. stats.leaking.. " leaks")
                end
                stats.collected = stats.collected + 1
                stats.leaking = stats.leaking - 1
            end
        end}
        for _,nativeName in ipairs(constructors) do
            local nativeConstructor = _ENV[nativeName]
            _ENV[nativeName] = function(...)
                local obj = nativeConstructor(...)
                if obj then
                    objRef[obj] = {obj}
                    setmetatable(objRef[obj], collectmetatable)
                    if debug then
                        if collecting then
                            collecting=false
                            print("Garbage Collection finished with " .. stats.leaking .. " leaks remaining.")
                        end
                        stats.leaking = stats.leaking + 1
                    end
                elseif debug then
                    print(nativeName.." returned nil: ")
                    stats.badconstruct = stats.badconstruct + 1
                end
                return obj
            end
        end
        GC[name.."Stats"] = debug and function(full)
            print("Statistics for " .. name .. ":")
            print("Garbage collected: " .. stats.collected)
            print("Manually destroyed: " .. stats.manually)
            print("Potential leaks: " .. stats.leaking)
            if full then
                print("Bad free calls: " .. stats.badfree)
                print("Bad constructor calls: " .. stats.badconstruct)
            end
        end or function()
            print("Can only print statistics when GC is running in debug mode.")
        end
    end
    OnTrigInit(function()
        registerType("location", "RemoveLocation", {
            "BlzGetTriggerPlayerMousePosition",
            "CameraSetupGetDestPositionLoc",
            "GetCameraEyePositionLoc",
            "GetCameraTargetPositionLoc",
            "GetOrderPointLoc",
            "GetSpellTargetLoc",
            "GetStartLocationLoc",
            "GetUnitLoc",
            "GetUnitRallyPoint",
            "Location"
        })
        registerType("group", "DestroyGroup", {"CreateGroup"})
        registerType("force", "DestroyForce", {"CreateForce"})
        registerType("rect", "RemoveRect", {
            "Rect",
            "RectFromLoc",
            "GetWorldBounds"
        })
    end)
end

Even with these edits, the game is extremely sluggish when trying to use locations natively. WarCraft 3 uses FAR more memory to instantiate a Location object than it does to instantiate a Lua table, and the FPS throughout the game are absolutely terrible (the motion of the hero on the demo map, and the difficulty in moving the mouse cursor, are the most telling).

Lua tables will also get garbage collected without needing __gc metamethod, making the garbage collection process a bit lighter when it runs.

I've attached a modification to your demo map for testing purposes. To toggle between "Location" and the "__gc metatable" approaches, you can disable the two Garbage Collector triggers and enable Location, or vice versa.

 

[Dr Super Good]

making the garbage collection process a bit lighter when it runs.

If it runs... As far as I am aware the issue is that if you create tables too fast the garbage collector will not be able to keep up and so they will effectively become permanent leaks.

 

[Bribe]

If it runs... As far as I am aware the issue is that if you create tables too fast the garbage collector will not be able to keep up and so they will effectively become permanent leaks.

In the demo that I configured, it's creating tens of thousands of leaks per second, and the garbage collector is still running. A real-world scenario is not going to come anywhere near that amount.

 

[Dr Super Good]

In the demo that I configured, it's creating tens of thousands of leaks per second, and the garbage collector is still running. A real-world scenario is not going to come anywhere near that amount.

Maybe this behaviour was improved by the recent update.