fix(uglify/index): don't use a worker farm unless more than one process is available

[hedgepigdaniel]

This should save some memory due to sharing common data within one process instead of spawning a second one.

[jsf-clabot]

All committers have signed the CLA.

[alexander-akait]

@hedgepigdaniel Looks like it is save memory only for one core system (one thread), right?

[hedgepigdaniel]

The difference is for a two core system - since the default number of workers is numcpus - 1. In our case we have parallel: true in the config, and this causes our two core build server to run two processes even though only one of them is actually doing the minifying.

Tbh, I haven't tested that is uses less memory but I assume that there is at least some memory that is shared in one process that needs to be duplicated in a second one. I tested that it works though 👍

[alexander-akait]

@hedgepigdaniel Looks you are right today, i am testing memory usage and i think we can merge this 👍

[michael-ciniawsky]

Could you additionally try with os.cpus().length instead of os.cpus().length - 1 and see if this works ?

[alexander-akait]

@michael-ciniawsky os.cpus().length:

1. When we have one core (os.cpus().length === 1) - we create worker, it is wrong (one thread should be left for webpack and no new thread)
2. When we have two core (os.cpus().length === 2) - we create two worker, it is wrong (one thread should be left for webpack and don't create new thread because we have 1 thread, it is does make sense, only decrease memory/cpu for creating one thread, i.e. no new thread)
3. When we have four core (os.cpus().length === 4) - we create 4 worker, it is wrong (one thread should be left for webpack and 3 for uglify)

[michael-ciniawsky]

The worker-farm docs don't state that there must be one core 'freed' for the 'main' process, but maybe you're right, as mentioned worth a try :).

[alexander-akait]

@michael-ciniawsky it is good practice, If at least one thread completed it is require webpack main thread to create new thread for other file (otherwise we will lose time).

[alexander-akait]

@michael-ciniawsky Also using os.cpus().length create new problem for one core system (other used in CI system). We create one extra thread on 1 core system and spend extra resources (memory and cpu). It does not make sense to create a new thread on one core system

[michael-ciniawsky]

@evilebottnawi We don't certainly know for sure if this would be the case or not, I'm not disagreeing but by at least trying we might get proof of the circumstances here

[alexander-akait]

@michael-ciniawsky why we should create new thread on one core system? It is does not have any sense

[alexander-akait]

Each creating new thread using more memory and cpu

[alexander-akait]

@michael-ciniawsky You can justify in detail what you do not agree with PR, or approve PR

[michael-ciniawsky]

I don't disagree with anything in this PR, but maybe using os.cpus()-length - 1 is unnecessary as mentioned in the review comment and can be removed

[alexander-akait]

@michael-ciniawsky What exactly do you disagree? os.cpus().length - 1 is good practice for all multi threading platforms, do you have knowledge about implement multi threading programs or have experience with this, if not it does not make sense to participate in the discussion and slow down solution of the problem. Each program running in multithreading mode has master process and we should always leave one thread for master (creating new threads, send signal for other process, regular work)

[alexander-akait]

In short:

• you have 8 files to uglify
• you system support 4 threads.
• plugin create 4 thread for uglification
• 1,2,3 thread completed uglification, if we don't leave one thread for master, no new threads for other 4 files, because master can't create new threads. We will have to wait until we finish 4 thread complete you works, because create new thread places in master code https://github.com/webpack-contrib/uglifyjs-webpack-plugin/blob/master/src/uglify/index.js#L53

[joshwiens]

Agree with @evilebottnawi here. If you don't have one master thread, you end up context switching all over the place and take a huge performance hit and may as well not thread it at all.

Reserving one thread to orchestrate execution has been a generally accepted practice pretty much since we started leveraging the power of multi-core processors.

[alexander-akait]

@d3viant0ne can we merge this? i don't have administration right here

Not available