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 allocation issue #2340

Open
oliviertassinari opened this issue Jul 21, 2023 · 12 comments · Fixed by #2381
Open

Memory allocation issue #2340

oliviertassinari opened this issue Jul 21, 2023 · 12 comments · Fixed by #2381
Labels
docs Improvements or additions to the documentation performance scope: toolpad-studio Abbreviated to "studio"

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 21, 2023

Steps to reproduce 🕹

Steps:

  1. Deploy a basic toolpad app in an environment with 512 MB of memory
  2. Limit the max old space of Node.js to 396 MB NODE_OPTIONS='--max-old-space-size=396'

Current behavior 😯

Crash memory allocation issue

Expected behavior 🤔

Works

Context 🔦

Trying to deploy the example of Toolpad in render.com

Your environment 🌎

No response

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work status: waiting for maintainer These issues haven't been looked at yet by a maintainer waiting for 👍 Waiting for upvotes labels Jul 21, 2023
@apedroferreira
Copy link
Member

So I'm guessing something like --optimize_for_size --max_old_space_size=460 --gc_interval=100 won't help?

@Janpot
Copy link
Member

Janpot commented Jul 21, 2023

Running some tests in docker

  1. docker run --rm -it -m 512m -w /app -v $PWD/examples/qr-generator:/app node sh
  2. yarn
  3. yarn build
  4. this fails due to memory issues
  5. NODE_OPTIONS='--max-old-space-size=396' yarn build
  6. fails due to memory issues as well
  7. NODE_OPTIONS='--max-old-space-size=450' yarn build
  8. this passes and builds the artifact

Let's try again without limits:

  1. docker run --rm -it -w /app -v $PWD/examples/qr-generator:/app node sh
  2. yarn build
  3. this passes and builds the artifact

Let's try to run that build

  1. docker run --rm -it -m 512m -w /app -v $PWD/examples/qr-generator:/app node sh
  2. yarn start
  3. this runs comfortably at around 311M memory usage

Memory issues seem to only happen at build, which is sort of expected to take quite some memory. It seems possible to build it in a container of 512m, if you tune it well. 512m is really pushing it though.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jul 21, 2023

In render.com, I believe fails at the yarn start step. I haven't seen it fail during yarn build. Could it be prisma/prisma#4972?

@oliviertassinari oliviertassinari changed the title Fix memory allocation issue Memory allocation issue Jul 21, 2023
@Janpot
Copy link
Member

Janpot commented Jul 22, 2023

Could it be prisma/prisma#4972?

Perhaps, if the end-user application is using prisma. Toolpad itself doesn't use prisma.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jul 23, 2023

I have narrowed down a bit more the problem:

@oliviertassinari oliviertassinari added performance and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 23, 2023
@Janpot
Copy link
Member

Janpot commented Jul 23, 2023

In https://github.com/oliviertassinari/mui-olivier-tools/pull/1/files, did this start happening between 0.1.19 -> 0.1.20 or between 0.1.13 -> 0.1.20?

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jul 23, 2023

did this start happening between 0.1.19 -> 0.1.20 or between 0.1.13 -> 0.1.20?

The issue seems to have been introduced in v0.1.18, per oliviertassinari/mui-olivier-tools@a69b619.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Aug 4, 2023

Great to see improvements in memory consumption. I had a look at the min memory requirement for more complex apps as I was curious:

But we can't compare Toolpad to these products so much, a bit of a different category.

I think https://mui.com/toolpad/tutorials/render-deploy/#creating-a-new-app-on-render could be set more clearly, we don't precise the minimum hardware requirements.


I tried to upgrade in oliviertassinari/mui-olivier-tools#3 to v0.1.22. It failed. In the last commit, I created a dummy express server to better isolate the problem.

It might be an issue with the initial memory allocated during the start of toolpad start?

@Janpot
Copy link
Member

Janpot commented Aug 4, 2023

It might be an issue with the initial memory allocated during the start of toolpad start?

Could be, I guess there'll be a memory spike on most apps during initialization. I would expect render.com to have a low pass filter in place though to ignore that.

Could it be toolpad build that's failing? One area where memory consumption could have increased is during toolpad build. We've moved some work that was continuously running to a build step that runs just once and stores the result. I think I can optimize toolpad build some more, I haven't payed much attention to it yet.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Aug 4, 2023

Could it be toolpad build that's failing?

I don't think so, I did the 3rd commit https://github.com/oliviertassinari/mui-olivier-tools/pull/3/commits precisely to rule out this possibility. It deploys correctly https://mui-olivier-tools-pr-3.onrender.com/

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Aug 5, 2023

I'm reopening, as in the worse case, I think we should document the minimum runtime requirement like https://docs.strapi.io/dev-docs/deployment#hardware-and-software-requirements, this also connects to #2432, so that developers have a higher chance to succeed in their deployment.

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Sep 26, 2023
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Sep 26, 2023

#2689 solved a significant chunk of the issue. Memory has been down significantly since v0.1.29:

Screenshot 2023-09-26 at 15 08 09

I could upgrade oliviertassinari/mui-olivier-tools#5 🎉


To close this issue, I think it's mostly a matter of:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation performance scope: toolpad-studio Abbreviated to "studio"
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants