Handle WSL case with "parallel: true" and "maxConcurrentCallsPerWorker"

[TiemenSch]

For users on WSL, webpack uglify hangs when parallel: true, much like #118. As mentioned there, eventually it's still Windows behind the scenes.

The fix for windows was done in #173 and includes some hints how to implement the check it for WSL.

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

See: 80d3706 for the Windows fix commit.

I thought I'd open an issue for independent WSL discussion and documentation.

[alexander-akait]

@TiemenSch it is always hangs? How fast way to reproduce this problem, maybe need fix something in node-farm

[TiemenSch]

I came across this webpack issue when trying to build an extension for JupyterLab. jupyterlab/jupyterlab#4276
My latest comment in that thread describes how I came here.

There, all WSL users cannot use the parallel option for production builds. Turning it off at least got me to the finish line. Otherwise, the symptoms (high memory, 0 CPU%, waiting indefinitely) match exactly. The mentioned PR already started discussing WSL, too.

It shows great similarity to the issue that was solved with the platform specific patch for Windows.

I would be very interested to check if applying a similar one liner to 80d3706 to handle the WSL case. Where I imagine the process.platform === 'win32' would have to be changed to something along the lines of process.platform === 'linux' && process.release === '*Microsoft'.

Please note that I have little JS knowledge, so this may be full of syntactical errors..

[alexander-akait]

@TiemenSch very interesting, can you modify code inside package:

const workerOptions = process.platform === 'win32' || os.release.includes('Microsoft') ? { maxConcurrentWorkers: this.maxConcurrentWorkers, maxConcurrentCallsPerWorker: 1 } : { maxConcurrentWorkers: this.maxConcurrentWorkers };

And check again on hangs.

Also can you post here process.platform value on WSL?

[TiemenSch]

> process.platform
'linux'
> process.release
{ name: 'node',
  lts: 'Argon',
  sourceUrl: 'https://nodejs.org/download/release/v4.2.6/node-v4.2.6.tar.gz',
  headersUrl: 'https://nodejs.org/download/release/v4.2.6/node-v4.2.6-headers.tar.gz' }
> os.release()
'4.4.0-17134-Microsoft'
> os.platform()
'linux'

Just tried to run the process after changing the mentioned line in index.js in my node_modules folder used by the build. Sadly, it hangs. What would be the command to at least let it print a quick "hello" to the console to see if the condition is satisfied?

[TiemenSch]

Ah, I think I spotted a tiny mistake in your line. It should at least be os.release().includes('Microsoft') (os.release is a function). Will try that now.

[TiemenSch]

Hmm, no luck so far. I even tried removing the guard altogether by replacing with:

const workerOptions = { maxConcurrentWorkers: this.maxConcurrentWorkers, maxConcurrentCallsPerWorker: 1 };

Any ideas how I could make a console.log() to the terminal while running the webpack?

[alexander-akait]

@TiemenSch need testing what happens in node-farm, if you can provide minimum step to create env i think i will fix it very fast

[TiemenSch]

How would I make a minimal repo, where all you run is webpack --config webpack.prod.config.js?

The one included in Jupyterlab is

var UglifyJSPlugin = require('uglifyjs-webpack-plugin');
var merge = require('webpack-merge');
var webpack = require('webpack');
var common = require('./webpack.config');

module.exports = merge(common, {
  devtool: 'source-map',
  plugins: [
    new UglifyJSPlugin({
      sourceMap: true,
      parallel: true,
      uglifyOptions: {
        beautify: false,
        ecma: 6,
        mangle: true,
        compress: false,
        comments: false,
      }
    }),
    new webpack.DefinePlugin({
      'process.env.NODE_ENV': JSON.stringify('production')
    })
  ]
});

But I think it would be better to steer clear of going too deep into the Jupyter side of things.
If you would like to debug the exact situation on WSL, you could run the following using conda python:

conda create -c conda-forge -n monaco jupyterlab nodejs
conda activate monaco
git clone https://github.com/jupyterlab/jupyterlab-monaco.git
cd jupyterlab-monaco
npm install
npm run build
jupyter labextension link .

[alexander-akait]

@TiemenSch thanks try to find time on this today 👍

[alexander-akait]

@TiemenSch Sorry, i can't reproduce problem using you steps (i am on linux). I also testing this on windows and all works fine. I don't know how i can setup WSL and not familiar with him. Can you provide more information or maybe you found solution?

[alexander-akait]

@TiemenSch friendly ping

[TiemenSch]

Hi! Installing WSL isn't too hard, luckily :). https://docs.microsoft.com/en-us/windows/wsl/install-win10

Then you'll have an Ubuntu shell up and running in no-time

[TiemenSch]

I haven't found a solution as of yet, by the way! I tried checking if something as simple as the line endings matter, since I cloned the repo on Windows and WSL mimics Linux, but to no avail.

[alexander-akait]

Feel free to investigate this, i don't have many time right now on testing this on WSL, sorry

[TiemenSch]

No worries! I'll keep stabbing at it from time to time. Pretty crammed as well ;)

[cascornelissen]

I'm running into the same problem on Windows 10/WSL/Node v10.6.0, setting parallel to false or setting maxConcurrentWorkers to 1 solves the problem but setting maxConcurrentCallsPerWorker to 1 seems to change nothing at all.

My proposal would be to change this line to something like:

const workerOptions = ((maxConcurrentWorkers) => {
  if ( process.platform === 'win32' ) {
    return {
      maxConcurrentWorkers: maxConcurrentWorkers,
      maxConcurrentCallsPerWorker: 1
    }
  }

  if ( process.platform === 'linux' && os.release().includes('Microsoft') ) {
    return {
      maxConcurrentWorkers: 1
    }
  }
    
  return {
    maxConcurrentWorkers: maxConcurrentWorkers
  }
})(this.maxConcurrentWorkers);

This solution will slow the uglifying down by quite a bit but in my opinion it's better that it not working at all.

@evilebottnawi, what are your thoughts on this?

[alexander-akait]

@TiemenSch can you try solution above? If it is solve problem, feel free to send PR 👍

[cascornelissen]

@evilebottnawi, any chance we could merge this sooner? I'm in dire need of this fix as well as I don't want to add a workaround to tens of project for just a few days/weeks until this gets released.

[TiemenSch]

What are the implications of setting to just 1 worker? Seems rather similar to a non-parallel setup (which is probably why it works). But it's a workaround and not a solution I believe. If @cascornelissen got it working, I've got no objections to merging it as a workaround for now, but it doesn't solve the issue.

[cascornelissen]

Good point, this is indeed a workaround for WSL and doesn't solve the original issue. Although I'm quite sure this isn't related to uglifyjs-webpack-plugin itself, and doubting it could be resolved in node-farm as it feels to me like an issue with WSL itself (but I don't have enough knowledge about workers/WSL to actually support that claim).

[cascornelissen]

@evilebottnawi, should I submit a PR containing this workaround?

[alexander-akait]

@cascornelissen i think better send PR to docs about this is feature doesn't support on WLS, with recommendation disable option

[ghost]

microsoft/WSL#2613
This issue is listed up here.

[alexander-akait]

Problem still exists?

[eynol]

Yes, it is.

Microsoft version 10.0.17134.285

 "uglifyjs-webpack-plugin": "^2.0.1",

[alexander-akait]

Guys i try to reproducible problem but i can't, maybe can create really minimum reproducible test repo with really minimum setup?

[ChristopherHammond13]

I know it's not a proper fix, but I hacked around this in our webpack.prod.js file by setting up UglifyJsPlugin differently depending on if WSL is running the build or not.
https://github.com/uclapi/uclapi/blob/master/frontend/webpack.prod.js

This could probably be optimised down to a one liner but it shows in a detailed way what's going on, and it's verbose enough that this workaround will be easy enough to strip out once this bug is rectified upstream from either the plugin or from WSL.

[alexander-akait]

Fixed #403