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

Nuxt 3: SSR memory leak in useHead() with computed getter on the server #15093

Closed
mrauhu opened this issue Oct 5, 2022 · 8 comments
Closed

Comments

@mrauhu
Copy link
Contributor

mrauhu commented Oct 5, 2022

Environment

  • Operating System: Linux
  • Node Version: v16.15.0
  • Nuxt Version: 3.0.0-rc.11
  • Nitro Version: 0.5.4
  • Package Manager: npm@8.5.5
  • Builder: vite
  • User Config: -
  • Runtime Modules: -
  • Build Modules: -

Reproduction

<script setup lang="ts">
import { useHead } from '#head';

// ...

useHead(() => getMeta(meta.value));
</script>

Describe the bug

Using a computed getter (() => ...)) as an argument of the getHead() results to memory leak on the server.

See also: #15095.

Temporary fix

<script setup lang="ts">
import { useHead } from '#head';

// ...

if (process.server) {
  useHead(getMeta(meta.value));
} else {
  useHead(() => getMeta(meta.value));
}
</script>

Additional context

Memory consumption

Production build of an application, 60 requests per second, 10 minutes of load (36 000 requests).

Before fix

image

After fix

image

Memory difference

Flame graph

image

Table

image

Logs

No response

@harlan-zw
Copy link
Contributor

harlan-zw commented Oct 5, 2022

Hey @mrauhu, thanks for the detailed audit.

Worth noting that the version of @vueuse/head in Nuxt RC 11 (0.7.12) does not officially support computed getter syntax.

It was introduced in https://github.com/vueuse/head/releases/tag/v0.8.0.

As there's no reproduction, would it be possible for you to test out https://github.com/harlan-zw/nuxt-hedge and see if it has the same issue? This uses the latest @vueuse/head, which I'm working to get into the core (nuxt/framework#8000).

Also, could you share your getMeta function so we can rule out it being a ref loop

@mrauhu
Copy link
Contributor Author

mrauhu commented Oct 5, 2022

Hi @harlan-zw.

As there's no reproduction, would it be possible for you to test out https://github.com/harlan-zw/nuxt-hedge and see if it has the same issue?

Tested nuxt-hedge with latest @vueuse/head.

Also, could you share your getMeta function so we can rule out it being a ref loop

Basically it's just converter from a flat object of metatags to MetaObject. There is no references inside, just one array:

// Simplified version
export function getMeta(meta: Record<string, string>) {
  const result = {
    meta: [],
  } as MetaObject;
  
  for (const [key, value] of Object.entries(meta)) {
    result.meta.push({
        name: key,
        key: key,
        content: value,
    });
  }
  
  return result;
}

Computed getter in useHead() from nuxt-hedge

Production build of an application, 60 requests per second, 10 minutes of load (36 000 requests).

Memory consumption

image

Memory difference

Computed getter in original useHead() versus nuxt-hedge

Same code, different libraries for useHead():

  • no fix, @vueuse/head@0.7.12
  • no fix, nuxt-hedge@0.2.3 and @vueuse/head@0.9.6.
Flamegraph

image_2022-10-05_19-53-11

Table

image_2022-10-05_19-53-54

Client-only computed getter in original useHead() versus nuxt-hedge

Different code, different libraries for useHead():

  • temporary fix, @vueuse/head@0.7.12;
  • no fix, nuxt-hedge@0.2.3 and @vueuse/head@0.9.6.
Flamegraph

image_2022-10-05_19-39-23

Table

image_2022-10-05_19-39-44

Total system memory usage

Production build of an application, 100 requests per second, 10 minutes of load (60 000 requests).

5.02 GB, client-only computed getter in original useHead()

image

6.23 GB, computed getter in useHead() from nuxt-hedge

image

Difference: + 1.23 GB

Conclusion

So using the nuxt-hedge is better than no fix, but the temporary fix is the best:

if (process.server) {
  useHead(getMeta(meta.value));
} else {
  useHead(() => getMeta(meta.value));
}

@harlan-zw
Copy link
Contributor

Thank you @mrauhu

I have some ideas on how to improve this, but it includes some risky changes around reactivity so it may not be available until @vueuse/head v1.

Will give you a ping when there's something to test

@antlionguard
Copy link
Contributor

antlionguard commented Oct 8, 2022

I have a question off topic. What is the name of the program/application you use to track of memory?

@mrauhu
Copy link
Contributor Author

mrauhu commented Oct 9, 2022

I have a question off topic. What is the name of the program/application you use to track of memory?

@antlionguard Datadog APM with Continuous profiler.

@harlan-zw
Copy link
Contributor

Hey @mrauhu

I have worked on the performance improvements, they are available in nuxt-hedge 0.5.0.

The client-side performance should be improved, the server performance should be a bit more stable but hard to say if it's solved the memory leak issue as I don't know the full scope of how the ssrContext is used.

@harlan-zw
Copy link
Contributor

Potential fix is also available in npm:nuxt3@latest

@danielroe
Copy link
Member

The issue here was not actually (IIRC) the use of useHead within the component but rather that Nuxt itself was calling use head in a plugin meaning that computed was being called outside of component setup.

I believe this is resolved as computed is no longer being called by @vueuse/head, and functional/computed types are excluded from app.head in nuxt.config.

Let me know if not and I'll happily reopen.

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

4 participants