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

[Core]: PrimeVue 4 is causing a memory leak #5957

Closed
luke-z opened this issue Jun 24, 2024 · 14 comments
Closed

[Core]: PrimeVue 4 is causing a memory leak #5957

luke-z opened this issue Jun 24, 2024 · 14 comments
Assignees
Labels
Package: Core name: @primevue/core Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@luke-z
Copy link

luke-z commented Jun 24, 2024

Describe the bug

Hello

Today we discovered a memory leak caused by the primevue nuxt(-module) / SSR.
Edit: As the problem arises with the core package, I changed the title :)

It appears to be caused by two lines:

app.provide(PrimeVueSymbol, PrimeVue);

ThemeService.on('theme:change', function (newTheme) {

-> This event listener is getting registered on each page refresh without being removed. So there will be more and more instances of this event listener.

If I comment out these two lines in the compiled server.mjs and restart the nuxt server, no memory leak is happening when doing the steps mentioned in the reproduction area.

Reproducer

https://github.com/luke-z/primevue-4-memory-leak

PrimeVue version

4.0.0-rc.2

Vue version

3.x

Language

TypeScript

Build / Runtime

Nuxt

Browser(s)

No response

Steps to reproduce the behavior

  1. Build the project using yarn build
  2. Run using node .output/server/index.mjs
  3. Send several requests to the index page on localhost:3000 (I used ab -n 10000 -c 100 localhost:3000/ to get fast results)
  4. Check the RAM usage -> I used the --inspect flag and launched the Node DevTools with Brave/Chrome to see the memory usage

Expected behavior

No memory leak

@luke-z luke-z added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Jun 24, 2024
@luke-z luke-z changed the title [Nuxt]: nuxt-module is causing a memory leak [Core]: PrimeVue 4 is causing a memory leak Jun 25, 2024
@mertsincan
Copy link
Member

Hello @luke-z and @RoyeL12,

in #5960, @RoyeL12 says this issue has gone in v3.56. Can you confirm?
When you don't use @primevue/nuxt-module, there is no such problem, right?

@mertsincan mertsincan self-assigned this Jun 25, 2024
@mertsincan mertsincan added Status: Pending Review Issue or pull request is being reviewed by Core Team and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Jun 25, 2024
@mertsincan mertsincan added this to the 4.0.0-rc.3 milestone Jun 25, 2024
@luke-z
Copy link
Author

luke-z commented Jun 25, 2024

Hello @mertsincan

I just created a barebone Nuxt 3.12.2 / PrimeVue 3.52.0 project with nuxt-primevue 3.0.0 with the same logic as my reproducer and can confirm that it doesn't happen there :)
Edit: I just tested it with 4.0.0-beta.5, which also leaks

I'll setup a Nuxt project without @primevue/nuxt-module quickly and report back.
However it seems like it's a core issue, so my prediction is that it will happen with this version as well.

@luke-z
Copy link
Author

luke-z commented Jun 25, 2024

@mertsincan I was able to replicate it by just using PrimeVue 4 without the nuxt module.
However, the impact is smaller and it escalates less.

https://github.com/luke-z/primevue-4-memory-leak/tree/no-nuxt-module

Edit: disabling ssr resolves the memory leak

Edit 2: Removing the theme key from the Plugin registration makes the memory leak more intense for the non nuxt module case:

vueApp.use(PrimeVue, {
-    theme: {
-      preset: Aura,
-    },
});

Edit 3: Registering the nuxt plugin as client only (primevue.client.ts) also fixes the problem.

@RoyeL12
Copy link

RoyeL12 commented Jun 25, 2024

Yes it seems that primevue is causing memory leak due to some interaction with ssr. I haven't been able to find the exact source yet, but 3.52 primevue is working fine

@mertsincan
Copy link
Member

mertsincan commented Jun 26, 2024

Thanks so much for your help, @luke-z and @RoyeL12! Can you try your cases after making the following changes?

node_modules/@primevue/core/config/index.mjs

// line 203
ThemeService.on('theme:change', function (newTheme) {
+    if (!isThemeChanged.value) {
+        app.config.globalProperties.$primevue.config.theme = newTheme;
+        isThemeChanged.value = true;
+   }
});

@luke-z
Copy link
Author

luke-z commented Jun 26, 2024

It seems to lower the curve, however it does not completely remove it.

With my ab command above I got around 120MB per dev console refresh. After applying your change it went to to around 60-80MB per refresh

image

However, both cases shoot up to 1-2GB of instance size quite fast.

Edit: I just noticed removing reactive from this line:

config: reactive(options)

And commenting out the entire ThemeService listener helped to keep the memory usage stable.
Not sure what implications the removal of reactive might have, but maybe it's triggering a lot of watchers?

@mertsincan
Copy link
Member

Thanks @luke-z ;) I think this line in TheemService is causing all these problems;

app.config.globalProperties.$primevue.config.theme = newTheme;

As you said, this may be calling more than one watcher. I keep digging :)

