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

Web app memory leak #48

Closed
dabreegster opened this issue Jul 30, 2021 · 6 comments
Closed

Web app memory leak #48

dabreegster opened this issue Jul 30, 2021 · 6 comments
Assignees

Comments

@dabreegster
Copy link
Contributor

Robin reported that the browser tab slows down quickly after dragging the marker around. Confirmed with Firefox's about:performance page.

clockboard.remove();
seems to be leaking the old layer. I've tried variations here from Leaflet/Leaflet#1416, https://gis.stackexchange.com/questions/221354/removing-layer-and-redrawing-it-again-in-leaflet, and a few other places, but nothing works yet. The docs (https://leafletjs.com/reference-1.7.1.html) also don't explain anything about memory management of layers; it's unclear who owns a layer after doing addTo(map).

I miss Rust already. :(

@dabreegster
Copy link
Contributor Author

@dabreegster dabreegster self-assigned this Jul 30, 2021
@Robinlovelace
Copy link
Contributor

Thanks for diagnosing this and confirming the issue. If only there were a Rust crate that could do the interactive web mapping thing and not rely on JS...

@dabreegster
Copy link
Contributor Author

There's probably some workaround for Leaflet; I just haven't found it, and because of my unfamiliarity with JS/the browser, I'm lacking the experience to easily dive into this. I'm confident it's solveable.

If only there were a Rust crate that could do the interactive web mapping thing and not rely on JS

The point of the web app is to use standard libraries like Leaflet, not reinvent something new! What's awesome is how easy it's been to glue the two approaches together.

@dabreegster
Copy link
Contributor Author

function labelZones(feature, layer) {

I think the problem is here -- we're cleaning up the main geojson layer, but not all of the markers we create for tooltips for each zone label! I'm still wrapping my head around the Leaflet memory model. When you add something to the map, I'm unclear who's supposed to own and clean it up. I think layer groups (https://leafletjs.com/examples/layers-control/) are meant to manage this. Will experiment.

dabreegster added a commit that referenced this issue Aug 3, 2021
…aw zone labels checked in, and it was creating a bunch of markers without ever cleaning them up. I'm covered in shame. #48
@dabreegster
Copy link
Contributor Author

Very dumb error on my part -- those markers were my attempt to draw permanent labels in the zones, instead of just tooltips. I never figured out the permanent labels, but I didn't clean up the experimental code. It was creating markers and never cleaning them up.

The leak is now fixed, but now it reveals that the new search bar doesn't quite work. I'll get that fixed, or reopen the issue to keep tracking it.

@Robinlovelace
Copy link
Contributor

Thanks Dustin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants