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

Consider using worker threads instead of using spawn. #61

Open
macisi opened this issue May 14, 2019 · 11 comments
Open

Consider using worker threads instead of using spawn. #61

macisi opened this issue May 14, 2019 · 11 comments

Comments

@macisi
Copy link

macisi commented May 14, 2019

It's the new feature in node 12.x. Maybe it works in older version with some running arguments.

@ianwalter
Copy link

I think worker threads were added in 11.7 actually, but maybe this can take advantage of https://github.com/josdejong/workerpool which has logic to automatically determine if worker threads are supported and use them for a worker pool or otherwise use child_process.

@kwonoj
Copy link

kwonoj commented Nov 30, 2019

after I created #81 and realized there are some breaking changes when use worker_threads, pushed bit more PR to spin up working module https://www.npmjs.com/package/webpack-loader-worker . Would be keen on upstream these changes eventually if there are interests, meanwhile publishing as user land module so anyone can give it a try.

@jsg2021
Copy link

jsg2021 commented Nov 8, 2020

I wonder if this pool implementation would help?

https://github.com/SUCHMOKUO/node-worker-threads-pool

@malash
Copy link

malash commented Dec 15, 2021

I forked the latest version of thread-loader and refactored it with worker_threads.

See malash/thread-loader-issue-61#1

It could 100% work in my Webpack projects, which means no error is emitted when bundling and the output is correct.

For better performance I didn't introduce any thread pool library, but only modified the src/WorkerPool.js with native worker_threads. I also removed all serialize/deserialize steps between object and buffer, because postMessage could clone the message data automatically.

But, when I test it in my large Webpack project, which contains 5k+ modules, I cannot find any performance improvement 😢 , even the worker_thread version is a bit slower than child_process ( 50s vs 47s, both use 3 workers).

This is not a rigorous performance benchmark, and because different Webpack config / project scale / test environments may effect the test result, I advice you to try it in yourselves projects, or create a benchmark. But for now, I don't think it worths to migrate to worker_thread. child_process is enough.

@jsg2021
Copy link

jsg2021 commented Dec 15, 2021

I would think you would need more than 3 workers to see any real difference.

@alexander-akait
Copy link
Member

Also do you warn up?

The main problem is not in webpack, webpack is pretty fast in most of cases, babel/sass/tsc are bottlenecks in much of projects, try to remove babel-loader (if you use it) and look at performance

@alexander-akait
Copy link
Member

Locally I do some benchmark with multi threading (tried rewrite it on jest-worker and worker_thread too) and most often it gives a very small performance gain for most projects, cache speed ups better your projects https://webpack.js.org/configuration/cache/

@malash
Copy link

malash commented Dec 17, 2021

@jsg2021 I tried 3 workers, 5 workers and more, and got same result. In fact, as the number of workers increased, the total build time may even increase.

As a reference, here is a benchmark result when I enabled thread-loader in 2020 Q1.

image

@malash
Copy link

malash commented Dec 17, 2021

@alexander-akait Webpack 5 cache is great. It reduces a lot of CPU time after the first build. But it still has some problem so I still need thread-loader.

  1. The first build is still slow, because the cache is empty.

  2. I cannot enable warn up when using cache because of this issue webpack build hangs with warmup and new cache API #122

Here is my full webpack.config.js. I enabled thread-loader for JS/TS, but not CSS duo to this issue webpack-contrib/css-loader#1401

@alexander-akait
Copy link
Member

@malash Can you profile code? Maybe we can find the slowest places

@jsg2021
Copy link

jsg2021 commented Dec 17, 2021

@malash The Xeon scenario is more in line with what this loader is for. High-core-count CPUs. Also, If you use more workers than your CPU has cores the perf will go down as you noticed with the i7. (Hyperthreading cores sometimes don't scale the same)

The perf may also depend on how your project is arranged.

Anyways, IMO, this loader is still valuable and worth improving.

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

6 participants