Wavesurfer.js: Canvas draw and animation creating Memory Leaks

[gitdisrupt]

Wavesurfer.js version(s):

1.4.0

Browser version(s):

Safari and Chrome

Use behaviour needed to reproduce the issue:

All use cases ...

I was seeing a never ending increase in memory usage when instantiating a wavesurfer instance and playing back an audio file (tried both, as MediaElement and WebAudio) that is NOT released after calling myWave.destroy() to destroy the instance.

At first i suspected the memory leaks had something to do with WebAudio or even the Media element, but then to test this theory, i modified the wavesurfer code and disabled the render of the canvas.

As soon as i disabled the render of the canvas the memory leak was gone.

It seems there are references to the canvas elements, event listeners and animation that are not properly removed on destroy.

[gitdisrupt]

OK EVERYONE!! I can't believe i didn't think to try this earlier ... but i believe i have made a HUGE improvement to performance and memory usage!!

VERY SIMPLY ...
For both the drawBars and drawWave methods, i wrapped their contents with a requestAnimationFrame() call, and this DRAMATICALLY reduced memory usage!!!!

ALSO, at the end of those calls, i cancel the request and NULL the property.

Here is what i did ...

drawBars: function (peaks, channelIndex, start, end) {
        let requestAnimationFrame = window.requestAnimationFrame || window.webkitRequestAnimationFrame;
        var scope = this;
        requestAnimationFrame(function() {
            // contents of the drawBars method here
            // all references to this. changed to scope.
        });
        requestAnimationFrame = null;
       scope = null;
}

And then of course i did the same for the drawWave method as well.

I am betting there are other improvements that can be done as it relates to memory leaks, like nulling properties when they are no longer needed, but i feel like this alone makes for a great improvement.

Hey @entonbiba, can this update be applied to the latest version?

*With all that said though ... i should point out that when destroying a wave instance, the memory that is used is still NOT released ... VERY important to note ... so this issue should remain open.*

I believe though that the memory usage that is never released is likely related to the canvas elements and not necessarily the WebAudio or MediaElement instance.

In the meantime im working to apply the same idea to other methods like fillRect for example

Cheers!!

[entonbiba]

@gitdisrupt looks fine to me, can you create a pull request for it?

[entonbiba]

@gitdisrupt @mspae @katspaugh I say let's create a util method for requestAnimationFrame = window.requestAnimationFrame || window.webkitRequestAnimationFrame; instead and just reference via this.util.requestAF(functionhere(this)); method. This way it will automatically set it to null in the util method.

[katspaugh]

This looks cool, @gitdisrupt, thank you very much!

@entonbiba I agree. Actually setting to null looks like some black magic. Does it really do anything? Doesn't the requestAnimationFrame release the callback (along with the scope) after executing it once?

[agamemnus]

Yes, I see a lot of memory leaks on destroy in relatively young libraries, too.

Setting those values to null won't do anything.

PR coming...

[mspae]

Closing this since the PRs were merged.