fix(index): set maxConcurrentCallsPerWorker to 1 (options.parallel)

[brentwilton]

Issues

• Fixes [v1.0.0-beta.2 (Windows)] Stuck in chunk assets optimization when parallelization is enabled (options.parallel) #118

Description

In Windows 10 if parallel: true is set then the parallel worker processes can stall and the build never completes.

The minify request is successfully sent from the plugin to the dist/uglify/worker.js which then minifies the file.

The forked worker returns the result to the plugin via the Node process.send() IPC call, but the parent plugin never receives the request from the worker.
The forked.child.on('message', this.receive.bind(this)) event in worker-farm/lib/farm.js is never invoked which causes the build to stall.

If we change the worker-farm to use maxConcurrentCallsPerWorker: 1 it will prevent the parent from queuing multiple requests via IPC to the workers. It waits until it receives a response from the worker, then it dispatches the next file to minify. This lowers the amount of pending IPC traffic between the plugin and the workers and does not trigger the issue.

Adding this limit will mean that the worker will need to wait for the parent to receive the response before getting a new file to process, but it allows parallel processing to complete on Windows.

Notable Changes

Changes the worker-farm to use { maxConcurrentCallsPerWorker: 1 }.

[jsf-clabot]

All committers have signed the CLA.

[michael-ciniawsky]

Thx 👍 Do you know how does this change may affect performance especially on other platforms ? Should we guard this to be set only on windows ?

[brentwilton]

On other platforms it will be slightly slower. Approximately (number of output modules) * (plugin worker IPC round trip time) slower.

I can add a guard to make it Windows only.

i.e.

var workerFarmOptions = {
    maxConcurrentWorkers: this.maxConcurrentWorkers,
};
if (os.platform() === 'win32') {
    Object.assign(workerFarmOptions, {
        maxConcurrentCallsPerWorker: 1,
    });
}
this.workers = workerFarm(workerFarmOptions, workerFile);

[michael-ciniawsky]

Yep, please add a platform check for this e.g

const workerOptions = process.platform === 'win32' ? {...} : {...}

this.workers = workerFarm(workerOptions, workerFile)

[joshwiens]

Has there been any root cause analysis as to why this is actually happening.

Uglify issue, ours, Microsofts ? ...

[michael-ciniawsky]

Has there been any root cause analysis as to why this is actually happening

Not really, I know that the IPC implementation in node for windows/unix(posix) are entirely different due to general OS differences and the windows IPC can behave differently for subtile aspects in case the underlying logic is hard to shim behaving 'the unix way', more details would need further investigation 😅 (maybe good to open an issue in worker-farm or node to clearify)

[StalkAlex]

This PR is not fully fixing support on Windows, because if I run on BashOnWindows(Ubuntu inside Windows 10) with parallel: true it still hangs.

This condition os.platform() === 'win32' is not correct for BashOnWindows, but maxConcurrentCallsPerWorker: 1 should be set as well as still Windows.

[alexander-akait]

@StalkAlex can you send PR with reproduce steps?

[StalkAlex]

@evilebottnawi how such PR could look like?

BashOnWindows

> os.platform()
'linux'
> os.release()
'4.4.0-43-Microsoft'
>

So one should also check for release besides platform.