clinicjs / node-clinic

Clinic.js diagnoses your Node.js performance issues
https://clinicjs.org
MIT License
5.67k stars 125 forks source link

Heap Profiler not building the same Flamegraph as in the Docs example #465

Open alfonsograziano opened 9 months ago

alfonsograziano commented 9 months ago

I was looking at the Heap profiler. I created a Memory Leak on a Next.js application with this code:

import styles from "./page.module.css";
import { headers } from "next/headers";

const array = []

const createMemory = () => {
  for(let i = 0; i < 1000; i++){
    array.push(i)
  }
}

const createMoreMemory = () => {
  for(let i = 0; i < 5000; i++){
    array.push(i)
  }
}

export default async function Home() {
  const headersList = headers();
  createMemory()
  createMoreMemory()
  return (
    <main className={styles.main}>{JSON.stringify(headersList, null, 2)}</main>
  );
}

I disabled optimizations and minification as well in the Next.js config like this:

module.exports = {
    productionBrowserSourceMaps: true,
    webpack: (config, { buildId, dev, isServer, defaultLoaders, webpack }) => {
      // Disable minification in production
        config.optimization.minimize = false;
        return config;
    },
  };

This is the output of the heapprofiler:

Screenshot 2024-01-04 at 10 36 20

I was expecting to see the memory footprint of the createMemory and createMoreMemory functions. I can only see the Home function called in the flame graph.

This made me think that maybe, for some reason, the heap profiler was not working as intended on Next.js. Before starting an investigation, I decided to test with the example provided in the Flame First Analysis documentation.

I just cloned the repo and I run the script as shown. In this case the expected output is the one shown in the documentation:

Screenshot 2024-01-04 at 10 41 06

The actual result on my machine is this one:

Screenshot 2024-01-04 at 10 41 36

The "soManyAllocation" reference is missing.

This is my config:

In both cases, since the functions that are using a lot of memory are missing, the heap profiler is not really useful. I'll try to play with it a little bit and reproduce exactly the example.

I also noticed that in the screenshot, the image reports the script called 1-server-with-leaks, but in the example repo, this script is not included. I'm not sure if is a different script or has just been renamed

RafaelGSS commented 9 months ago

That's due to recent Node.js versions.

simoneb commented 9 months ago

Following up to a conversation we were having with Alfonso, I'm wondering if there's an opportunity here to revise the approach used in Clinic which relies considerably on Node internals, and rather switch to use an approach that's more robust across Node.js versions, perhaps relying on published APIs. I'm not even sure if this is feasible, but I wonder for example what other profilers do, e.g. those built into Chrome.