Memory allocated by new mapnik.Image() doesn't seem to be freed by GC #950

Activity

1. zdila commented on Jul 6, 2020

   

   zdila

   on Jul 6, 2020

   ContributorAuthor

   More actions

   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.
2. zdila

   added a commit that references this issue on Jul 6, 2020

   added workaround to mapnik-node memory leak (see mapnik/node-mapnik#950)

   4f07d96
3. 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[/+] on Jul 6, 2020
4. springmeyer commented on Aug 8, 2020

   

   springmeyer

   on Aug 8, 2020

   Member

   More actions

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

   

   zdila

   on Aug 14, 2020

   ContributorAuthor

   More actions

   @springmeyer I can reproduce the testcase also in version 4.5.2 (eats 16GB ram in 4 seconds and then starts to swap).
6. artemp commented on Aug 14, 2020

   

   artemp

   on Aug 14, 2020

   Member

   More actions

   @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
7. zdila commented on Aug 14, 2020

   

   zdila

   on Aug 14, 2020

   ContributorAuthor

   More actions

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