Juggler: Reduced memory allocations by removing the Dictionary implementation for object ids

Gamua / Starling-Framework

The Cross Platform Game Engine

http://www.starling-framework.org

Other

2.82k stars 821 forks source link

Juggler: Reduced memory allocations by removing the Dictionary implementation for object ids #1099

Closed Adolio closed 1 year ago

Adolio commented 2 years ago

This should prevent many String allocations

Changes

The _objectIDs is now a Vector.<uint> containing the objects' ids
The cells cleaning logic as been factorized by using the new removeByIndex method
object in _objectIDs have been replaced by contains(object)
The cells re-oganization (in the advanceTime method) logic has been updated accordingly

This PR needs further testing. Changes are working on my side but I'm not sure I have covered all cases.

PrimaryFeather commented 1 year ago

This is really an exemplary pull request, Aurélien! Thanks a lot!

I really like how you got rid of the ugly code duplication I had in there whenever an object was removed. I'm actually a little embarrassed that it wasn't implemented like that originally! 🫢

Thankfully, the Juggler class is extensively unit tested – there are a few traps in there which can easily be overlooked, but I think all of them are covered by the unit tests. And those are running just fine after merging your changes.

So, everything is looking great! Thanks a lot for your careful implementation, Aurélien! 😄👏

Adolio commented 1 year ago

Thanks Daniel for your careful review 😁

I'm very happy that you liked it, this means a lot to me coming from you! ☺️