Polymer / polymer

Our original Web Component library.
https://polymer-library.polymer-project.org/
BSD 3-Clause "New" or "Revised" License
22.04k stars 2.01k forks source link

Polymer 2 Memory Leak in IE11 #4511

Closed alvarezguille closed 2 years ago

alvarezguille commented 7 years ago

Description

Reloading Polymer 2 apps on IE11 generates a spike in memory consumption. Also app fails to load one third of the times. The memory leak was initially reported for Polymer 1 and webcomponentsjs v0 in:

Steps to Reproduce

Simplest way to reproduce it is with Polymer 2 stater kit app

  1. polymer init
  2. select polymer-2-starter-kit
  3. polymer serve
  4. open in IE11

(also verified with latest commit of 2.0-preview branch of polymerelements/polymer-starter-kit)

Expected Results

App loads correctly and can be reloaded multiple times without errors. And there is no memory leak for each reload.

Actual Results

Memory spikes and doesn't recover. One third of times app doesn't load.

Browsers Affected

Versions

bahrus commented 7 years ago

I see the memory go up by ~14 Megs with each refresh, but it never fails for me. That's from replicating the tests above (but not sure if I have the same exact version of polymer-cli).

Fortunately, I'm not seeing memory climb with pure polymer 2 (nothing hybrid) pages. Do you?

dfreedm commented 7 years ago

@bahrus @alvarezguille How are you making memory measurements?

If I use the IE 11 developer tools, I can see what you are describing, but using Task Manager and watching IE memory usage, I can't with any regularity.

Also, I can't replicate the "One third of times app doesn't load" statement without the dev tools open.

bahrus commented 7 years ago

I did not have dev tools open. I monitored the memory usage via the details tab of task manager. I also noted, on the positive side, that if I repeatedly opened the site in different tabs, which would spawn (slowly) more iexplore.exe processes, those processes did flush after a minute or so, after closing all those tabs..

The test where I saw memory climb was refreshing within one tab. I'll try the test some more with all the latest.

bahrus commented 7 years ago

I'm embarrassed to say that when I ran the tests last time, I actually ran them with the Polymer1-starter-kit. In my weak defense, the cli highlights in bright green the one below the selection. It highlights the selected one with a more muted blue color (and a > to the left). Maybe this is standard for cli's... Anyway...

I updated my polymer-cli, and reran the steps tests alvarezguille (more carefully this time).

I monitor the column "Memory ( private working set)" in the Task Manager. I also disabled add-ons in internet explorer.

When I first load Polymer-2-starter kit in IE11, it settles down around 50Megs. 4 refreshes later, it climbs to ~80 megs. 4 more refreshes and it is at 110 Megs. I closed IE11 and tried this three times, same approximate results each time.

So with Polymer2-starter-kit, my memory climbs 7 Megs each refresh, not 14.

dfreedm commented 7 years ago

@bahrus Can you say which version of IE you have or version of Windows?

bahrus commented 7 years ago

11.1066.14393.0 Update Versions: 11.0.41 (KB4014661)

Windows 10 Home 1607

OS Build 14393.1066.

dfreedm commented 7 years ago

Hmm, I have

11.0.15063.0 Update Versions: 11.0.41 (KB4012204)

Windows 10 1704

So I wonder if this is fixed with the Creator Update to Windows 10 :/

bahrus commented 7 years ago

Very possible. I'll test again when I get it, and try my other PC's as well.

dfreedm commented 7 years ago

I can also try spinning up another VM with something more closely matching what your environment is.

bahrus commented 7 years ago

Now that the same laptop I was testing with above has updated to Creator Update, running the same tests (including the same version of polymer as before), I find that memory only climbs approximately 400 kbytes per refresh. This seems to be in line with other web sites I try. Much better!

alvarezguille commented 7 years ago

I've been testing with modern.ie VMs and checking memory with task manager. The memory is not the only issue here. The fact that the compiled app fails to load every now and then is affecting us more. @azakus, should I create a separate issue for the refresh part?

ronnyroeller commented 7 years ago

The fact that the compiled app fails to load every now and then is affecting us more

+1, same issue here

ShaggyDude commented 7 years ago

When you guys say compiled app you mean javascript compiled down to ES5 right? My app is failing to load every time now as well.

xanister commented 6 years ago

Beyond the memory leak, Polymer 2 seems to run scales slower on IE11 than Polymer 1. Is there any plan to improve performance on IE11 for Polymer 2.x?

rbjarnason commented 6 years ago

