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 usage doesn't come down when the map is removed/unmounted from the DOM #9126

Open
iamanvesh opened this issue Dec 18, 2019 · 24 comments
Assignees
Labels
needs investigation 🔍 Issues that require further research (e.g. it's not clear whether it's GL JS or something else) performance ⚡ Speed, stability, CPU usage, memory usage, or power usage

Comments

@iamanvesh
Copy link

iamanvesh commented Dec 18, 2019

While #8771 fixed the memory issue while using the map for prolonged period of time, we found that the memory used by the map is not being garbage collected when it un-mounts from the DOM. Please find attached the screenshots of the memory usage (react app). To test this behavior in isolation I've created a simple app which has only two pages, one of which loads a map onto the DOM and the other just has some text. Please use a resource intensive style URL (which has at least 4-5 layers and >300-500 features) for testing this scenario. Our dataset has >100K features spread across the USA.

Initial load of the app (Map has not been mounted yet):

image

Peak memory usage of the map (after panning/zooming around multiple times):

image

~15 seconds after the map unmounts:

image

When the map unmounts after repeating these steps 4 times:

image

mapbox-gl-js version: 1.6.0

browser: Predominantly on Safari but Chrome retains some memory too.

Steps to Trigger Behavior

  1. Run the app by building the production version of the app [steps in the readme].
  2. Open the home page on the browser and start watching the memory (using Task manager in chrome or Activity Monitor / Safari Timeline). It should be ~50-60MB.
  3. Now open the map page and zoom/pan around until you see the memory grow upto 500-600mb.
  4. Go back to the empty page.
  5. Repeat the steps 3 & 4 for some time and when you finally come back to the empty page after 4th or 5th time, the app retains a significant amount of memory (~500mb).

Link to Demonstration

Without react: https://jsbin.com/zimigow/edit
With react: https://github.com/iamanvesh/mapbox-memory-leak-sample

Expected Behavior

The memory usage should come down to what is observed on the initial page load.

Actual Behavior

The memory usage stays at around ~500mb even after waiting for ~30-60 seconds.

@vakila vakila added performance ⚡ Speed, stability, CPU usage, memory usage, or power usage needs investigation 🔍 Issues that require further research (e.g. it's not clear whether it's GL JS or something else) labels Dec 18, 2019
@vakila
Copy link

vakila commented Dec 18, 2019

Hi @iamanvesh - thanks for using Mapbox & for the detailed report!

You say this is not Safari-specific, you're seeing the same issue in Chrome? I assume you've observed/tested this only on Mac and not other platforms? I wasn't immediately able to reproduce on Linux Chrome but we'll investigate on Mac and see what we can see.

cc @kkaefer @arindam1993 @asheemmamoowala

@iamanvesh
Copy link
Author

iamanvesh commented Dec 18, 2019

@vakila I've seen similar trend on chrome (mac) but the retained memory is negligible. These stats are from Safari. Also, please test the react app, although both the samples do the same, I observed that the react app tends to take some extra time to clean up the used memory.

@kkaefer
Copy link
Contributor

kkaefer commented Dec 19, 2019

Thanks for this report. I created an automated test that creates + destroys the map repeatedly. I could observe growth in the Images section:

image

Does the "resource intensive" style you're referring to include raster tiles?

@kanirudhumar92
Copy link

@kkaefer Just some more info in addition to provided by @iamanvesh
The Custom Style is not having any raster tiles , all the tiles are in vector type.

@iamanvesh
Copy link
Author

I've updated the issue description, our style has more than 100K features spread across the USA.

@arindam1993
Copy link
Contributor

arindam1993 commented Dec 19, 2019

Hi, another thing thats probably valuable to doublecheck is if leaks caused by the Safari Web Inspector itself, I have been bitten by this in the past. Sometimes just closing the Web Inspector ends up releasing a bunch of memory.

@iamanvesh
Copy link
Author

I've tried running the same test with just the activity monitor. Getting similar results more or less.

@iamanvesh
Copy link
Author

iamanvesh commented Jan 6, 2020

@arindam1993 @kkaefer Have you guys made any progress on this issue?

@asheemmamoowala
Copy link
Contributor

@iamanvesh We have been able to verify that there is a small leak related to creating and destroying maps in Safari and are investigating the root cause. We'll update here when we have additional information or questions.

@iamanvesh
Copy link
Author

@asheemmamoowala Any update on the issue?

@badders
Copy link

badders commented Jan 29, 2020

Im seeing this issue still in mapbox 1.7.0, if i create a map, do a map.remove(), create the same map again and repeat this cycle memory useage grows quickly, and even waiting for GC ticks on each cycle average memory usage still continuously grows

@jkoontz2010
Copy link

jkoontz2010 commented Feb 24, 2020

Able to reproduce this on Chrome with mapbox-gl v1.7.0. Just observing the Task Manager shows a sustained ~100-200mb jump in memory usage that doesn't get GC'd every time a Mapbox gets instantiated. I'm able to rack up over 1gb in memory for a single page and over 2.4gb in GPU memory usage by opening and closing modals that contain a mapbox.

Is there any progress or potential fix for this right now?

@arindam1993
Copy link
Contributor

arindam1993 commented Feb 26, 2020

@iamanvesh How are you profiling the memory usage? I'm asking because I'm unable to reproduce on my end.
Here's my profiling setup:
I used an automated version of the page similar to yours(https://jsbin.com/basalocuho/edit?html,output) , it does 100 iterations of adding and removing the map.
I then ran

top -pid {PID} -stats pid,command,time,mem | grep {PID} --line-buffered > safari_log.txt`

replacing {PID} with the PID for the safari tab to generate a logfile of the memory usage of the process in safari_log.txt, this is the exact same data that you would see in Activity Monitor.
Here are the results:
safari_log.txt
Plotting the data in the log looks like this.
Screen Shot 2020-02-25 at 4 54 34 PM
(plotting was done in this observable notebook)

You can see a clear spike when the first map loads, it then plateaus and drops off once the final map is unmounted.

I used top instead of the safari Web Inspector because I found that using the Web Inspector causes a lot of the GC to fail since it appears to retain references to the JS objects in the page. This seems especially true for the gl context for the Canvas, which it seems to retain for the purpose of the Canvas inspector tab.

There could be something in your style or the interactions that you perform that causes a leak, we'll need that information inorder to debug. The only leak vector I could find through blind digging was the one in (#9337) around event handlers. If the style is sensitive information and not sharable publicly, you could reach out to mapbox support with that information.

@iamanvesh
Copy link
Author

I was initially doing it manually with & without (using activity monitor) web inspector. I followed the same procedure that you've mentioned above and here are my observations.

v1.6.0

image

unbind-event-listeners

image

I see that the total memory retained at the end of the process has indeed reduced on #9337 v/s v1.6.0. However, I do see that there's around ~100M memory retained even after a minute has passed by after the execution of the test script. I feel that this is should not be the case (as the page doesn't have any DOM nodes other than a button and a div), and it will be an issue on memory-constrained environments such as iOS Safari. Is it something that can be reduced by playing around with the init configuration? Perhaps maxTileCacheSize?

@arindam1993 Lastly, thanks for your effort in trying to figure this thing out.

@arindam1993
Copy link
Contributor

arindam1993 commented Feb 26, 2020

However, I do see that there's around ~100M memory retained even after a minute has passed by after the execution of the test script. I feel that this is should not be the case (as the page doesn't have any DOM nodes other than a button and a div), and it will be an issue on memory-constrained environments such as iOS Safari. Is it something that can be reduced by playing around with the init configuration? Perhaps maxTileCacheSize?

Yes, but it seems like that the ~100M is also reused when a new map is initialized, otherwise you'd see memory usage keep on growing. Heap snapshots in both Safari and Chrome aren't showing any JS objects being retained as far as I can tell.
I think this is caused by internal data-structures in Safari's runtime that are cached, or internal containers like a std::vector which grow and never shrink back.

Setting maxTileCacheSize will help reduce the overall memory ceiling, gl-js actually has 2 levels of caching, in addition to the in-memory Javascript Object cache we also use the browser provided Cache API to cache tiles. It's possible that Safari's internal implementation of Cache API actually keeps the data in memory, and attributes it to the process. You could also try using mapboxgl.clearStorage() to clear the browser cache used by the library and see if it helps reduce that memory floor.

@iamanvesh
Copy link
Author

iamanvesh commented Mar 5, 2020

@arindam1993 I did try to run mapboxgl.clearStorage() after the test script is done executing but it hasn't made any difference in the retained memory. I still see around ~157M one minute after the test script is done executing, whereas the initial memory before running the script is around ~38M.

I've shared the style URL with the support team, were you able to find any issues with the style, if any?

@arindam1993
Copy link
Contributor

@iamanvesh , I've received the style URL, I'm the process of debugging with it. I don't see any possible memory leaks caused by the style itself. Chrome and Safari Dev tools don't seem to be showing anything different compared to regular mapbox streets.

To answer definitively where this is coming from, I'm going to run profiling on debug builds of WebKit. This is somewhat slow and time consuming but I'll post my finding's here when I have something.

@iamanvesh
Copy link
Author

@arindam1993 Is there any update on this issue?

@arindam1993
Copy link
Contributor

To everyone still following this issue, we don't have any leads on why this is Safari specific, the one we worked around surrounding Transferrable objects in WebWorkers has been acknowledged and fixed by the WebKit team.
https://bugs.webkit.org/show_bug.cgi?id=203990

But no further updates are available.

@joewoodhouse
Copy link
Contributor

joewoodhouse commented Nov 10, 2020

@arindam1993 Any update on this one?

I found exactly the same issue in one of our hybrid applications, and created the same testing setup as @iamanvesh, and then later came across this issue.

I created a basic site that recreates our crash https://blissful-mcclintock-aa92d1.netlify.app/. Basically it just loads a map, starts a flyTo and then removes the map, waits a bit, then repeats the process.

Running this page on device (I'm using iPhone6S+, iOS14.2) Safari will crash after 25 runs or so.

Running this on desktop Safari it takes quite a long time (50 runs or so, depends on available RAM I suppose), but it does eventually crash, with memory increasing constantly. We also see WebGL: INVALID_OPERATION: loseContext: context already lost errors after 15 or so runs.

This bug is a massive problem for us. We use mapbox in a suite of hybrid mobile data collection apps which regularly create new maps as the user is entering data. To have it crash every ~20th time for the user is unacceptable. So we're actively looking into solutions and would be glad to assist in any investigation, even if you have a lead/hunch that you'd like looked at.

Edit: Mildly interesting point to add, I just updated my reproduction site (see link above) to let you choose between running with a vector or a raster layer. With a raster (satellite-v9) layer, the crash takes much longer to happen - about ~120 runs on my iPhone6S compared to ~20 for the vector layer.

@joewoodhouse
Copy link
Contributor

Just to add my own graphs/analysis. This is from running on desktop

Overall Memory

Screenshot 2020-11-10 at 21 35 17

JS Allocation Snapshots over time

Screenshot 2020-11-10 at 21 35 32

Allocation Instances mid-run

Screenshot 2020-11-10 at 21 36 27

Allocation Instances end-of-run

Screenshot 2020-11-10 at 21 36 40

Allocation Instances Detail end-of-run

Screenshot 2020-11-10 at 21 36 47

@felix-mittermeier
Copy link

@arindam1993 Any update on this one?

I found exactly the same issue in one of our hybrid applications, and created the same testing setup as @iamanvesh, and then later came across this issue.

I created a basic site that recreates our crash https://blissful-mcclintock-aa92d1.netlify.app/. Basically it just loads a map, starts a flyTo and then removes the map, waits a bit, then repeats the process.

Running this page on device (I'm using iPhone6S+, iOS14.2) Safari will crash after 25 runs or so.

Running this on desktop Safari it takes quite a long time (50 runs or so, depends on available RAM I suppose), but it does eventually crash, with memory increasing constantly. We also see WebGL: INVALID_OPERATION: loseContext: context already lost errors after 15 or so runs.

This bug is a massive problem for us. We use mapbox in a suite of hybrid mobile data collection apps which regularly create new maps as the user is entering data. To have it crash every ~20th time for the user is unacceptable. So we're actively looking into solutions and would be glad to assist in any investigation, even if you have a lead/hunch that you'd like looked at.

Edit: Mildly interesting point to add, I just updated my reproduction site (see link above) to let you choose between running with a vector or a raster layer. With a raster (satellite-v9) layer, the crash takes much longer to happen - about ~120 runs on my iPhone6S compared to ~20 for the vector layer.

Still experiencing the same problem 👍🏻

Did you fine any solution in the meanwhile? The issue is quite huge for us.

@JuanIrache
Copy link

Any pointers to solve this on MapBox 3+? Memory keeps growing after .remove()

@kamil-sienkiewicz-asi
Copy link
Contributor

@stepankuzmin @mourner Hey, is there any work planned for this issue? I can see that even with most recent mapbox v3.2.0-beta.1 it's kept alive in memory after mounting and re-mounting it few times with proper clean-up (map.remove() and DOM removal, via React to be exact).

image

https://codesandbox.io/p/sandbox/mapbox-leak-zr336q?file=%2Findex.js

In this sandbox it's vanilla JS, simply removing map and then adding it back again. After a few retries it's clearly visible that Map object is not garbage collected and it's piling up in memory Heap snapshot. :<

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation 🔍 Issues that require further research (e.g. it's not clear whether it's GL JS or something else) performance ⚡ Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests