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 in "partykit dev" (runBuild) #855

Open
kettanaito opened this issue Mar 29, 2024 · 5 comments
Open

Memory leak in "partykit dev" (runBuild) #855

kettanaito opened this issue Mar 29, 2024 · 5 comments

Comments

@kettanaito
Copy link

After some time working in the PartyKit + Remix template, esbuild starts spawning more things than necessary:

Screenshot 2024-03-29 at 14 37 07

The esbuild process also ends up taking an absurd amount of RAM (15GB and counting), which strongly suggests a memory leak.

Given this doesn't happen in plain Remix, I suspect this can be related to how PartyKit orchestrates server updates.

Reproduction steps:

git clone https://github.com/kettanaito/tug-o-war
cd tug-o-war
npm run dev

Go to any TypeScript file in app and change it multiple times. Watch the esbuild process accumulate unreleased memory.

@threepointone
Copy link
Contributor

Ugh good catch.

@kettanaito
Copy link
Author

Should be fairly easy to reproduce. I get the RAM bump after 3-5 edits.

@kettanaito
Copy link
Author

Okay, doing some profiling of this and I can see that the useEffect() that creates esbuild.context() is called numerous times upon incremental change:

# Initial run: partykit dev
useEffect: called!
runBuild: called!
runBuild: added partykit plugin!

# Incremental change:
useEffect: called!
runBuild: called!
useEffect: called!
runBuild: called!
useEffect: called!
runBuild: called!
runBuild: added partykit plugin!
Build succeeded, starting server...
runBuild: added partykit plugin!
Build succeeded, starting server...
useEffect: called!
runBuild: called!
useEffect: called!
runBuild: called!
useEffect: called!
runBuild: called!
useEffect: called!
runBuild: called!

useEffect: called!
runBuild: called!
runBuild: added partykit plugin!
Build succeeded, starting server...
runBuild: added partykit plugin!
Build succeeded, starting server...
runBuild: added partykit plugin!
Build succeeded, starting server...
runBuild: added partykit plugin!
Build succeeded, starting server...
runBuild: added partykit plugin!
Build succeeded, starting server...
runBuild: added partykit plugin!
Build succeeded, starting server...

I've added logs in here, here, and here.

The parent useDev() hook is also called multiple times upon incremental changes.

@threepointone, can you tell me more about why esbuild is used in a React's hook? That's quite unusual :D

I also suspect that keeping ctx as a variable scoped to useEffect is a bad idea. useEffect can be called multiple times without cleaning up (whenever its dependency change). But we do nothing right now to dispose of ctx when that happens.

@kettanaito
Copy link
Author

kettanaito commented Apr 2, 2024

I've trimmed down the initiator of useEffect to be the assetsMap variable. It contains all the assets emitted (by the build?) so, naturally, when I update a module, it may produce new assets or update exiting ones.

Here's an example of the assetsMap diff that triggered useEffect (i.e. runBuild):

{
  prevSource: {
    current: {
      devServer: 'http://127.0.0.1:49603',
      browserTTL: undefined,
      edgeTTL: undefined,
      singlePageApp: undefined,
      assets: [Object]
    }
  },
  source: {
    devServer: 'http://127.0.0.1:49603',
    browserTTL: undefined,
    edgeTTL: undefined,
    singlePageApp: undefined,
    assets: {
      'arena.png': 'arena.png',
      'background.png': 'background.png',
      'favicon.ico': 'favicon.ico',
      'manifest.json': 'manifest.json',
      'mockServiceWorker.js': 'mockServiceWorker.js',
      'sign.png': 'sign.png',
      'fonts/morris-roman.black.ttf': 'fonts/morris-roman.black.ttf',
      'dist/index.js': 'dist/index.js',
      'dist/index.js.map': 'dist/index.js.map',
+      'build/__remix_entry_dev-KOSYFGLN.js': 'build/__remix_entry_dev-KOSYFGLN.js',
+      'build/__remix_entry_dev-KOSYFGLN.js.map': 'build/__remix_entry_dev-KOSYFGLN.js.map',
      'build/css-bundle-URXWV324.css': 'build/css-bundle-URXWV324.css',
      'build/css-bundle-URXWV324.css.map': 'build/css-bundle-URXWV324.css.map',
      'build/entry.client-VNZNSQKD.js': 'build/entry.client-VNZNSQKD.js',
      'build/entry.client-VNZNSQKD.js.map': 'build/entry.client-VNZNSQKD.js.map',
      'build/manifest-5116B4E4.js': 'build/manifest-5116B4E4.js',
      'build/root-I4ZBLVPU.js': 'build/root-I4ZBLVPU.js',
      'build/root-I4ZBLVPU.js.map': 'build/root-I4ZBLVPU.js.map',
+      'build/root-RJOJ2B3P.css': 'build/root-RJOJ2B3P.css',
+      'build/root-RJOJ2B3P.css.map': 'build/root-RJOJ2B3P.css.map',
+      'build/routes/_index-DXQU5XVU.js': 'build/routes/_index-DXQU5XVU.js',
      'build/routes/_index-DXQU5XVU.js.map': 'build/routes/_index-DXQU5XVU.js.map',
      'build/routes/game.admin-3Y3BYEWW.js': 'build/routes/game.admin-3Y3BYEWW.js',
      'build/routes/game.admin-3Y3BYEWW.js.map': 'build/routes/game.admin-3Y3BYEWW.js.map',
      'build/_shared/browser-Z2C4YXXQ.js': 'build/_shared/browser-Z2C4YXXQ.js',
      'build/_shared/browser-Z2C4YXXQ.js.map': 'build/_shared/browser-Z2C4YXXQ.js.map',
+      'build/_shared/chunk-ACACETXL.js': 'build/_shared/chunk-ACACETXL.js',
+      'build/_shared/chunk-ACACETXL.js.map': 'build/_shared/chunk-ACACETXL.js.map',
+      'build/_shared/chunk-H36SQQE5.js': 'build/_shared/chunk-H36SQQE5.js',
+      'build/_shared/chunk-H36SQQE5.js.map': 'build/_shared/chunk-H36SQQE5.js.map',
+      'build/_shared/chunk-IHZXMV55.js': 'build/_shared/chunk-IHZXMV55.js',
+      'build/_shared/chunk-IHZXMV55.js.map': 'build/_shared/chunk-IHZXMV55.js.map',
+      'build/_shared/chunk-JKUASME7.js': 'build/_shared/chunk-JKUASME7.js',
+      'build/_shared/chunk-JKUASME7.js.map': 'build/_shared/chunk-JKUASME7.js.map',
      'build/_shared/chunk-JXHNNPNR.js': 'build/_shared/chunk-JXHNNPNR.js',
      'build/_shared/chunk-JXHNNPNR.js.map': 'build/_shared/chunk-JXHNNPNR.js.map',
+      'build/_shared/chunk-LCVWOUJY.js': 'build/_shared/chunk-LCVWOUJY.js',
+      'build/_shared/chunk-LCVWOUJY.js.map': 'build/_shared/chunk-LCVWOUJY.js.map',
+      'build/_shared/chunk-N4FG5RPV.js': 'build/_shared/chunk-N4FG5RPV.js',
+      'build/_shared/chunk-N4FG5RPV.js.map': 'build/_shared/chunk-N4FG5RPV.js.map',
+      'build/_shared/chunk-RODUX5XG.js': 'build/_shared/chunk-RODUX5XG.js',
+      'build/_shared/chunk-RODUX5XG.js.map': 'build/_shared/chunk-RODUX5XG.js.map',
+      'build/_shared/chunk-TVZC3ZTX.js': 'build/_shared/chunk-TVZC3ZTX.js',
+      'build/_shared/chunk-TVZC3ZTX.js.map': 'build/_shared/chunk-TVZC3ZTX.js.map',
      'build/_shared/client-5VJPMT66.js': 'build/_shared/client-5VJPMT66.js',
      'build/_shared/client-5VJPMT66.js.map': 'build/_shared/client-5VJPMT66.js.map',
      'build/_shared/esm-ST23SWPS.js': 'build/_shared/esm-ST23SWPS.js',
      'build/_shared/esm-ST23SWPS.js.map': 'build/_shared/esm-ST23SWPS.js.map',
      'build/_shared/jsx-dev-runtime-O7QZHBCI.js': 'build/_shared/jsx-dev-runtime-O7QZHBCI.js',
      'build/_shared/jsx-dev-runtime-O7QZHBCI.js.map': 'build/_shared/jsx-dev-runtime-O7QZHBCI.js.map',
      'build/_shared/jsx-runtime-4JVBCWRJ.js': 'build/_shared/jsx-runtime-4JVBCWRJ.js',
      'build/_shared/jsx-runtime-4JVBCWRJ.js.map': 'build/_shared/jsx-runtime-4JVBCWRJ.js.map',
      'build/_shared/react-44WC4HVE.js': 'build/_shared/react-44WC4HVE.js',
      'build/_shared/react-44WC4HVE.js.map': 'build/_shared/react-44WC4HVE.js.map',
      'build/_shared/react-dom-HTVZIA6L.js': 'build/_shared/react-dom-HTVZIA6L.js',
+      'build/_shared/react-dom-HTVZIA6L.js.map': 'build/_shared/react-dom-HTVZIA6L.js.map',
+      'build/_shared/remix_hmr-ULZE7MV6.js': 'build/_shared/remix_hmr-ULZE7MV6.js',
+      'build/_shared/remix_hmr-ULZE7MV6.js.map': 'build/_shared/remix_hmr-ULZE7MV6.js.map',
+      'build/_shared/runtime-GJVYN4KP.js': 'build/_shared/runtime-GJVYN4KP.js',
+      'build/_shared/runtime-GJVYN4KP.js.map': 'build/_shared/runtime-GJVYN4KP.js.map'
    }
  }
}

What's odd, is that all I did in this scenario is hit CMD+S without making any actual changes to the code. The fact that it produces new modules and sourcemaps is... odd.

@kettanaito kettanaito changed the title Memory leak in esbuild Memory leak in "partykit dev" (runBuild) Apr 2, 2024
@kettanaito
Copy link
Author

I'm convinced the fix would be not to create a scoped ctx but instead keep a hook-level useRef for the context where any current build is stored. When a new build starts (useEffect triggers), the last reference to the build must be disposed of before esbuild.context() is ever called. I'm trying this now to little success but I'm editing the 100k LOC node module so I'm a bit slow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants