rungwiroon / BlazorGoogleMaps

Blazor interop for GoogleMap library
MIT License
309 stars 99 forks source link

Map pages consumes a lot of Memory in browser #270

Open turner11 opened 1 year ago

turner11 commented 1 year ago

When I plot a lot of circles, the browser task manager shows that the memory footprint of the page gets high very quickly.
In this pull request, I implemented the IAsyncDisposable pattern as per MS documentation in order to try and reclaim the memory, but even after everything is disposed the memory stays high.

I suspect that items on the JavaScript side are not being disposed properly, but I'm afraid its a bit out of my comfort zone.

I have added a demo page for reproducing the issue (/dispose-circle-list) Examples:

Memory before plotting: image

Memory After plotting 6k circles: image

Memory after plotting 6k circles and disposing circles & map: image

valentasm1 commented 1 year ago

Sadly not enough time to help.

Nickztar commented 1 year ago

This is a somewhat known issue if I am not mistaken, see #191 with a difficult solution and not a lot of time going around to fix it.

TheAtomicOption commented 2 months ago

(quoting from #284) I am not sure if reusing map object should be library responsibility. It could make other issues. Maybe having as option to reuse could be one way. Also Polygon and polyline dont have "remove" method. Only event listeners have it. Despite all i think it is issue with memory consumption and release on google maps and parts could be solve from our side.

Correct, this library should not reuse polygons or other objects automatically. I only mentioned reuse as a workaround that users might use for the current version of the library since it does not dispose of polygons with delete.

BGM ought to ensure that JS garbage collection will dispose of objects after C# disposes of the JSInterop handle, since there is no way for C# to reconnect with these JS objects. I know for sure that happens if you call delete [objectName] in JS, as we do for event listeners. I'm less certain but I don't think that merely setting a polygon's map attribute to null is sufficient (happy if I'm wrong here).

IIRC when a map is disposed, we loop connected objects such as markers and delete them. However we still currently have a JS memory leak for anything other than event listeners which aren't connected to a map when their C# handle is disposed as no map's dispose loop will find them.

valentasm1 commented 2 months ago

Map dispose function does some deleting. I read somewhere that dispose object could be enough to set it to null. Maybe set object to null and then remove from map element array. Maybe there is bug in this code. I think @TheAtomicOption since you know more, you could play around and maybe will find the best solution. Memory leak investigation is always hard :).

 disposeMapElements(mapGuid) {
     var keysToRemove = [];

     for (var key in mapObjects) {
         if (mapObjects.hasOwnProperty(key)) {
             var element = mapObjects[key];
             if (element.hasOwnProperty("map")
                 && element.hasOwnProperty("guidString")
                 && element.map !== null
                 && element.map !== undefined
                 && element.map.guidString === mapGuid) {
                 keysToRemove.push(element.guidString);
             }
         }
     }
     for (var keyToRemove in keysToRemove) {
         if (keysToRemove.hasOwnProperty(keyToRemove)) {
             var elementToRemove = keysToRemove[keyToRemove];
             delete mapObjects[elementToRemove];
         }
     }

     if (controlParents !== null && controlParents.hasOwnProperty(mapGuid)) {
         const map = mapObjects[mapGuid];
         for (let position in map.controls) {
             this.internalRemoveControls(mapGuid, position);
         }
         delete controlParents[mapGuid];
     }

     if (controlParents !== null && Object.keys(controlParents) == 0) {
         controlParents = null;
     }
 },