lit / lit-element

LEGACY REPO. This repository is for maintenance of the legacy LitElement library. The LitElement base class is now part of the Lit library, which is developed in the lit monorepo.
https://lit-element.polymer-project.org
BSD 3-Clause "New" or "Revised" License
4.49k stars 318 forks source link

(Chrome dev tools - not lit element) Memory leaking - happens with lit-element README example too #1090

Closed cintaccs closed 3 years ago

cintaccs commented 3 years ago

Does anyone else than me see memory not being completely cleaned up with lit-element?

dynamicai-dialog-login js file:

import { LitElement, html } from 'lit-element';

class dialogLogin extends LitElement {
    render() {
        return html `<p>hello world</p>`;
    }
}

customElements.define('dynamicai-dialog-login', dialogLogin);

test.html page:

<!DOCTYPE html>
<html>
<head>
    <script type='text/javascript' src='DynamicAI_Login.js?version=7.0.1234' charset='utf-8'></script>    
</head>
<body>
    <dynamicai-dialog-login></dynamicai-dialog-login>
</body>
</html>

Initial:

Screenshot 2020-10-13 at 04 39 46

After pressing Ctrl R (page reload) approx: 100 times:

Screenshot 2020-10-13 at 04 41 07

Snapshot:

Screenshot 2020-10-13 at 04 43 50

Manual garbage collect have no effect.

"lit-element": "^2.4.0",
"lit-html": "^1.3.0",
"@rollup/plugin-node-resolve": "^9.0.0",
"rollup": "^2.29.0",
"rollup-plugin-delete": "^2.0.0",
"rollup-plugin-license": "^2.2.0",
"rollup-plugin-minify-html-literals": "^1.2.5",
"rollup-plugin-strip-banner": "^2.0.0",
"rollup-plugin-terser": "^7.0.2",

Rollup config extract:

export default [
    {
        input: 'src/DynamicAI/dynamicai-dialog-login.js',
        inlineDynamicImports: true,
        output: {
            file: 'app/DynamicAI_Login.js',
            format: 'es',
            sourcemap: false
        },
        plugins: [
            resolve(),
            minifyHTML(),
            terser(),
            strip({
                functions: ['console.log'],
            }),
            license({
                thirdParty: {
                    output: {
                        file: path.join(__dirname, 'app', 'dependencies.txt'),
                        encoding: 'utf-8', // Default is utf-8.
                    },
                },
            }),
            stripBanner()
        ],
        preserveEntrySignatures: false,
    },

Browser: Chrome Version 86.0.4240.75 (Official Build) (64-bit) OS: Windows 10 Pro version 2004 - build: 19041.508

This example shows a small problem - reloading 100 times is not really a use-case - however with a bigger application - the memory can grow to crazy heights... I have seen 8-900 MB ....

cintaccs commented 3 years ago

Using the

https://gist.githubusercontent.com/sorvell/48f4b7be35c8748e8f6db5c66d36ee29/raw/67346e4e8bc4c81d5a7968d18f0a6a8bc00d792e/index.html

referenced on the README for lit-element - the same memory stacking - (leak) - is occuring.

Seems to me that there is a problem to be addressed

cintaccs commented 3 years ago

@sorvell - hi sorvell - sorry to bother you with a direct message - but this "leak" really bothers me for potentially any lit-element implementation (even the one provided as example on this repo) - it should take you less than 5 minutes to verify whether you believe that there is something that needs investigating (which I am willing to do also - if I get some hints)... again I don't want to bother - just want to move forward in the best direction for lit and lit-element. Thanks in advance!

justinfagnani commented 3 years ago

@cintaccs thanks for the report and the included details, but please understand that we can't always respond to issues within a day or two. Also, it's not helpful to add many exclamation points and question marks. That might actually discourage people from looking at your issue, which is probably the opposite of your intention.

We have seen similar reports to this in the past, and in the past it's always been a Chrome bug. There is generally no way for userland script to hold onto memory between page loads. The last such report was due to a leak in Chrome when using adoptedStyleSheets: https://github.com/Polymer/lit-element/issues/804 This was fixed in Chrome 78: https://bugs.chromium.org/p/chromium/issues/detail?id=1002763

However, I failed to reproduce this leak. I used the gist you posted above and reloaded the page a large number of times. The heap snapshot seemed to grow about .1 MB per ~100 reloads and I didn't see any evidence of the growth in HTMLDocument like you have.

In order to chase this down we're going to need to make a minimal reproduction. First, we need to make sure that no extensions are running, since those might be able to hold references to documents across page loads. Then we'll need an idea of what's retaining the documents, following the shortest path int he retainers panel of the snapshot. Then we can try to build a reproduction that doesn't ideally include Rollup or LitElement.

cintaccs commented 3 years ago

@justinfagnani Thanks for the feedback - my apologies if you feel offended by the "???" - as I understand you once had a presentation in Copenhagen - and in this part of the world - we do not take things too serious.... kind of easy going not trying to accuse or offend anyone by it - just a style/cultural thing that maybe differ. :-)

