avatarTobias Koppers

Summarize

Bugfixing with thread-loader and Scope Hoisting (week 15)

2017/07/10–2017/07/16

This week was sponsored by Segment. Thanks for this!

Next to normal bugfixes for webpack 3, I hunted some really difficult/annoying bug and I want to tell you this story.

I was investigating how to improve build performance with the cache-loader + thread-loader combination. It really worked out great, but for some scenarios it just started to hang. After a bit of isolation I found out that it only occurs when using the sass-loader inside of a thread-loader. But it only occurs in bigger builds and doesn’t occur when compiling only few files.

I asked myself: What is special to the sass-loader when running on a thread-loader worker? The sass-loader uses the this.resolve function of the loader API to resolve @imports. This causes a message send from worker to main process and calling the original resolve method from webpack. The response is sent back form the main process to the worker process.

The thread-loader doesn’t use the normal ipc node.js API (it’s to slow and not suitable for binary data), but has a custom implementation on top of binary node.js streams of stdio 3 & 4.

After a bit debugging I figured out that sometimes the resolve message is not transferred from worker to main process. I assumed that is some error in my custom ipc implementation. Node.js stream are complex.

I didn’t find one…

I figured out the actually problem by luck after reading some piece of sass-loader source code.

// This queue makes sure node-sass leaves one thread available for executing
// fs tasks when running the custom importer code.
// This can be removed as soon as node-sass implements a fix for this.
const threadPoolSize = process.env.UV_THREADPOOL_SIZE || 4;
const asyncSassJobQueue = async.queue(sass.render, threadPoolSize - 1);

node-sass uses threads from the Node.js thread pool to do it’s processing. This isn’t a too big problem until you start to add a custom importer, which the sass-loader does to inject webpack’s resolve logic (by calling the resolve method from the loader API).

While running the custom importer code the thread for the sass processing is still blocked by node-sass. This cause problems when more than UV_THREADPOOL_SIZE - 1 sass modules are processed simultaneous and resolve calls are issued. In this situation too many Node.js threads are allocated by node-sass and ipc communication do not happen. This means resolve calls hang, sass compilation hangs, webpack hangs…

The fix should happen in node-sass. It should use it’s own thread pool. Node.js thread pool is for IO anyway!

As workaround you can limit the simultaneous processed jobs in the thread-loader:

{
  loader: "thread-loader",
  options: {
    workerParallelJobs: 2
  }
}

Annoying issue…

I also discovered a bug in the experimental ModuleConcatenationPlugin (Scope Hoisting). It can reorder the evaluation of imported modules, which is not correct.

// module.js
import "commonjs.js";
import "ecmascript-module.js";
console.log("eval module.js");

This will eval in this order by default: commonjs.js ecmascript-module.js module.js

But with the ModuleConcatenationPlugin it will evaluate in this order: ecmascript-module.js commonjs.js module.js

At first I thought: Is this a bug or is the evaluation order implementation defined by spec?

So I read the spec a bit. It seems like the order is defined as in source code occurrence order. It’s a bit hidden, but it’s defined.

[[RequestedModules]]
List of String
A List of all the ModuleSpecifier strings used by the module represented by this record to request the importation of a module. The List is source code occurrence ordered.
...
6. For each String required that is an element of module.[[RequestedModules]] do,
  ...
  c. Let status be requiredModule.ModuleEvaluation().
  ...

The broken situation is pretty likely to occur. It will occur everytime you import a commonjs module before a EcmaScript module:

import React from "react"; // CommonJS
import Component from "./component"; // ESM
...

But in most cases this don’t case any issue. Modules shouldn’t depend on the evaluation order. If they do depend on other modules evaluated before, they usually depend on these modules.

To fix this, we need to

  • either deoptimize this fragment
  • or find a more clever concatenation technique to ensure correct order.

Deoptimization would be unfortunate, but good a first step to ensure correctness. Maybe we make it a option to the ModuleConcatenationPlugin to opt-in to unsafe but better optimization.

Maybe this issue solves itself when more modules switch to ESM instead of CJS.

A more clever concatenation technique could move the imports to the correct location in the concatenated module, but is probably a bit work to do.

< week 12–14 week 16 >

webpack is not backed by a big company, unlike many other big Open Source products. The development is funded by donations. Please consider donating if you depend on webpack… (Ask your boss!)

Special thanks to these sponsors: (Top 5)

JavaScript
Recommended from ReadMedium