callstack / haul

Haul is a command line tool for developing React Native apps, powered by Webpack
MIT License
3.64k stars 194 forks source link

CircleCI Crashes on Ubuntu instances due to Memory Usage #529

Closed hjhart closed 5 years ago

hjhart commented 5 years ago

Ask your Question

The CircleCI boxes that are used for ubuntu testing (specifically our android build) have 36 cores. When we are running our yarn bundle --platform=android build, we are running out of memory with this error:

./node_modules/react-native/Libraries/Utilities/stringifySafe.js
Module build failed (from ./node_modules/haul/node_modules/thread-loader/dist/cjs.js):
Thread Loader (Worker 32)
ENOMEM: not enough memory, read

We don't see this problem on our other workstations, that have 8 cores.

When we modified the thread-loader options to only have 1 worker, we no longer see this issue.

How do you recommend that we modify the configuration to have less thread-workers? What strategies do people normally use to merge in webpack config settings? webpack-merge?

Currently we are doing this:

export default {
  webpack: env => {
    const config = createWebpackConfig({
      entry: `./index.js`
    })(env)
    config.module.rules[1].use[1].options.workers = 1
    // A bunch of other rules and settings
  }
}

But this is obviously sub-optimal.

Thanks for looking and helping!

James

hjhart commented 5 years ago

Another solution that is still not ideal is this one:

   const config = createWebpackConfig({
      entry: `./index.js`
    })(env)

    config.module.rules = config.module.rules.map(rule => {
      if (Array.isArray(rule.use)) {
        rule.use.map(usage => {
          if (RegExp('thread-loader').test(usage.loader) ) {
            usage.options.workers = Math.min(7, usage.options.workers)
          }
          return usage
        })
      }
      return rule
    })

I'm thinking a better solution would be to just cap the number of workers to 15 (16 cores - 1)? I'm not sure what the tipping point is for number of spawned threads and out of memory errors, but we have only seen issues on these beefy 36 core machines. Otherwise we are using 8 cores + hyperthreading on our MacBook Pros. So maybe 7 is the right threshold.

Looking for insight! Thanks.

tido64 commented 5 years ago

I seem to recall reading something about Webpack having memory leaks. Which version are you using? Could you try upgrading and see if that helps?

hjhart commented 5 years ago

Hi @tido64, thanks for your response!

We are using webpack 4.29.5, so just a patch version behind. Seems unlikely to be a version issue.

tido64 commented 5 years ago

I think I found the thread where we were discussing the issue at work. Seems upgrading to latest Node (11.13.0), or latest LTS version (10.15.3), solved that particular case. Something worth looking into?

hjhart commented 5 years ago

@tido64 Thanks again. We were already using node LTS 10.15.3.

When you spin up 35 threads each that consume ~500mb of memory, you're bound to hit some limits, as that's nearly 18GB of memory.

Can we think of adding a maximum thread count to the thread-loader, or perhaps make it easily configurable?

tido64 commented 5 years ago

Ah, I completely missed the number of cores. Sorry about that. The thread count is set in src/utils/makeReactNativeConfig.js:

{
  loader: require.resolve('thread-loader'),
  options: {
    workers: Math.max(os.cpus().length - 1, 1),
  },
},

It looks like we're counting logical cores and subtract one. Should be simple to make configurable. What do you think, @thymikee? @hjhart, would you like to give it a try?

hjhart commented 5 years ago

Yeah, I can submit a pull request. Much friendlier than manipulating nested JavaScript objects...

On Apr 6, 2019, at 16:28, Tommy Nguyen notifications@github.com wrote:

Ah, I completely missed the number of cores. Sorry about that. The thread count is set in src/utils/makeReactNativeConfig.js:

{ loader: require.resolve('thread-loader'), options: { workers: Math.max(os.cpus().length - 1, 1), }, }, It looks like we're counting logical cores and subtract one. Should be simple to make configurable. What do you think, @thymikee? @hjhart, would you like to give it a try?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

zamotany commented 5 years ago

How about we just remove thread-loader? There's almost not gain, from my measurements on 4 cores/8 threads machine it's around ~10% difference only. IMO it should be a opt-in instead of opt-out (current implementation).

zamotany commented 5 years ago

For anyone having this problem: the issue has been fixed on a next branch, when we do a Haul revamp. If you're running RN 0.59 or 0.60, please check it out - it's much more stable.

zamotany commented 5 years ago

If anyone is having this issue please use @haul-bundler/cli (if not already using) - where this issue is no longer occurring - and feel free to open a new issue or reopen this one.