Leaflet.glify icon indicating copy to clipboard operation
Leaflet.glify copied to clipboard

Memory leak in instance arrays

Open melgrove opened this issue 2 years ago • 2 comments

There is a pretty large memory leak when adding multiple instances of Points, Shapes, or Lines.

Reproduce

  1. When points(), shapes(), or lines() is called, it adds a Points, Shapes, or Lines instance to the global pointsInstances, shapesInstances, or linesInstances array. Each instance contains all of the layer information.
  2. When .remove() is called on one the instances (for example: let pointLayer = glify.points({...}); pointLayer.remove()), the instance is not removed from the global instances array. This causes a very large memory leak when rendering and rerendering points many times, since the entire object for every render stays in memory after it has been removed.

A solution

My application must be able to rerender many points many times so I created a workaround. I think that this could be added to the .remove() method, unless I'm missing something.

  1. Store an ID with the instance when creating it:
    let pointLayer = glify.points({...});
    const savedID = generateUniqueID(); // just needs to be unique across the instances
    pointLayer.layerID = savedID;
    
  2. When .remove()ing the instance, also remove it from the instance array manually:
    const instanceIndex = glify.pointsInstances.map(inst => inst.layerID == savedID ? true : false).indexOf(true);
    glify.pointsInstances.splice(instanceIndex, 1);
    

melgrove avatar Jun 04 '22 07:06 melgrove

It doesn't look like there have been any updates to the package since I created this issue so I don't think so, unless you are referring to something else. I'm probably going to PR a fix and add it to my fork

melgrove avatar May 02 '23 21:05 melgrove