@xanister I would also ask the same question and voice my disappointment about the performance on IE11 after upgrading to Polymer 2. While the past 12 months usage of IE11 has dropped from 4.3% to 2.2% many of our partners are local governments around Europe that many are stuck with old IT infrastructure featuring IE11. We are even considering relaunching the Polymer 1 app and serve it to IE11 users, but this is a major inconvenience for a small nonprofit open source developer. Performance on Edge is also not great, which is another 2.1% of our users - on the upside everywhere else the performance is excellent.

TimvdLippe commented 6 years ago

Per https://github.com/Polymer/polymer/issues/4511#issuecomment-293992301 and https://github.com/Polymer/polymer/issues/4511#issuecomment-298484976 we were unable to definitively reproduce the memory leak as stated in the first comment. To resolve this issue, we require a small reproduction case that shows the memory leak in IE11 that we can confirm and then fix.

Regarding other performance issues in IE11, we require a reproduction case as well which show significant performance regressions compared to other comparable websites. Most notably, we require reproduction cases which show the measured performance numbers regressing. If you can provide a minimal reproduction case, please open a separate issue (please look through other existing issues as well, as it might have already been tracked), as this issue is tracking the memory leak instead.

rbjarnason commented 6 years ago

@TimvdLippe I'm sure the IE11 performance is acceptable in most "minimal reproduction cases" but our problem is that our feature rich citizen participation app has become unusable on IE11 after we upgraded to Polymer 2. Many features result in long delays including getting "long running script" popups.

We're a tiny non-profit and having to go back to also maintaining a Polymer 1 version of the app is daunting but the app not working on IE11 is causing us a lot of problems and we must solve this.

We've built the app using best practices through Polymer 0.5 then 1.0 and now 2.0. We have optimized everything we can think of. The app is blazing fast on Chrome and Safari but unusable on IE11 and slow on Edge. Please help...!

Here are live production servers running the Polymer 2.0 version: https://yrpri.org/ https://betrireykjavik.is/ https://consultation.parlement-ouvert.fr/group/4

Here is the source code: https://github.com/CitizensFoundation/your-priorities-app

rbjarnason commented 6 years ago

Just to add we do get one JavaScript error in IE11 on load but probably not connected - this seems to cause the initial theme to fail to load causing color problems but you can still click through and too slowly use the app without errors.

image

image

TimvdLippe commented 6 years ago

@rbjarnason Hm this is tough, as I feel like the Chrome version is a bit slow as well (not trying to name and shame, but it might explain why IE11 is running into issues as well). For example, when I run the performance audit on https://yrpri.org/domain/3 in Chrome Dev Tools (https://developers.google.com/web/updates/2017/05/devtools-release-notes#lighthouse), I see a 35 performance score with several issues: image What jumps out to me is the large number of DOM nodes (6k, while a good target would be 1500). I can imagine that having a lot of DOM nodes makes the polyfills doing a lot of work. Traversal of the DOM is expensive and the polyfills make heavy use of that.

My initial guess would be that by reducing the amount of DOM you generate, as well as lazy-loading more content should significantly speed up your website on both modern browsers and IE11. (I see you are bundling a lot in yp-app which is a total of 1.05 MB https://github.com/CitizensFoundation/your-priorities-app/blob/master/client_app/build/bundled/src/yp-app/yp-app.html)

Would you mind taking a stab at these issues, trying to work on them and report back if the speedup on Chrome is similar or not to IE11? If there is a similar speedup, this is great. If there is not (in other words, IE11 performance is still horrible, but Chrome has sped up), we can do a deep dive what the difference could be. Please let us know if you are unsure on how to improve or tackle these issues!

rbjarnason commented 6 years ago

Hey @TimvdLippe thanks for this! :) Good point to look at the DOM nodes size.

After the quite time consuming Polymer 2 upgrade we spent about 2 person months to get first meaningful paint from 32 sec to under 5 sec in the Audit tool - in reality on most modern phones its less than 2s - this was a major effort but gave us good results.

The nature of our citizen participation projects, where citizens have real influence on local and national government policy, gives us low bounce rates even in the loading time is not fully optimal on the slowest devices and the app works fast after its loaded except on IE11 and Edge. We still want it to go faster but most of the development of the app is volunteer based at the moment and there only 24 hours in the day so we're highly resources constraint. We would not go ahead to make it faster on Chrome (for most it now loads almost instantly) except to fix this IE11 problem. We have a long list of features we want to implement with our limited time but this would further compound the IE11 problem.

For all practical purposes the app works fast on Chrome, Safari and Firefox even with this node count and worked ok on IE11 and Edge in Polymer 1 with the more or less the same element count from the app side.

I realize that we use a lot of web components but even on a simpler page with a lot less showing in the UI then this page has over 7.200 DOM nodes, 2192 HTMLElements and 4916 HTMLDivElements. Only a few hundred DOM elements, at most, are visible but still counts over 7.200 in the Audit. https://betrireykjavik.is/post/17249

Further help in tackling those issues is greatly needed and appreciated!

Do all the web components we lazy load and their structures contribute to the DOM nodes size? Do components loaded with Service Worker contribute to this count?

We're already using the PRPL pattern but turns out that each of my lazy loads has a lot of common elements making the first download so large. I lazy load all dialogs that the user could click on and slice them by regular users, admins and special feature dialogs. What more can I do here?

Can I somewhere in the Chrome debug tools see what HTMLElement classes have the most instances? I guess the HTMDivElement classes are divs used in the HTMLElement classes.

Is the Shady DOM polyfill key suspect in the performance regression with IE11 on Polymer 2?

image

image

rbjarnason commented 6 years ago

Hmm, while the elements are actually lazy loaded using HTML Import then I'm using iron-pages that I thought did not instantiate the elements but looks like they do when I found your element here ;) Going to test it. https://github.com/TimvdLippe/iron-lazy-pages

EDIT: I actually checked the source code it looks like the element only adds the dynamic loading part, still using only hidden. Correct me if I'm wrong. Maybe I could wrap each of my top level iron-pages elements in a dom-if to only be true when the page is selected?

I'm also waiting on a 500mb IE11 profiling file to open in VS, will report on that later.

rbjarnason commented 6 years ago

Initial results from the IE11 profiling.

image

TimvdLippe commented 6 years ago

@rbjarnason I think the very first low-hanging fruit is your usage of iron-list. Initially I suspected you were not using the element, given the very large rendering of elements. However, after closer inspection it appears you ARE using the element. However, as pointed out at https://github.com/PolymerElements/iron-list#sizing-iron-list you have to explicitly size the list height. On https://yrpri.org/domain/3 the iron-list in yp-community-grid does not have an explicit height, and it defaults to 6000px height. If you set this to something like 90vh, you should be able to render a LOT less dom elements.

the app works fast on Chrome

Do you have any data based on your target audience? I am unable to reproduce these results. Granted, the website loads reasonably fast on my development machine, on lower-end machines I am not seeing the same results. IE11 on a slow machine will be definitely a lot harder to optimize.

Do all the web components we lazy load and their structures contribute to the DOM nodes size? Do components loaded with Service Worker contribute to this count?

Every element you render in the DOM will be counted, including in shadow dom. So yes they will be counted.

We're already using the PRPL pattern but turns out that each of my lazy loads has a lot of common elements making the first download so large. I lazy load all dialogs that the user could click on and slice them by regular users, admins and special feature dialogs. What more can I do here?

You could split up the bundles manually. In other words, create some common dependencies .html file where you specify the dependencies and how to load them. Then bundle the dependencies in that specific file and lazy-load it. In general, I think reducing the amount of custom elements you define and use is a good approach. Sometimes the large amount of layers will be a big performance issue.

Looking at Polymer.telemetry in the console, there are 2189 instances of custom elements while there are only 88 custom element definitions on the page, that is quite a lot. In comparison, https://www.youtube.com/ has 1500 instances for 500 definitions.

Is the Shady DOM polyfill key suspect in the performance regression with IE11 on Polymer 2?

That is quite hard to say sadly, there could be multiple factors. In general, for every custom element you define the polyfill has to do some setup work. Then for every instances (especially with the usage of CSS mixins) there needs to be additional style computations. I would first focus on the DOM nodes issue with iron-list before doing other profiling.

Hmm, while the elements are actually lazy loaded using HTML Import then I'm using iron-pages that I thought did not instantiate the elements but looks like they do when I found your element here ;) Going to test it. https://github.com/TimvdLippe/iron-lazy-pages

While I am glad you discovered this element, it is not a golden solution and I was not specifically advocating for the element in this issue. That said, if the element can help you out, that is of course great :smile:

TLDR: I have a big suspicion that fixing the iron-list issue will actually solve most of the problems :pray:

LarsDenBakker commented 6 years ago

@rbjarnason We also have a very large and complex application in Polymer, and noticed a similar performance degradation in IE11 and Edge when upgrading from Polymer 1 to Polymer 2.

I spent a lot of time looking into this, and dom size plays a huge role. Another huge degrading factor is the amount of shadow dom you use on your page. If you use shadow dom just for UI components (buttons, toggles etc.) it's fine but if like us you use shadow dom for your entire application structure it can really add up. This seems to especially degrade performance on IE11 and Edge, and I think is a missing factor in many benchmarks.

It's a shame that many of the performance degrading factors, are default settings or advocated patterns when using Polymer. Each polymer element automatically uses shadow dom, and you really need to know what you're doing to not use it. Also common design patterns of conditionally showing elements (iron-pages, hidden attribute, dom-if without restamp) keeps elements in the document just hidden with display: none. Non-visual elements (iron-ajax etc.) also add load to the document and trigger unnecessary polyfill processing despite not being a web component. (I never understood those elements to be honest).

Some of the polymer elements themselves are quite bloated, with several layers of composition for a simple UI element. This is a leftover from Polymer 0.5 times, where class inheritance was not possible.

The main thing we did to improve performance is to look at our main page and to make sure that only the things that visible are actually rendered and in the DOM. Everything else is rendered lazily, by using things such as dom-if or a custom pages component which is quite similar to iron-lazy-pages.

Also we looked at the level of dom composition, especially in heavily repeated elements. People often use web components for things that could also easily be solved through simple CSS classes. Simplifying these things really helped us shave off seconds. An example of this is the paper-material element, which is deprecated now in favor of CSS.

In the end we were able to go down from 30+ sec loadtime to 4sec. I think that's still a bad experience in my opinion, so we're still looking to improve.

I love Polymer, and I think there is something good in IE11 making the weak spots of our application more apparent to us. But I also feel like the difference between IE11 + Edge and the other browsers is really huge. Especially the fact that it's almost as slow on Edge. Chrome and Safari, both of which have native shadow dom, are blazing fast. Firefox is a bit slower, but not too bad.

rbjarnason commented 6 years ago

Thanks @TimvdLippe - I will look into the iron-list on the front page and your other suggestions. I was aware of this iron-list requirement and thought I had fixed this everywhere in the app but this is sadly not the problem. If you look at this page then it has two iron-lists with only 3 elements total but this page has over 7200 DOM elements, that is more than the front page. https://betrireykjavik.is/post/17249

rbjarnason commented 6 years ago

Thanks @LarsDenBakker!

LarsDenBakker commented 6 years ago

@rbjarnason One of your culprits is the dialogs container, which has active dom nodes for all possible dialogs. Even though they're not shown, the polyfill has to write all these up in the DOM. We had the exact same issue actually :)

screen shot 2018-04-14 at 4 28 43 pm

rbjarnason commented 6 years ago

@LarsDenBakker @TimvdLippe This is great news, thank you both so much! :) It looks like we have paths to first a quick fix by wrapping top level iron-pages elements in dom-if with restamp! And to add dom-if also to the dialog system which should be quick as its quite well abstracted in a single file. Then a second path to simplifying repeated elements that use elements like iron-ajax and paper-material - I will report the results here later.

TimvdLippe commented 6 years ago

@LarsDenBakker Thank you very much for your insights and debugging the dialog issue. Much appreciated!

Definitely agreed that heavy usage of the DOM-heavy elements can run into performance problems. Light-weight elements for these use-cases could be more appropriate. We are doing research on these elements and how we can improve them, so hopefully we can publish some results in the near future. In the mean time, the proposed solutions could already shows some improvement, hopefully enough.

rbjarnason commented 6 years ago

@TimvdLippe @LarsDenBakker Success! After implementing iron-lazy-pages with dom-if restamp both for the top level pages and for the dialogs we're down to 1,362 DOM nodes from 6,230 when not logged in and when admin from - gulp - 18,547 notes to 9,012 nodes (More work to figure out what admin features can be instantiated on demand) but a great start.

I will report after testing this on IE11 and Edge.

image

rbjarnason commented 6 years ago

Ok, the app is lot faster on Edge and IE11 :) I will provide detailed numbers later but I'm getting two errors in both browsers and one that makes some dialogs fail to load using the new pattern. One seems to have started to appear somewhere along the way of webcomponentsjs updates and has to do with Shady DOM and results in the color scheme not be fully loaded with "Invalid argument". The other makes the dialog fail to load is a dom mutation error (NoModificationAllowedError) maybe connected to https://github.com/Polymer/polymer/issues/4878

image

image

There is a live staging server here: https://staging-test.idea-synergy.com/group/2

It's also faster on Chrome and in the audit tool. image

@TimvdLippe Seems that blocking stylesheets are the main problem now, any ideas?

image

I wonder why fonts are loaded twice: image

rbjarnason commented 6 years ago

@TimvdLippe I've created a new issue to track the Invalid argument crash in Edge and IE. https://github.com/Polymer/polymer/issues/5191

TimvdLippe commented 6 years ago

Glad the website is loading a lot faster now, thanks for the detailed investigation :tada:

stale[bot] commented 4 years ago

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[bot] commented 2 years ago

This issue has been automatically closed after being marked stale. If you're still facing this problem with the above solution, please comment and we'll reopen!