ml5js / ml5-library

Friendly machine learning for the web! πŸ€–
https://ml5js.org
Other
6.46k stars 902 forks source link

[utils] Memory Leak in ml5.flipImage() ? #803

Open AliKarpuzoglu opened 4 years ago

AliKarpuzoglu commented 4 years ago

Dear ml5 community,

I'm submitting a new issue. Please see the details below.

β†’ Step 1: Describe the issue πŸ“

When running a classifier on a flipped video using p5.js I have notied that ml5.js keeps creating a new canvas all the time. Is this intended? The page gets huge, even if the previous canvas gets a display:none What do you think?

β†’ Step 2: Screenshots or Relevant Documentation πŸ–Ό

Here's some helpful screenshots and/or documentation of the new feature https://github.com/googlecreativelab/teachablemachine-community/issues/39 Here is a similar issue, I run almost the same code. For a live demo see :

https://editor.p5js.org/AndresCuervo/sketches/W7jI_BWNY The new canvas gets created each time flipimage is called Maybe it's possible to remove the previous canvas when creating a new one?

This was my version, but I have created a workaround - deleting a lot of the newly created canvases - heres a demo: https://alikarpuzoglu.com/snake_ml var canvases = document.getElementsByTagName("canvas") for (i = 2; i < canvases.length-5; i++) { canvases[i].remove(); I have chosen the arbitrary number length-5, but I needed to start at 2 because otherwise I would delete the important canvas

β†’ Step 3: Share an example of the issue πŸ¦„

Here's some example code or a demonstration of in https://github.com/ml5js/ml5-examples OR in the https://editor.p5js.org or codepen/jsfiddle/etc...

https://editor.p5js.org/full/tqoOkW_ai

is the sample I am using

https://editor.p5js.org/AndresCuervo/sketches/W7jI_BWNY This also shows the created canvas

Other relevant information, if applicable

β†’ Describe your setup πŸ¦„

Here's some helpful information about my setup...

AliKarpuzoglu commented 4 years ago

I think you can close the issue, I just saw that the elements get removed when you call flippedImage.remove() after classification.

joeyklee commented 4 years ago

@AliKarpuzoglu Many thanks for catching this issue. @shiffman and I will take a look at a better way to try and handle this behind the scenes. Let's leave this open for now until it is resolved. Thanks again!

@all-contributors please add @AliKarpuzoglu for bug maintenance

allcontributors[bot] commented 4 years ago

@joeyklee

I've put up a pull request to add @AliKarpuzoglu! :tada:

shiffman commented 2 years ago

Hi all! I'm checking in on some older, unresolved issues today. Is adding a call to remove() to the examples and the documentation a solution for this? This one seems less like an issue with ml5 and more just the way canvas works with making a new temporary image each frame? If it's no longer a relevant issue we can also close!

joeyklee commented 2 years ago

Hi all! I'm checking in on some older, unresolved issues today. Is adding a call to remove() to the examples and the documentation a solution for this?

  • as a quick fix, I think this would be a good way to go! πŸ‘

As a longer term fix, we might need to go in here and update how we handle those returned canvases 😬