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 issue #1

Open
Anurag9589 opened this issue Jan 24, 2019 · 1 comment
Open

memory leak issue #1

Anurag9589 opened this issue Jan 24, 2019 · 1 comment
Labels
enhancement New feature or request

Comments

@Anurag9589
Copy link

It is very nice for scanning using web assembly and very helpful repository, thanks for sharing.

However, I came across following scenario, please read,
when running demo sample application from iPad, it keeps adding memory and at certain point page gets crashed. It clearly gets noticed in iPad, however in Chrome it shows memory getting increased and after some memory limit, chrome also shows crash behavior.

please suggest or improve the memory management.

@jjhbw
Copy link
Owner

jjhbw commented Jan 24, 2019

Hi Anurag, nice find!

I didn't put much thought in testing for memory leaks, so I agree that this is likely a memory leak.

A new wasm buffer is allocated for each new image frame, which is supposed to be destroyed after prediction has finished.

It is possible that this buffer is not freed correctly.
This is the JS code that invokes the buffer freeing method:

// clean up
//(this is not really necessary in this example as we could reuse the buffer, but is used to demonstrate how you can manage Wasm heap memory from the js environment)
api.destroy_buffer(p);

It should call this wasm function, with a pointer to the buffer as an argument.

// this function can be used from the javascript environment to free an image buffer.
EMSCRIPTEN_KEEPALIVE
void destroy_buffer(uint8_t *p)
{
free(p);
}

Probably something went wrong in invoking this function. Sadly, these bindings are pretty fragile and don't throw errors when something goes wrong.

An alternative, more elegant option would be to re-use the buffer for new frames. This should also improve performance a bit.

I currently dont have time to dive into this, if you find the solution, let me know and i'll merge it in if you want.

@jjhbw jjhbw mentioned this issue Apr 6, 2019
@jjhbw jjhbw added the enhancement New feature or request label Nov 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants