Gamua / Starling-Framework

The Cross Platform Game Engine
http://www.starling-framework.org
Other
2.82k stars 821 forks source link

Juggler: String allocation when using IAnimatable as a Dictionary key #1098

Closed Adolio closed 1 year ago

Adolio commented 2 years ago

Hey Daniel,

While doing few optimizations by tracking down all possible allocations in my game, I noticed that the Juggler class is actually allocating quite a lot of String in my case.

Apparently, AIR is automatically calling the toString() method of an object when it used as a key in a Dictionary: https://help.adobe.com/en_US/FlashPlatform/reference/actionscript/3/flash/utils/Dictionary.html

From Scout: Scout_Juggler_String_Allocations

Line in question in the Juggler class: Juggler_Allocation_Cause

@PrimaryFeather Do you remember if this dictionary optimization was important the framework or if I could somehow change the code to simply browse the list instead of having the object being indexed?

My goal here is to have more control over the allocations but more importantly for the performance drops related to the garbage collector "passage" 😉

Thanks in advance for your help!

Best, Aurélien

Adolio commented 2 years ago

Since code worth hundred of words, here is an early change proposal: https://github.com/Adolio/Starling-Framework/tree/juggler-memory-imp (to be tested..) 🙂

PrimaryFeather commented 2 years ago

Hey Aurélien!

As far as I remember, there's no specific reason I used the Dictionary in that case – probably I thought it would just be the most efficient way to do it, but that's definitely not the case if AIR is allocating a String for each of those operations. 😱

So your way of doing it is definitely preferable. I can't think of any downsides!

Could you maybe turn this into a pull request? Then I'll happily merge that change into the master branch.

Thanks a lot for making me aware of this! 👍

Adolio commented 2 years ago

Thanks for your answer Daniel :)

Here is the PR: https://github.com/Gamua/Starling-Framework/pull/1099

Happy review!

PrimaryFeather commented 1 year ago

Thanks a lot for the pull request! In the meantime, it has been merged – so I think we can close this issue! 😄 Thanks again!