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

GeoJSONWorkerSource leaks memory: it never deletes old WorkerTiles from removed sources/layers #3595

Closed
Que3216 opened this issue Nov 11, 2016 · 9 comments
Assignees
Labels
medium priority performance ⚡ Speed, stability, CPU usage, memory usage, or power usage

Comments

@Que3216
Copy link
Contributor

Que3216 commented Nov 11, 2016

mapbox-gl-js version: v0.26.0

Steps to Trigger Behavior

  1. Open this jsfiddle: http://jsfiddle.net/h93hgmko/1/. The fiddle repeatedly adds and removes a layer and source, each time with a different id.
  2. Take repeated heap snapshots of one of the workers, and you'll see the retained memory usage gradually increase to an unbounded size

Expected Behavior

Since the amount of data on the map never increases, the retained memory usage of the worker should hit a ceiling.

Actual Behavior

Retained memory usage of the worker increases unboundedly:

2a5fd2e3-fbb5-4362-9d40-ed61be855236

If you add a breakpoint in geojson_worker_source.js you can see that the worker is retaining the old WorkerTiles for sources/layers which have since been deleted:

image

It looks like the old data never gets deleted -- the 8 workers collectively have references to all of the data from each previous iteration.

@mourner
Copy link
Member

mourner commented Nov 11, 2016

Thanks for the report! Related issues: #2607 #3220. This is definitely a priority for us — going to address those shortly.

@mourner mourner added performance ⚡ Speed, stability, CPU usage, memory usage, or power usage medium priority labels Nov 11, 2016
@mourner mourner self-assigned this Nov 11, 2016
@Que3216
Copy link
Contributor Author

Que3216 commented Nov 11, 2016

Thanks for prioritising this! Do you have an estimate of when this may be fixed?

@Que3216
Copy link
Contributor Author

Que3216 commented Nov 11, 2016

I tried repro-ing this on the master branch of MGL. The behaviour is slightly different, however it still leaks memory (the retained memory of a worker goes up over time). It now correctly erases old WorkerTiles in the loaded hash, however it still leaks GeoJSONVT's in the _geoJSONIndexes:

On MGL 0.26.0:

2a1a3531-e5af-4102-b3e6-f5dbef67d2cc

On master:

e0184c88-c7b8-4d5f-9814-9e95f9c5d1d0

(good)

ca601440-deea-4924-a2a7-c637355cb902

(bad)

@Que3216
Copy link
Contributor Author

Que3216 commented Nov 11, 2016

Each GeoJSONVT seems to be separate objects, and so are taking up mem: _geoJSONIndexes["points_1"].tiles[0].features !== _geoJSONIndexes["points_2"].tiles[0].features

@Que3216
Copy link
Contributor Author

Que3216 commented Nov 11, 2016

I've pushed a quick PR that removes old sources from _geoJSONIndexes #3602. It appears to help, however the retained memory on the worker thread still grows over time. I can't spot any remaining problems immediately by browsing the references from the worker.

@Que3216
Copy link
Contributor Author

Que3216 commented Nov 12, 2016

These fixes appear to slow the memory growth down by an order of magnitude.

@Scarysize
Copy link

Scarysize commented Jan 11, 2017

Sorry to comment on closed issue. Do/Did raster sources have an similar issue (memory leak) when they are being removed?

What we found to be problematic is the retaining of textures, when removing/re-adding many sources:

https://github.com/mapbox/mapbox-gl-js/blob/master/js/render/painter.js#L308

@mourner
Copy link
Member

mourner commented Jan 11, 2017

@Scarysize this seems to warrant a new GitHub issue. Can you report?

@Scarysize
Copy link

Will do. Thanks for the fast response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium priority performance ⚡ Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests

4 participants