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

Memory leak regression after upgrading to >= 6.3.0 #1848

Closed
johanrd opened this issue Mar 8, 2024 · 3 comments · Fixed by #1852
Closed

Memory leak regression after upgrading to >= 6.3.0 #1848

johanrd opened this issue Mar 8, 2024 · 3 comments · Fixed by #1852
Labels
bug Something isn't working

Comments

@johanrd
Copy link
Contributor

johanrd commented Mar 8, 2024

Hi. I have experienced memory leaks in my app after upgrading ember-intl from v6.2.2 to >v6.3.0.

Steps to reproduce:

  1. Clone https://github.com/johanrd/ember-memory-leak
  2. Check out the ember-intl branch
  3. Run pnpm start
  4. Transition between about and dashboard a few times, and see that e.g. the ChartJs Component piles up in memory each time you enter the dashboard route:
memory-leak-ember-intl.mov
  1. Downgrade to ember-intl v6.2.2 and see that the memory leaks with the ChartJs Component are gone:
no-memory-leak-ember-intl.mov
@ijlee2
Copy link
Contributor

ijlee2 commented Mar 8, 2024

Hi, @johanrd. Thanks for posting recordings to show some of the steps that you had taken. From the videos, though, I didn't understand what I'm supposed to look at, which suggests that there is a memory leak. Can you describe this to me?

Secondly, I found the commit history hard to follow. Can you recreate the history so that each commit (each step) clearly indicates what's particular about that step?

At the moment, I have to doubt that ember-intl is the issue, because your reproduction repo includes a few other variables, such as ember-truth-helpers (in whose repo you had raised a similar issue), ember-resources, ember-template-imports (for *.gts), ember-route-template, and chart.js.

/* package.json */
{
  "dependencies": {
    "@ember/render-modifiers": "^2.1.0",
    "chart.js": "^4.4.2",
    "ember-intl": "^6.5.1",
    "ember-resources": "^7.0.0",
    "ember-route-template": "^1.0.3",
    "ember-truth-helpers": "^4.0.3",
    "reactiveweb": "^1.2.2"
  }
}

If you can write a reproduction app with the minimum necessary dependencies (don't include addons that are relatively unstable), I think it will help with your investigation more.

@ijlee2 ijlee2 added the status: needs information Issue needs more context to help everyone understand the problem label Mar 8, 2024
@johanrd
Copy link
Contributor Author

johanrd commented Mar 8, 2024

@ijlee2 Hi. Thanks for the quick reply. Yes, the commit history is a mess – memory leaks are a mess.

Don't follow the commit history. Please follow my steps to reproduce

  1. Clone https://github.com/johanrd/ember-memory-leak
  2. Check out the ember-intl branch
  3. Run pnpm start
  4. Click on the "about' link.
  5. Take a heap snapshot.
  6. Search for "ChartJs". See that there are no such component in memory.
  7. Visit the "dashboard" page and go back to "about" N times.
  8. Take a new snapshot. See that the ChartJS component is retained in memory for each visit to dashboard, i.e. N times
Screenshot_2024-03-08_at_13_31_47
  1. Downgrade to ember-intl v6.2.2.
  2. Repeat steps 1-7 and see that ChartJS component is successfully released from memory every time you exit the dashboard route.

Here, the only difference is the downgrade of ember-intl from v.6.3.0 to v6.2.2. One has a memory leak. The other does not. I can try to create an even more miniomal steps to reproduce (hence, the messy commit history), but at least there are steps to reproduce.

@johanrd
Copy link
Contributor Author

johanrd commented Mar 8, 2024

Update: I have now stripped it down to a minimal reproducible example here: https://github.com/johanrd/ember-intl-memoy-leak-reproduction
Screenshot 2024-03-08 at 14 02 03

Just search for LeakyIntlConsumingComponent in the heap memory, and it should be evident that the component (which is only consuming the t-helper) is not released from memory when it is destroyed.

Downgrade to ember-intl v6.2.2 and the memory leak is gone

johanrd added a commit to johanrd/ember-intl that referenced this issue Mar 10, 2024
johanrd added a commit to johanrd/ember-intl that referenced this issue Mar 10, 2024
… I can verify that the leak is gone, but not sure if this is the preferred way
@ijlee2 ijlee2 added bug Something isn't working and removed status: needs information Issue needs more context to help everyone understand the problem labels Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants