crisbeto / angular-svg-round-progressbar

Angular module that uses SVG to create a circular progressbar
https://crisbeto.github.io/angular-svg-round-progressbar/
MIT License
741 stars 173 forks source link

Memory leak when drawing progress #221

Open Blackvals opened 3 years ago

Blackvals commented 3 years ago

Hi, We are using this component at version 3.0.1 on our application with angular 8 and I have been investigating a memory leak narrowing it down to this component. Let me give some context on how we use this component.

So, with the UI on idle state from a user perspective but the round-progress being updated, I could see memory increasing (even considering garbage collection after some time). Reproducing the same scenario but removing your component from the screen makes memory stable. I tried also with the demo application in latest version (6.0.1) and after toying around for some time I could see that memory went up (checking chrome task manager). Even in one case on the demo app I found memory going crazy up without me bothering it anymore, to later be cut down by the GC (I assume; this rare situation could be just Chrome though). I have briefly looked at your code but I have not seen a clear reason behind it, though I'm no JS/TS expert.

Could it be a configuration problem? It is a problem to have matching duration and timer updates? What happens if animation hasn't finished and a new change occurs?

crisbeto commented 3 years ago

Honestly there isn't that much that could cause a memory leak in the component since it's basically taking the values of a bunch of inputs and generating an SVG out of them. The one place that is slightly risky is the animation based on requestAnimationFrame, but if that were leaking, you'd see a spike in CPU usage, rather than a memory leak (see https://github.com/crisbeto/angular-svg-round-progressbar/blob/master/src/lib/round-progress/round-progress.component.ts#L109). There is also some logic that should prevent overlapping animations.

One thing to try is to update to the latest, or at least a later, version of Angular which may contain a fix for your leak.

Blackvals commented 3 years ago

Thanks for your quick response. I agree with you, the only part on your code that brought me doubts was the requestAnimationFrame precisely for being a recursive call based on the duration. As for the CPU spike, I was not checking it because the main resource problem was memory related, but it could be.

Following that trail, I have tried reducing the duration to the minimum (1) and the memoy stays in check. In the end, if we are updating the progress every second for a operation in the range of minutes or hours, it is barely noticeable the fancy animation; so is a trade-off we can pay.

Unfortunately the Angular update is something not possible at the moment, but something we look forward for the near future.