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

Option to limit number of processes/threads/fds #424

Closed
vicary opened this issue May 26, 2020 · 17 comments
Closed

Option to limit number of processes/threads/fds #424

vicary opened this issue May 26, 2020 · 17 comments

Comments

@vicary
Copy link

vicary commented May 26, 2020

Feature motivation

My team is using serverless-webpack to bundle ~100-ish Lambda functions at the same time, due to constraints in serverless-appsync-plugin we cannot split them into multiple batches, and with webpack + ts-loader alone it is impossible to bundle them all at once.

This module opens up the possibility to keep the type checking, but we quickly hit another problem. i.e. With 100 forked processes it drained the fd limit in no time.

While it is possible to raise such limit to millions for one machine, the development cycle becomes exponentially complicated when it comes to CI/CD in containers.

Feature description

In webpack/webpack@9cb4225 webpack itself limits how many files it opens each time, but fork-ts-checker-webpack-plugin multiplies the limit of 15 by the total number of functions to be bundled.

It would be great if there is an option, or at least a sane default, to limit the total number of threads/forked processes.

Feature implementation

Maintain the number of active forks, delaying further forking until earlier child processes is finished, packages like p-limit is worth considering.

@piotr-oles
Copy link
Collaborator

Hi @vicary

I think it's not a common issue so I wouldn't like to implement this logic in the plugin. Instead, I could modify the start hook on the alpha branch to provide AsyncSeriesWaterfallHook. Then you could tap into this hook and add awaiting logic that suits your case :)

Is it something that would help you?

@piotr-oles
Copy link
Collaborator

piotr-oles commented May 26, 2020

This is a pseudo-code that I came up:

import ForkTsCheckerWebpackPlugin from 'fork-ts-checker-webpack-plugin';
import { Limit } from 'p-limit';

interface LimitForkTsCheckerWorkersWebpackPluginOptions {
  limit: Limit;
}

class LimitForkTsCheckerWorkersWebpackPlugin implements webpack.Plugin {
  constructor(private readonly options: LimitForkTsCheckerWorkersWebpackPluginOptions) {}

  apply(compiler: webpack.Compiler) {
    const hooks = ForkTsCheckerWebpackPlugin.getCompilerHooks(compiler);

    // intercept service starting hook
    hooks.start.tapPromise('LimitForkTsCheckerWorkersWebpackPluginOptions', this.options.limit((changes) => changes));
  }
}

export = LimitForkTsCheckerWorkersWebpackPlugin;

and usage:

// shared/limit.js
const pLimit = require('p-limit');
const limit = pLimit(10);

module.exports = limit;
// webpack.config.js
const limit = require('shared/limit');
const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
const LimitForkTsCheckerWorkersWebpackPlugin = require('shared/plugin/LimitForkTsCheckerWorkersWebpackPlugin');

module.exports = {
   // ...
  plugins: [
    new ForkTsCheckerWebpackPlugin(),
    new LimitForkTsCheckerWorkersWebpackPlugin({ limit })
  ]
  // ...
};

@vicary
Copy link
Author

vicary commented May 26, 2020

Hi @piotr-oles,

Thanks for the quick reply.

Haven't worked with plugin hooks yet, do you mean I can control each processes with something like the following?

const forkTsCheckerWebpackPlugin = new ForkTsCheckerWebpackPlugin();
let activeCount = 0;
const resolveFn = [];

forkTsCheckerWebpackPlugin.hooks.serviceBeforeStart.tapPromise("throttlePlugin", new Promise((resolve, reject) => {
  if (activeCount < 10) {
    resolve();
    activeCount++;
  } else {
    resolveFns.push(resolve);
  }
}));

forkTsCheckerWebpackPlugin.hooks.done.tap("throttlePlugin", () => {
  const resolve = resolveFns.shift();
  if (resolve) {
    resolve();
  } else {
    activeCount--;
  }
});

This looks like the current AsyncSeriesHook in serviceBeforeStart and SyncHook in done already works.

Am I getting it correctly?

@vicary
Copy link
Author

vicary commented May 26, 2020

Oh wow thanks for the sample code! So I have to make my own plugin to tap into those hooks, right?

@piotr-oles
Copy link
Collaborator

I think it's the most convenient way to do this :) You can also manually get the compiler instance but this means that you would have to use webpack's Node.js API (because you don't have a compiler instance when you just export the configuration).

The pseudo-code I sent you is for the alpha branch - https://github.com/TypeStrong/fork-ts-checker-webpack-plugin/tree/alpha :) I don't want to add new features to the current stable version. It will be maintained only for critical bug fixes, new features will be merged to the alpha branch which is a result of a complete rewrite of the plugin #404 :)

@vicary
Copy link
Author

vicary commented May 26, 2020

Sounds great!

Just wanna make sure no unnecessary works on your side, does the hook serviceBeforeStart do the job?

@piotr-oles
Copy link
Collaborator

Yes, it should work :)

piotr-oles added a commit that referenced this issue May 26, 2020
Adds possibility to delay the start of the plugin by tapping into the
start hook.

BREAKING CHANGE: 🧨 start hook changed from SyncWaterfallHook to AsyncSeriesWaterfallHook

✅ Closes: #424
@piotr-oles
Copy link
Collaborator

Please let me know when you implement it if it worked 😃

piotr-oles added a commit that referenced this issue May 26, 2020
Adds possibility to delay the start of the plugin by tapping into the
start hook.

BREAKING CHANGE: 🧨 start hook changed from SyncWaterfallHook to AsyncSeriesWaterfallHook

✅ Closes: #424
@piotr-oles
Copy link
Collaborator

🎉 This issue has been resolved in version 5.0.0-alpha.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

@vicary
Copy link
Author

vicary commented May 27, 2020

Yes, it worked to a certain degree, definitely prevented the EMFILE error. Now it only explodes at heap size, which is solved in this PR serverless-heaven/serverless-webpack#517.

Really thanks for the help!

@vicary vicary closed this as completed May 27, 2020
@vicary
Copy link
Author

vicary commented May 27, 2020

Shamelessly edited your sample script into a tiny npm package 😄

@piotr-oles
Copy link
Collaborator

Nice! :)

@piotr-oles
Copy link
Collaborator

🎉 This issue has been resolved in version 5.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@piotr-oles
Copy link
Collaborator

🎉 This issue has been resolved in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@fasiha
Copy link

fasiha commented Mar 21, 2023

Can @vicary's plugin (to configure limiter) be backported to fork-ts-checker-webpack-plugin version 4.1.6?

@vicary
Copy link
Author

vicary commented Mar 22, 2023

@fasiha I can squeeze out some time after I finished my current documentation work in some other projects, possibly in the next few weeks. If you need it right now, I am also accepting PRs.

@vicary
Copy link
Author

vicary commented Apr 30, 2023

@fasiha Reading the source codes, my plugin should be compatible from v4 all the way up to v8. I expanded the peer version and published 1.2.0, give it a try and let me know if anything breaks.

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

No branches or pull requests

3 participants