Global functions generate `MethodClosure` at each frame

[Adolio]

Problem Description

Call to flash.utils.getTimer() generates (at least) one MethodClosure per frame (~18KB / second).

If possible, removing those allocations would help to reduce frame rate drop when the GC is passing by.

Simulation on 600 frames at 60 fps:

Code to Reproduce

package
{
    import flash.display.Sprite;
    import flash.events.Event;
    import flash.utils.getTimer;

    public class Main extends Sprite
    {
        public function Main()
        {
            // Setup targeted frame rate
            stage.frameRate = 60;

            // Register to frame event
            addEventListener(Event.ENTER_FRAME, onEnterFrame);
        }

        private function onEnterFrame(e:Event):void
        {
            //for (var i:int = 0; i < 1000; i++)
                getTimer();
        }
    }
}

Known Workarounds

Unknown

[ajwfrost]

Hi

We've had a quick look here as it depends on what's within that onEnterFrame method to see whether the method closure will get created... it seems like this may be a restriction somehow in how ActionScript/avmplus works! Because getTimer() is a function on its own, rather than being a class member, there is a flag being set to tell the avmplus to generate a method closure for it. I'm not sure whether this is actually valid, or whether it's something that needs to be investigated/corrected, but it's quite low-level stuff.

The simplest option would probably be for all functions like this to be turned into methods, so that they have a normal calling pattern which would then eliminate the problem..

There is actually a workaround:

    private var myGetTimer : Function = null;
    private function onEnterFrame(e:Event):void
    {
        if (!myGetTimer) myGetTimer = getTimer;
        myGetTimer();
    }

which limits there to just being one method closure which is assigned to myGetTimer and then reused.

thanks

[Adolio]

Hi Andrew,

Thanks a lot for the investigations & the provided workaround! This is definitively the root cause of all the remaining MethodClosure that I was tracking down lately.

I do confirm that all functions defined in packages are generating MethodClosure including isNaN and even the utility functions provided by the Starling Framework such as deg2rad or rad2deg (FYI @PrimaryFeather).

Ideally a "fix" in AIR would be very welcome because developers don't always have access to all the calls to those functions when they are embedded in dependencies - and only one call is sufficient to generate the closures...

In the meantime I will do what I could with the workaround. Thanks!

[ajwfrost]

It's basically looking like any method that doesn't have a "declarer" ends up with needing a method closure: https://github.com/adobe/avmplus/blob/master/core/MethodInfo.cpp#L522

But this should really be to do with functions that are called within another context where you want to keep the context available within the function e.g.

var i : uint = 5;
var fnc : Function = function(j : uint) : uint { return i * j; };
var k : uint = fnc(6); // k = 30

So we'll see if we can work this out, for top-level functions that are just contained within a package definition, whether we can eliminate this step.... although a simpler route would just be to create a "GlobalFunctions" class and stick them all in as static methods to this :-)

[Adolio]

Very interesting to see that in the avmplus source code!

Yes - I totally agree with you, this should definitively only be the case for anonymous functions or maybe we missed a tricky special case.

I'm quite surprised that this issue hasn't been raised before. I did some tests and if I run my game at 120 fps I can see a visible freeze every ~15 seconds.

On my side, a fix would be very welcome 🙏

[PrimaryFeather]

Wow, that's an interesting find! It's really curious that this never came up before. I remember that I always wondered where those MethodClosures were coming from, but I was never able to pin it down like that.

It seems it's only three of such functions in Starling (deg2rad, rad2deg, and execute), so it would be possible to create static alternatives for them. However, an optimization that would fix the problem universally would always be preferable. But I can understand this is quite a low level change ...

[Adolio]

@ajwfrost Do you have any update on this issue?

Please let me know if I could help by testing a beta. Thanks

[Adolio]

Hey @ajwfrost - I'm coming back here to see if you have any news regarding this issue. Did you find some time to explore/test a bit more? Thanks in advance.

[ajwfrost]

Hi @Adolio - so this wasn't quite what we thought it was at first.. it looks like there do need to be these closure objects for every 'global' function call. So a class method is called using findpropstrict and then callproperty, but for a global function there's a getlex and then call instruction, with getlex returning a closure object.

However, it looks like these closures are cached in a weak reference table .. and as soon as you go out of scope, these are promptly cleaned up. So we can probably improve things there by keeping a certain number of these objects around based on usage.. will have to see how this works in a "real world" app though!

[Adolio]

Thanks a lot @ajwfrost for the follow up and the fix provided in AIR SDK 50.2.2.1! This was really appreciated.

I did a quick check on my side and everything seems to be looking great - no more method closure allocations at each frame 🥳

Simulation on 600 frames at 60 fps:

Thanks again to everyone involved in this change 👏

[PrimaryFeather]

Wow, that's awesome! A big thanks from me, as well! 🙏

[ajwfrost]

You're very welcome :-)

Now I'm just wondering why on earth there is a separate "enterFrame" event being generated for every frame. As far as I'm aware, there aren't any properties in that event that could be changed, and it doesn't have any 'preventDefault' type behaviour I believe.. assuming it has the same "target" property each time, I'm really not sure why we can't just re-use the same AS3 object each time....

Any thoughts? There may be some background/history to this that I don't know!

[PrimaryFeather]

I was always wondering the same thing – after all, it's not that the event would contain anything useful (like the number of passed milliseconds, for example). Nothing I know would prevent this event object from being re-used!

[ylazy]

@ajwfrost @PrimaryFeather There's a case where we should not use the same object for all enterFrame handlers:

When you want to have something like object.removeAllEventListeners() to automatically remove all events that added to object, including frame-based events.

I'm building a framework that all objects can use .destroy() method to remove all event listeners and can test whether an object is ready to removed completely from memory, if it's not ready, it can list all nested objects that cause the destroying failed. So it's needed for resource management.