I have full understanding for the fact that "my" issues are not addressed immediately - however as a good advocate for your solutions yourself - you have convinced me and many others I assume - to base a lot of work on your platform, vision and architecture - that sometimes reduce the number of places to look. (I obviously did not post the issue above before narrowing it completely down to the bare minimum as in your own example file (the gist)). Again - all I want to achieve is to get a better and more solid solution as I assume everyone else would like to. I will double check again and post my findings here. My initial test was using an IIS localhost/ site to serve the

https://gist.githubusercontent.com/sorvell/48f4b7be35c8748e8f6db5c66d36ee29/raw/67346e4e8bc4c81d5a7968d18f0a6a8bc00d792e/index.html

I will try some other variants and get back here with my findings.

Thanks for responding - and sorry if I offended anyone - that was not my intention at all!

cintaccs commented 3 years ago

Ensuring that I don't have any weird extensions....

Screenshot 2020-10-14 at 23 35 29

and eliminating localhost / iis potential impact - by just loading the file (see html file content below) from disc.

Screenshot 2020-10-14 at 23 03 00

where test2.html served as a file includes the following:

<!doctype html>
<html>
<head>
</head>
<body>
  <!-- Works only on browsers that support Javascript modules like
       Chrome, Safari, Firefox 60, Edge 17 -->
  <script type="module">
    import {LitElement, html, css} from 'https://unpkg.com/lit-element/lit-element.js?module';

    class MyElement extends LitElement {

      static get properties() {
        return {
          mood: {type: String}
        }
      }

      static get styles() {
        return css`.mood { color: green; }`;
      }

      render() {
        return html`Web Components are <span class="mood">${this.mood}</span>!`;
      }
    }

    customElements.define('my-element', MyElement);
  </script>

  <my-element mood="great"></my-element>

</body>
</html>

The 20.2MB builds up after repeated Ctrl R reloading the page.

here is what is network:

Screenshot 2020-10-14 at 23 07 01
justinfagnani commented 3 years ago

I was able to reproduce this after upgrading Chrome from 85 to 86. But I was also able to reproduce it without any scripts in the page at all. A completely blank page retains documents on reload.

cintaccs commented 3 years ago

I agree! I just got kind of the same by eliminating the lit-element part

Screenshot 2020-10-14 at 23 54 52
cintaccs commented 3 years ago

So I guess it is a Chrome "bug" of some sort.... I haven't seen the behaviour before lately (thought it potentially could be after upgrading to the newer lit versions) ... but it seems lit is (as always before) solid and error free! :-)

cintaccs commented 3 years ago

it seems related to "Developer tools" being loaded.... i.e. if you open a new browser instance - without the dev tools - and reload the page a lot of times - then open the dev tools - then the page memory will be small. Then when you reload - it will start building up the memory issue.

I stumbled over something named "data_view_fun"

Screenshot 2020-10-15 at 00 09 08
cintaccs commented 3 years ago

not sure whether this is something I would know where to report to... but I agree with you it is not related to lit at all. I apologize for the inconvinience! Thanks for directing me to the origin of the problem!