Memory leaks

[bhousel]

Opening this issue because I dug into it a bit, but don't have more time this week.

iD leaks memory pretty bad. This relates to #2743 - performance as a user makes many edits - but the causes and solutions are different.

We expect iD's memory usage to grow in certain ways as a user edits:

1. Scrolling the map downloads more entities from OSM which get added to the base graph.
2. Each edit makes a new immutable Graph, and they get stacked up in history.
3. Changing an Entity makes a new immutable Entity.
4. Caches (Graph's parentWays/parentRels/childNodes, History's rbush, Features' internal cache)

But we expect memory to be garbage collected at other times. D3 should remove() DOM elements from exit selections as things are no longer needed. This applies to:

1. SVG elements on the map
2. Everything in the inspector / entity editor / sidebar
3. Modals

We are sometimes lax about calling that remove, and other times remove can't do its job properly because of references elsewhere in the code.

In that screenshot from the Chrome heap profiler, red things are DOM elements that are detached but can not be garbage collected, yellow things are DOM elements that we still refer to somewhere.

The screenshot shows that the event handler which toggles the raw tag editor is still holding a reference to a D3 selection of the raw tag editor itself. So D3 remove() detached it from the DOM, but it stuck around in memory anyway.

A good summary of what's happening can be found on the Chrome dev tools page, especially this graphic:

This is an easy problem to study, because all you need to do is take a heap snapshot, select/deselect a thing on the map so that the entity inspector shows and then clears, then take another heap snapshot and compare the two. But that's just one memory leak in one UI element, and the bigger problem is that this is a widespread "death by a thousand cuts" antipattern everywhere in iD.

We need to be smarter about removing stale references to things in:

• D3 selections
• Event handlers
• DOM element __data__
• closures that mash all of the above together

Also related: https://github.com/mbostock/d3/issues/1041 mbostock's take on this: "well, duh"

[jfirebaugh]

The presence of detached DOM nodes in memory is not in and of itself a bug. There are cases where for performance reasons, retention is intended, so that the DOM nodes can be re-attached when the UI component becomes visible again, rather than rebuilt from scratch. I believe this is the case for some of the things in the "inspector / entity editor / sidebar".

[gy-mate]

Unfortunately this is still an issue (using Safari 17.3). iD uses 4+ GB of memory after opening the editor, making & saving edits at different places and closing the editor (going back to OSM Carto) several times without closing the tab itself. This makes iD unresponsive for several seconds after a single click on an element.