-
-
Notifications
You must be signed in to change notification settings - Fork 243
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
Comments
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 Is it something that would help you? |
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 })
]
// ...
}; |
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 Am I getting it correctly? |
Oh wow thanks for the sample code! So I have to make my own plugin to tap into those hooks, right? |
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 :) |
Sounds great! Just wanna make sure no unnecessary works on your side, does the hook |
Yes, it should work :) |
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
Please let me know when you implement it if it worked 😃 |
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
🎉 This issue has been resolved in version 5.0.0-alpha.9 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Yes, it worked to a certain degree, definitely prevented the Really thanks for the help! |
Shamelessly edited your sample script into a tiny npm package 😄 |
Nice! :) |
🎉 This issue has been resolved in version 5.0.0-beta.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This issue has been resolved in version 5.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Can @vicary's plugin (to configure |
@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. |
@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. |
Feature motivation
My team is using
serverless-webpack
to bundle ~100-ish Lambda functions at the same time, due to constraints inserverless-appsync-plugin
we cannot split them into multiple batches, and withwebpack
+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.The text was updated successfully, but these errors were encountered: