Use worker threads to implement render()

[nex3]

Currently, we implement the asynchronous render() method in Node.js in one of two ways:

• By default, it's run through the AsyncEvaluator which is synchronized via code gen with the synchronous Evaluator. This is much slower than running synchronously, and imposes developer overhead by requiring synchronization.

• If the user passes in a reference to the fibers package (which uses a C++ extension to add a coroutine-style asynchrony model to Node), we run the synchronous code path and use coroutines to call out to async helpers. This is as fast as running synchronously but requires the end user to opt in to a C++ compilation which may be difficult to compile.

Sass's business logic itself is fully synchronous, so we get no benefit from this asynchrony other than the ability to invoke asynchronous function and importer callbacks. I think we can avoid the downsides of both of these methods by running the whole Sass compiler in a worker thread and using Atomics.wait() to block that thread until the callback completes.

[hegelocampus]

I am thinking about taking on the implementation of this and I had a few quick clarification questions:

• First of all, is this still a desired enhancement?
• To be explicitly clear, this would involve the porting of the needed components of the node.js worker_threads library, right?
• Am I correct that the check for fibers along with the call to renderAsync would all be removed in favor of renderSync?
• Then in src/node.dart a worker thread would be setup to deal with the rendering within _renderSync?

Sorry I've got all of these clarification questions, I don't want to write a bunch of code only to realize I completely misinterpreted what you were asking for here.

[nex3]

First of all, is this still a desired enhancement?

Yes, definitely!

To be explicitly clear, this would involve the porting of the needed components of the node.js worker_threads library, right?

You wouldn't need to port anything. Dart compiled to JS can invoke JS libraries, so you'd just need to call this library.

Am I correct that the check for fibers along with the call to renderAsync would all be removed in favor of renderSync?

We'd definitely be getting rid of fibers. I think we want to retain a distinction between renderAsync() and renderSync() both for backwards compatibility, and so we can avoid the overhead of spinning up a worker thread when only synchronous callbacks are necessary.

Then in src/node.dart a worker thread would be setup to deal with the rendering within _renderSync?

You'd be doing this in _renderAsync rather than _renderSync.

[lehni]

It looks like this feature has now become more urgent than before, since node-fibers has officially reached the end: laverdet/node-fibers@8f28098

[jamesrwaugh]

Because of the breakage of fibers in Node.JS v16 and this dependency, It currently looks like using sass and dart-sass will prevent a project from upgrading to v16.

[nex3]

@lehni I'm working on a blog post describing the potential ways of working around this issue. We're going to be focusing on the Node embedded host as the main workaround.

@jamesrwaugh Sass doesn't actually declare a dependency on fibers—it just takes a parameter that's expected to follow the fibers API. So it won't actually prevent apps from upgrading.

[ArcanoxDragon]

What is the status of this issue? Our build time increased by about 500% when switching from node-sass to sass in our Webpack project. We have to revert back to node-sass and accept incompatibility with certain style libraries as a result because a 300 second build time is not acceptable.

Is there anything in particular blocking progress on this issue being implemented?