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.

Sign up for GitHub

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

Jump to bottom

Memory leaks #95

Closed
denar90 opened this issue Nov 2, 2016 · 9 comments
Closed

Memory leaks #95

denar90 opened this issue Nov 2, 2016 · 9 comments
Assignees
@blasten

Comments

@denar90
Copy link

denar90 commented Nov 2, 2016

I was playing with perf investigations and found that going from home -> list/mens_outerwear -> home listeners and number nodes are increasing. (I can assume that this problem can be for other pages)

Number of listeners increased form 89 to 130, nodes 1881 -> 2532.

home - shop - timeline

Also I made a snapshot for current state and new one, repeating list/mens_outerwear -> home actions.
It looks like there are a lot of detached nodes which have references to window.

home - shop - snapshot

P.S. Maybe before fixing this one it make sense to use drool and it's wrapper for polymer - polymer-drool for testing possible leaks. It requires add package.json to project (probably replacing bower).
@ebidel
Copy link
Contributor

ebidel commented Nov 2, 2016 •
edited
Loading

Here's another profile clicking between "Men's Outerwear" and hitting the back button. Nodes appeared to be GC'd but listeners are growing indefinitely.

screen shot 2016-11-02 at 9 15 59 am

Clicking between the "Men's Outerwear" tab and "Ladies Outwear" tabs:

screen shot 2016-11-02 at 9 18 14 am

@keanulee
Copy link
Contributor

keanulee commented Nov 2, 2016

This may be related to how we use this.importHref on every _pageChanged - you can see that the <head> is filled with HTML imports for the same file, and each one of those is probably adding a listener for "load" (though at least the XHR request is not repeated since HTML imports are de-duplicate).

@denar90
Copy link
Author

denar90 commented Nov 2, 2016

@ebidel yeah plenty of leaks and @keanulee's solution might fix most of them. Other then that I propose to write some tests to find and fix other possible leaks.

@frankiefu
Copy link
Member

frankiefu commented Nov 2, 2016 •
edited
Loading

There is a bug in Polymer.Base.importHref which it's not cleaning up the listeners (load/error) when it's trying to import the same file again. @blasten has a potential fix for this. He is also looking at the other issue which even on the same list page, switching between the "Men's Outerwear" tab and "Ladies Outwear" tab seems to cause the node count to grow.

@blasten blasten mentioned this issue Nov 2, 2016
Closed
@blasten
Copy link
Contributor

blasten commented Nov 2, 2016

This PR should fix the first issue: Polymer/polymer#4124.

I was trying to repro the second issue: Clicking between the "Men's Outerwear" tab and "Ladies Outwear", and it seems like if you click the trash can to force GC, things drop to normal. cc @ebidel

@ebidel
Copy link
Contributor

ebidel commented Nov 3, 2016

Yea, if I force GCs things drop (not completely), but reasonable:

screen shot 2016-11-02 at 5 37 36 pm

@blasten
Copy link
Contributor

blasten commented Nov 3, 2016

yeah. In my test, I went to https://shop.polymer-project.org/list/mens_outerwear and then clicked on ladies_outerwear. Then, I went back and forth between those two pages for a while and eventually stopped back on mens_outerwear, forced GC a few times, and went to ladies_outerwear one last time. In this test, I don't see a memory leak:

screen shot 2016-11-03 at 10 24 39 am

I think the key is to return to state from which the test started, and lastly press the trash can a few times.

@denar90
Copy link
Author

denar90 commented Nov 21, 2016

I recorded my profiling in incognito - https://monosnap.com/file/JDfs7ukxcQSO5cpjrNsRCgURfeKJam

Also here link to timeline-viawer with my profiling. It's not in incognito mode because of strange issue in Chrome, it's not saving timeline data in incognito, but other than that this result shows that issue with memory leak is still present. Maybe it can be helpful.

@blasten blasten closed this as completed Jan 12, 2017
@blasten
Copy link
Contributor

blasten commented Jan 12, 2017 •
edited
Loading

It might need an update. shop.polymer-project.org is running Polymer 1.4.0 and Polymer/polymer#4124 was fixed in 1.6.0 I believe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@blasten blasten

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@frankiefu @ebidel @keanulee @blasten @denar90