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 Leak when changing innerHTML having dom-bind #4954

Open
6 tasks done
anuragpathak opened this issue Nov 24, 2017 · 7 comments
Open
6 tasks done

Memory Leak when changing innerHTML having dom-bind #4954

anuragpathak opened this issue Nov 24, 2017 · 7 comments

Comments

@anuragpathak
Copy link
Contributor

anuragpathak commented Nov 24, 2017

Description

When we change inner HTML having dom-bind, earlier nodes added are not removed completely hence causing memory leak.
Use Case: In our app we add components dynamically which has data binded with other existing components. To make binding work we change innerHTML in a div (as given in below example)

Live Demo

https://plnkr.co/edit/q0mvZVoXjiSxuVyzIBVn?p=preview (Polymer v 1.11.0)
https://plnkr.co/edit/NrLB0iXLhbzRORwWfeiN?p=info (Polymer 2.2.0)

Steps to Reproduce

  1. Create a component 'test-component'
  2. Create a string having dom-bind component as wrapper and add this component to dom using innerHTML.
  3. Use same string and again change the innerHTML.
  4. Take a heap snapshot at regular interval.
  5. Observe that there is a continuous growth in memory usage.
  6. In below example few nodes are used, causing 1 MB of leak every 40th change.
    In attached snaps observe that there is a memory increase
    First snapshot is taken initially
    Second after replacing innerHTML 40 times (after 1 second each) - see attached PLunker code.
    third after 80th time
    fourth at 120th time.

In below example click on 'Start Adding' button. It will add innerHTML and replace it after 1 second continuously.

I left this code to run for 30 minutes - Memory heap grew from 50 MB to 150 MB.

Browser used for taking heap snapshot - Chrome - Version 64.0.3276.0 (Official Build) canary (64-bit)

Expected Results

Such change should not leak out memory

Actual Results

Memory usage is growing continously

Browsers Affected

  • Chrome
  • Firefox
  • Edge
  • Safari 9
  • Safari 8
  • IE 11

Versions

  • Polymer: v1.11.0 & Polymer:v2.2.0
  • webcomponents: v0 and v1

Please go through the linked plunker code - Polymer v 1.1.4 - Heap taken at 0,40, 80 1nd 120th reset of innerHTML
memoryleak-1 1 4polymer
Please go through the linked plunker code - Polymer v 1.11.0 - Heap taken at 0,40, 80 1nd 120th reset of innerHTML
memoryleak-1 11polymer
Please go through the linked plunker code - Polymer v 2.2.0 - Heap taken at 0,40, 80 1nd 120th reset of innerHTML
memoryleak-2 2polymer

@43081j
Copy link
Contributor

43081j commented Nov 29, 2017

This leak is probably partially explained by your setInterval. It will never be cleared, even after the associated element has been disconnected from DOM. So the number of nodes in memory increases over time. You should be starting the timer in connectedCallback and clearing it in disconnectedCallback.

May not explain this problem entirely, but it explains much of it.

@anuragpathak
Copy link
Contributor Author

Updated code as per you suggestion. Still same amount of memory leak.

@rubenstolk
Copy link
Contributor

@TimvdLippe can you elaborate on the status here?

@TimvdLippe
Copy link
Contributor

Paging @kevinpschaaf and @sorvell who actively maintain Polymer atm

@kevinpschaaf
Copy link
Member

When I run the Polymer 2 version, I do not see memory or dom nodes growing unbounded. With the memory devtools tab open, I see these grow until a gc, at which point they return to within a bounded region. Pressing the "Collect garbage" button (forcing a manual gc) appears to return the heap/dom nodes numbers to the same baseline point. Can you re-confirm?

Also note there was a long-standing memory leak on Polymer 2.x/3.x that was fixed/released in March of this year (#5250) related to the dom-if/dom-repeat debouncer flush queue, but I believe was unrelated to this code since 2.x's dom-bind does not use a debouncer.

@rubenstolk
Copy link
Contributor

Actually we were facing a memory leak in one of our legacy apps but after diving it might be related to another problem, I also didn’t see the issue in the plunkr above... Just assumed two things were related.

I guess you should close this issue.

@stale
Copy link

stale bot commented Oct 7, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants