vscode icon indicating copy to clipboard operation
vscode copied to clipboard

reduce impact of memory leaks related to editor

Open SimonSiefke opened this issue 1 year ago • 0 comments

Helps with #218077

Changes

This PR changes the editor dispose function to recursively clear all child elements of the editor dom node.

DOM nodes hold strong references to parent node, sibling nodes and child nodes. When a memory leak occurs in a child component of the editor (e.g. Minimap, GlyphMarginWidgets, TextAreaHandler, ...) the editor dom (linked to the child dom) is leaked as well.

By recursively clearing all editor child elements, only the child elements will be leaked.

Reducing impact of memory leaks

While this doesn't fix any memory leak, it reduces the impact of memory leaks related to the editor. For example, when opening an editor 197 times, the number of leaked dom nodes is much lower than before:

Metric name Before After
detached canvas element count ~800 0
detached dom node count 14307 937
leaked object count ~16400 ~2000

Making it easier to find the cause of editor memory leaks

With less leaked dom nodes, it could make it easier to find the cause of memory leaks related to the editor. For example, these are the detached dom nodes now when opening an editor 197 times:

{
  "detachedDomNodes": {
    "after": [
      {
        "className": "HTMLDivElement",
        "description": "div.lines-content.monaco-editor-background",
        "count": 399
      },
      {
        "className": "HTMLDivElement",
        "description": "div.margin",
        "count": 399
      },
     {
        "className": "HTMLSpanElement",
        "description": "span.codicon.codicon-sync.codicon-modifier-spin",
        "count": 10
      },
      {
        "className": "HTMLDivElement",
        "description": "div.orthogonal-drag-handle.end",
        "count": 8
      },
      {
        "className": "HTMLDivElement",
        "description": "div.orthogonal-drag-handle.start",
        "count": 7
      },
      // ...
    ]
  }
}

It seems there is a memory leak related to the editor background and/or editor margin component.

Performance

Clearing all dom nodes recursively has a performance overhead of ~1ms to ~1.8 ms on my machine. On the other hand, garbage collection could potentially be faster.

Test Results

The number of canvas elements now stays pretty much constant:

canvas-count-2

SimonSiefke avatar Jun 30 '24 14:06 SimonSiefke