Large memory leaks.

[ngokevin]

Describe the bug

Hey! Been using anime in A-Frame, an open source WebVR framework. Love it. Though there is a lot of memory leak that can lead to GC pauses.

Taken from a few seconds from the anime homepage, it's allocated hundreds of kilobytes of memory garbage.

This issue can be alleviated for free with these practices:

• Cache regexes (https://github.com/juliangarnier/anime/blob/master/src/index.js#L69 https://github.com/juliangarnier/anime/blob/master/src/index.js#L358) rather than allocating new one each call
• Use for loops rather than Array.forEach which allocates an entire function (https://github.com/juliangarnier/anime/blob/master/src/index.js#L779)
• Try to avoid array helpers such as Array.map and Array.filter and Array.reduce and Object.keys as these will allocate a new array (https://github.com/juliangarnier/anime/blob/master/src/index.js#L1209 https://github.com/juliangarnier/anime/blob/master/src/index.js#L321).
• Avoid allocating arrays in critical codepaths, reusing a helper / auxiliary array when possible. A single array can be reused by clearing the length array.length = 0; (https://github.com/juliangarnier/anime/blob/master/src/index.js#L959 https://github.com/juliangarnier/anime/blob/master/src/index.js#L308)
• Try to avoid cloning objects (https://github.com/juliangarnier/anime/blob/master/src/index.js#L337)

This is something I could try to help out with.

To Reproduce Steps to reproduce the behavior:

1. Go to anime homepage (or any anime animation).
2. Open devtools memory / performance.
3. Record memory.

Expected behavior Little memory garbage / allocations.

Desktop (please complete the following information):

• OS: macOS
• Browser firefox

[juliangarnier]

Hey thanks for pointing this ! Is it something only related to V3 or was already the case in V2.x? Also is it just me or Chrome doesn't have this issue?

[ngokevin]

It was an issue with v2 as well, I just noticed.

Chrome should have the same issue. Here's the homepage over about five seconds in Chrome tools.

Allocations by JS call (would be easier to dissect if unminified). Can see some pieces of code allocating hundreds of KBs or MBs. (String.split, Array.map, etc):

Sawtooth / GC pattern:

[juliangarnier]

Alright, I'm going to focus on this for 3.1. I created a branch called memory plumbing, any help on this is welcome! Thanks !

[ItsJonQ]

any help on this is welcome! @juliangarnier Awesome! I'll poke around and see how I can help refactor.

I've used Anime.js in the past - haven't in a while! I'm a huge fan of the library. It's an amazing feat of engineering to so many animations in such a tiny package.

If I were to create a PR, would it be against the dev-memory-plumbing branch? Thanks!

[ItsJonQ]

@ngokevin This is amazing feedback! Would love to see some of these solutions! Admittedly, I only have a bit of experience with some of these low level performance fixes you suggested. I've already learned a thing or two 😊

[juliangarnier]

@ItsJonQ simply PR to the dev-memory-plumbing branch :) Thanks !

[mubaidr]

@juliangarnier Can you we please also add, some lint configuration to make sure all contributions share same style/practices. (eslint + prettier?) Easier for code reviews, issue identifications.

and What about tests? Would it be suitable to add tests?

[ItsJonQ]

@mubaidr That would be nice :).

@juliangarnier I can help with this if you're cool with it :). Maybe not tests (as that we be a larger thing). But I can certainly help with prettier/eslint, and/or making the dev flow a little more seamless

[juliangarnier]

@mubaidr eslint + prettier sounds like a good idea. Regarding tests, I'm not really sure where to start to be honest. Is it really possible to properly automate testing on an animation library since a lot of the tests rely on visual results? Do you have any testing library recommendation?

@ItsJonQ Totally, eslint + prettier setup would be nice!

[mubaidr]

@juliangarnier then i would be taking on tests.

We can test all methods exported by this library, states during and after timing functions etc

If you agree, i can research and provide my findings on this.

[juliangarnier]

@mubaidr Of course, I'm open to any suggestions on this subject. Thanks!

[ngokevin]

Would be a separate issue, but we have unit tests for animations for A-Frame on top of anime (https://github.com/aframevr/aframe/blob/master/tests/components/animation.test.js#L4), and for most of A-Frame which is a VR library (also visual).

We test many things, that animation configs are parsed and set up properly, test that if we tick an animation past its duration that it reaches its end value, if we tick an animation mid-way that the values are between the from and to, test that animation callbacks are called, that the animation reverses with alternate or reverse, etc.

[mubaidr]

@ngokevin that is the goal. Thanks for confirmation.

[juliangarnier]

@ngokevin The latest changes break the code, I merged without double checking...

[ngokevin]

Whoops, I'll check it out. Thought I did a quick pass, but might've not got the latest build through.

[tedmeftah]

I see this issue become more about improving the code base, hopefully you don't mind me asking this here @juliangarnier what are your thoughts about moving the code to typescript?

[ngokevin]

I believe this issue should be constrained to memory leaks versus unit testing, linting, and Typescript.