Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak using goToAndStop and destroy #1159

Closed
Jokero opened this issue Jul 20, 2018 · 12 comments
Closed

Memory leak using goToAndStop and destroy #1159

Jokero opened this issue Jul 20, 2018 · 12 comments

Comments

@Jokero
Copy link

Jokero commented Jul 20, 2018

Tell us about your environment

  • Browser and Browser Version:
    Chrome 66.0.3359.181

  • After Effects Version:
    Last version

I have a page in an angular application with around 20 animations rendered by SVGRenderer. And I'm using goToAndStop for displaying specific frame of animations (like preview of an animation).

When I leave that page, I'm calling destroy method for every animation, so all resources should be freed up. But then I'm opening the page with animations several times, at some time the whole page freezes and becomes unresponsive (actually it's always 4th time). The page memory consumption is increasing very fast. In that case I only can kill the page in process manager.

First finding was the typo in property name (https://github.com/airbnb/lottie-web/blob/master/player/js/elements/svgElements/SVGShapeElement.js#L303):

SVGShapeElement.prototype.destroy = function(){
    this.destroyBaseElement();
    this.shapeData = null; // <--- there is no "shapeData" property, it's called "shapesData" (plural form)
    this.itemsData = null;
};

But it didn't help unfortunately. Then I added breakpoint here https://github.com/airbnb/lottie-web/blob/master/player/js/elements/ShapeElement.js#L24 and saw that number of shapes on every page opening dramatically increases:

renderModifiers: function() {
    if(!this.shapeModifiers.length){
        return;
    }
    var i, len = this.shapes.length; // <--- on first open it's 11, then 121 (11^2) and then 1331 (11^3) 
    for(i=0;i<len;i+=1){
        this.shapes[i].sh.reset();
    }

    len = this.shapeModifiers.length;
    for(i=len-1;i>=0;i-=1){
        this.shapeModifiers[i].processShapes(this._isFirstFrame);
    }
}

Looks like not all resources are freed up after animation desctruction and search through shapes is broken.

Hope it will give you some insight into the issue

@bodymovin
Copy link
Collaborator

Hi, can you share a link with an example or the animations you're loading so I can reproduce the case?

@Jokero
Copy link
Author

Jokero commented Jul 21, 2018

@bodymovin I sent you email with link and video with steps to reproduce

@luizgamabh
Copy link

Any solution? I have the same problem. I am adding an animation and destroying after 3 seconds but some garbage is not being collected because each time I fire a new animation my app will being slow, unfeasible, until Chrome tab gets completely frozen.

@bodymovin
Copy link
Collaborator

hi @luizgamabh
Jokero's issue was related to using the same json animation instance for multiple elements.
Since the animation had repeaters, it would increase exponentially on each initialization.
can you share more info about your animation or implementation?

@Jokero
Copy link
Author

Jokero commented Sep 7, 2018

Solution was to always clone original animation data object, what will prevent it from polluting and memory leak. So I used lodash.cloneDeep for that

@luizgamabh
Copy link

luizgamabh commented Sep 7, 2018

I'm also using the same animation for multiple elements, but they are not fired in parallel. I have a timeline and when somebody likes a feed item, I create a new element on the fly:

Something like:

var params = {
    name: name, // name is random UNIQUE name
    container: place, // place is instance of HTMLElement created on the fly and appended to my timeline item
    renderer: 'svg',
    loop: false,
    autoplay: true,
    animationData: animation // animation json
};

lottie.loadAnimation(params);
setTimeout(function () {
    lottie.destroy(name);
    $(place).fadeOut('fast', function () {
        place.parentNode.removeChild(place); // element created on the fly now removed from DOM
    });
    isFunction(callback) && callback.call(null, name);
}, time); // time = 3000

@Jokero
Copy link
Author

Jokero commented Sep 7, 2018

Try to clone always animation object

@Jokero
Copy link
Author

Jokero commented Sep 7, 2018

This fix improved overall performance but my angular page with animations is still freezing for some time due to long rendering and animation destroying. I guess OffscreenCanvas feature #1212 can solve that problem

@luizgamabh
Copy link

My animation json is cached at starting phase of my app, so I never read it again, but I use the same one.

@Jokero
Copy link
Author

Jokero commented Sep 7, 2018

Your cached animation JSON can be mutated by Lottie, so try to clone deeply it every time you create a new animation

@luizgamabh
Copy link

Oh, I see. Thank you Jokero. It solved my problem.

@Jokero Jokero closed this as completed Sep 7, 2018
@zeorin
Copy link

zeorin commented Oct 20, 2020

Sorry to necro this issue, but wow it's wild that Lottie mutates animationData! That should really be documented behaviour. Nowadays with hot reload even if you're not explicitly reusing an instance of animationData, you may still be doing so on hot-reload. In my case on a Gatsby site, but that doesn't really matter.

Deep cloning the animationData worked. Wasted a whole day on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants