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 allocated by new mapnik.Image() doesn't seem to be freed by GC #950

Open
zdila opened this issue Jul 6, 2020 · 5 comments
Open

Comments

@zdila
Copy link
Contributor

zdila commented Jul 6, 2020

Our server is regularily killed by kernel because it eats all the RAM. It seems to be caused by not freeing unreferenced images created by new mapnik.Image(...);

Testcase:

for (let i = 0; i < 1000000; i++) {
  new mapnik.Image(256, 256);
}
@zdila
Copy link
Contributor Author

zdila commented Jul 6, 2020

I am not sure if this issue affects only image or other types, for example mapnik.Map, mapnik.Color. Also if image.resize() returns new image then maybe it is also never freed.

zdila added a commit to FreemapSlovakia/freemap-mapserver that referenced this issue Jul 6, 2020
@zdila zdila changed the title Memory allocated by new mapnik.Image() doesn't seems to be freed on GC Memory allocated by new mapnik.Image() doesn't seem to be freed by GC Jul 6, 2020
@springmeyer
Copy link
Member

@zdila can you try the v4.5.1 release to see if it improves your memory usage? Based on my testing, it should.

@zdila
Copy link
Contributor Author

zdila commented Aug 14, 2020

@springmeyer I can reproduce the testcase also in version 4.5.2 (eats 16GB ram in 4 seconds and then starts to swap).

@artemp
Copy link
Member

artemp commented Aug 14, 2020

@zdila - This is not a memory leak but rather design issue/feature

mapnik.Image is allocating external memory : https://github.com/mapnik/node-mapnik/blob/master/src/mapnik_image.cpp#L195 but GC is not aware of this. The way to inform GC about these allocations is to call Napi::MemoryManagement::AdjustExternalMemory(env, size); when image allocated and Napi::MemoryManagement::AdjustExternalMemory(env, -size); when finaliser is called. Those calls don't come for free, though.

It's more efficient to do something like

var mapnik = require('../');
for (let i = 0; i < 1000000; i++) {
  new mapnik.Image(256, 256);
  if (i % 1000 == 0)
  {
    global.gc();
  }
}

and not call Napi::MemoryManagement::AdjustExternalMemory for ever new mapnik.Image(...) . I guess this was behind original motivation for this implementation.
/cc @springmeyer

@zdila
Copy link
Contributor Author

zdila commented Aug 14, 2020

Thanks for the explanation. To prevent memory leaks we already implemented reusing mapnik.Image instances in our project.

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

3 participants