@luke-z
Copy link
Author

luke-z commented Jun 27, 2024

I figured out something more.

I went into the node_modules/@primeuix/styled/index.mjs and added the following lines in createService()

setInterval(function () {
    allHandlers.clear();
  }, 10000)

right below the initialization of allHandlers.
This, after waiting for a second for the garbage collector to kick in and sometimes having to manually start the garbage collection, cleared up the memory as it should.
This was in combination with the removed reactive as mentioned above :)

So on one hand, the reactive thing is messing with the memory and on the other hand the EventBus is not cleaning up properly. Don't know if a weakmap would help there?

Adding a simple console.log('hello') to the ThemeService.on('theme:change') also shows, that with each page refresh, a new "console log" is added. So on first page load it will log "hello", then 2x, then 3x and so on

@mertsincan
Copy link
Member

mertsincan commented Jun 27, 2024

Yes, you're right! I made some changes eventbus and config files. Could you please try;

node_modules/@primeuix/styled/index.mjs

// line 329
+    },
+    clear() {
+        allHandlers.clear();
+    }

node_modules/@primevue/core/config/index.mjs

// line 168
function setup(app, options) {
  var PrimeVue = {
    config: reactive(options)
  };
  app.config.globalProperties.$primevue = PrimeVue;
  app.provide(PrimeVueSymbol, PrimeVue);
+  clearConfig();
  setupConfig(app, PrimeVue);
  return PrimeVue;
}
+ var stopWatchers = [];
+ function clearConfig() {
+   ThemeService.clear();
+   stopWatchers.forEach(function (fn) {
+     return fn === null || fn === void 0 ? void 0 : fn();
+   });
+  stopWatchers = [];
+ }
function setupConfig(app, PrimeVue) {
  var isThemeChanged = ref(false);
// line 212
ThemeService.on('theme:change', function (newTheme) {
+    if (!isThemeChanged.value) {
      app.config.globalProperties.$primevue.config.theme = newTheme;
      isThemeChanged.value = true;
+    }
  });

  /*** Watchers ***/
+ var stopConfigWatcher = watch(PrimeVue.config, function (newValue, oldValue) {...);
+ var stopRippleWatcher = watch(function () {...);
+ var stopThemeWatcher = watch(function () {...);
+ var stopUnstyledWatcher = watch(function () {...);

+ stopWatchers.push(stopConfigWatcher);
+ stopWatchers.push(stopRippleWatcher);
+ stopWatchers.push(stopThemeWatcher);
+ stopWatchers.push(stopUnstyledWatcher);

@mertsincan mertsincan added the Package: Core name: @primevue/core label Jun 27, 2024
@luke-z
Copy link
Author

luke-z commented Jun 27, 2024

I just applied these changes to the reproducer I attached and can confirm that the memory leak seems to be resolved!

When spamming the application with the ab command, the instance size jumps around between 50-90MB and when stopping the process it falls back to the base size!

@mertsincan
Copy link
Member

Great!! Thank you very much for your help! The details really helped me in solving the problem.

@mertsincan mertsincan added Type: Bug Issue contains a bug related to a specific component. Something about the component is not working and removed Status: Pending Review Issue or pull request is being reviewed by Core Team labels Jun 27, 2024
@luke-z
Copy link
Author

luke-z commented Jun 27, 2024

Thanks for your effort!

@fabioselau077
Copy link

This problem still occurs even in the latest version

@prempopatia
Copy link

I am also facing the same issue in primevue without SSR.
On every refresh memory is incresing only and after considerable amount of time, it is even htiing 1GB.
I tried with primevue version 4.0.7 as well but no luck.

Can anyone help me?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: Core name: @primevue/core Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
Development

No branches or pull requests

5 participants