Use persistent Node processes for compilation

[SLaks]

Instead of spawning a separate Node process for each file, we should maintain a pool of persistent processes and invoke the compilers in them.

Without this, Node needs to re-compile all of the JS files for each compiler on each invocation; this can be costly, especially on non-SSDs.

Here are some implementation notes:

• We should have one Node process per machine CPU core to efficiently parallelize. Compilation is partly IO-bound, so we should also compile multiple files in parallel per Node process. I don't know the correct balance.
• We need to restart the Node processes if they fail, and make sure they're closed when VS exits (I think Windows does that for child processes automatically)
• We need to find a repeatable function call to invoke each compiler, including source map compilation args.
• We need to decide how to communicate in both directions.
  We can make the Node background runner listen for JSON instruction payloads on stdin, and have it write resulting filepaths (also in JSON) to stdout; the C# compiler would then wait for a single line of output)
  Alternatively, we can expose an HTTP server; this would make much of the code simpler & potentially more reliable.
• Each node process should load every compiler; I don't think there is any reason to specialize

[am11]

This is a great idea and it will save us from creating / disposing Process overhead for each compiler call. We can launch node in interactive in a process and perform the desired operations. That would be a different approach from the (current) "sending CLI arguments to compiler" one. It would be like sending automated JS input to Process running Node.exe -i.

[SLaks]

node -i is not enough to reliably get results, especially with parallel compilation & error handling.

We would need an HTTP server that can accept requests to compile files & serve HTTP responses with the results, as well as a heartbeat check & a shutdown command.

We should also consider grunt compatibility / integration.

[am11]

Perhaps with OWIN we can architect the desired HTTP stack; since the web tooling required to function WE already has OWIN assemblies.

Nonetheless, It would be a dilemma: as OWIN is a counterpart of NodeJS and we would be encapsulating NodeJS server in OWIN. But some people have already done this kind of thing.

[SLaks]

No; we need to run an HTTP server in the Node.js processes, so that WE can send requests to them (and get HTTP responses with compilation results).

None of that would do any good; we simply need to use Node's built-in HTTP server (perhaps with higher-level modules like Express for routing)

[am11]

On which port number we should be running?

Also, how would it work with in restrictive environments, where most of the unused ports are blocked? Some workplaces have such policies.

[SLaks]

Each Node process will need to choose a random unused port to listen on. (the C# code can choose a port number, then pass it on the command line)

Firewalls block ports from the outside; I've never heard of anything restricting communications within the machine.

[am11]

We can consider a load-balancer to invoke fixed / maximum number of workers (say 8), where each worker is capable of entertaining all kinds of compilation requests:

https://mazira.com/blog/introduction-load-balancing-nodejs (also see part 2 for seaport module)

Ideally, if we select maximum workers topology (as opposed to fixed one), we may want establish a logic to spawn a worker on demand and turn off when its inactive.

[am11]

Here is another idea. We can devise a job format, something like:

{
    "type": "compile",
    "filename": "path\\to\\some\\file.coffee",
    "target": "path\\to\\target.js",
    "module": "coffeescript"
}

And then store these (JSON-base) jobs files in a directory.

Then using require("fs").watch( _ ) function we watch that directory via single forever-running nodejs process. Once the job is executed, the corresponding job file is deleted. If the node process is terminated via Task Manager (or crashed), then while registering the next job via C# code, we can see if there are multiple pending jobs, then spawn a new long-running node process.

For callback; on completion, we can use Edge.js (see Hanselman's blog post) to notify the .NET code when a task is finished.

(perhaps we can use a DB instead of a directory for queuing jobs)

[SLaks]

Neither of those options offer any advantages over plain HTTP, and introduce a lot of complexity that HTTP handles for you.

I don't know whether it would be easier to maintain & communicate with multiple Node processes directly within the C# code (as I was saying) or using a separate Node.js load balancer.

[am11]

The load-balancer will make it easy to manage workers; as opposed to keeping track of various HTTP node server processes in C# code. Only one process would be initiated, which will further generate workers.

[ctolkien]

In the interest of simplicity, I'd suggest initially going with a single Node process rather than a process per core. We're yet to have an issue with running a single Node process to handle our compilation at the moment (as mentioned in another thread issue, it's ~10 secs to compile with WE, <500ms to compile on straight Node).

Obviously still need to manage respawning this when it falls over...

[SLaks]

If it takes 10 seconds to compile with WE, something else is wrong.

What processes use CPU during those 10 seconds? (check the per-process CPU graphs in Process Explorer)
Unless you have very large numbers of source files, Node's initialization shouldn't take that long.

[ctolkien]

Have taken a quick video here, showing the saving, task manager, and output window. Takes 7 seconds for this to complete at the moment (reboot seems to have improved things a bit):

https://dl.dropboxusercontent.com/u/3274103/lesssaving.mp4

Here is my LESS settings:
http://i.imgur.com/2U3kXro.png

Should point out too that this project is on a spinning rust drive.

[ctolkien]

Giving this a bump to hopefully get some life into this issue. We're still unable to use WE's Less option due to the perf issues - we're currently using Gulp to watch the files and compile on save.

[am11]

Here is what I have got so far:

• Made a simple node.js based server.js.
• The server pass the request to controller.js.
• There is a state-machine like handler, which passes the request to appropriate sub-handle on bases of service name from query string.
• Implemented LESS handler, which hooks directly to their parser.
• Successful compilation from browser.

The URL format is simple. I used GET method and pass everything via query string:

http://127.0.0.1:1337/?
service=less&
sourceFileName=C:\Users\Sony\Documents\Visual Studio 2013\Projects\WebApplication1\WebApplication1\Content\LESS\a.less&
targetFileName=C:\Users\Sony\Documents\Visual Studio 2013\Projects\WebApplication1\WebApplication1\Content\LESS\a4444.less.css&
mapFileName=C:\Users\Sony\Documents\Visual Studio 2013\Projects\WebApplication1\WebApplication1\Content\LESS\a4444.less.css.map&
strictMath=true

Next thing is error handling and preparing the JSON output. Then it will be ready to replace our LESS compilation (only).

That's the first part, now after this there are eight more compilers to go. After that refactoring WE code.

If anyone want to chip in, I can push the code to a feature branch.. :)

[SLaks]

This is actually a security hole; any webpage can send a request to the server and end up reading any file on disk.
You need to disallow requests from client-side Javascript.

You can do that by requiring a custom HTTP header in every request to the service. C# can send those easily, but foreign JS cannot (unless you allow CORS)

[am11]

I haven't started working on client yet; using browser for debugging. Here are some security checks I have (currently commented):

// Unauthorized access checks
if (!request.headers["web-essentials"] || request.headers["origin"] !== "web essentials" || request.headers["user-agent"] !== "web essentials")
{
    response.writeHead(404, { "Content-Type": "text/plain" });
    response.write("404 Not found");
    response.end();
}

Which we can call from C# like this:

// using System.Net.Http;
HttpClient client = new HttpClient();
client.DefaultRequestHeaders.Add("origin", "web essentials");
client.DefaultRequestHeaders.Add("user-agent", "web essentials");
client.DefaultRequestHeaders.Add("web-essentials", "web essentials");

//HttpResponseMessage response = client.GetAsync("http://127.0.0.1:1337").Result;

Plus we will be running on random port, each time Web Essentials is loaded.

In order to take more robust security measures, we can intermittently check something random on the file-system which doesn't change frequently and both C# and nodejs have access too (for instance send the name of a random dll file and its hash via request headers and let njs calculate and compare its SHA-1 hash in a discontinuous manner); since JavaScript in browser doesn't have access to FS.

[madskristensen]

Let's release 2.3 before this makes it in. Seems like a change that could cause unforeseen issues if not properly tested

[SLaks]

Adding a header should be enough.
You could also pick a cryptographically-random secret, pass it on the command-line to the Node server, then include it in every request (equivalent to a CSRF token).
This will securely block requests from other sources, without needing disk access.

[am11]

"Something went wrong reaching:" in sass is a safeguard for C++ segmentation fault, which I submitted PR for: sass/node-sass#361

I don't know when they will merge in. Their release cycle is usually bit slower.

As you can see there, it can happen when we save (to compile) before completing the statement. (for instance div { color: red without the closing mustache OR the unsupported features of SASS 3.x which you can find in libsass' issue tracker).

I have also spotted the bugs in their upstream libsass and awaiting for my existing PRs to merge in, before I can do more. Meanwhile, currently there are number of hacks in our code, just to prevent node-sass and its upstream from crashing during the lifetime of WE's NodeServer.

We should expect other services to work normal; Autoprefixer, LESS and JS compilers and linters.

[SLaks]

As an interim workaround, we could try to parse the text with VS's SASS parser, and not compile if that fails.

On the other hand, that will completely prevent people from using features that VS cannot parse. (I don't know if there are any such)

[tysonmatanich]

But what about completely valid Scss files?

[SLaks]

Since VS's parser would (hopefully) not give an error, they would be compiled.

[am11]

Can you share some code which is causing issue? I tried with some zurb-foundation sass code from a rails project and can't reproduce the bug. FWIW, it has the import depth of five.

Also, if the code is valid and compiling well with node-sass, it should compile with Web Essentials. If its a valid SASS 3.x code, and node-sass / libsass is yet to support that particular feature, then it will throw, we will catch and show the pretty error.

[tysonmatanich]

Sorry I can't share the code, but let me know when you have a new vsix file for me to test with.

[am11]

Well that's a pickle. Without reproduction steps, we can't do much. The current build is at version 2.2.6.1 (released today).

@SLaks, can you please test if you can reproduce this 404 with SASS or LESS?

Thanks!

Sent from my Windows Phone

---

From: Tyson Matanichmailto:[email protected]
Sent: ‎7/‎15/‎2014 10:12 PM
To: madskristensen/WebEssentials2013mailto:[email protected]
Cc: Adeel Mujahidmailto:[email protected]
Subject: Re: [WebEssentials2013] Use persistent Node processes for compilation (#381)

Sorry I can't share the code, but let me know when you have a new vsix file for me to test with.

---

Reply to this email directly or view it on GitHub:
#381 (comment)

[tysonmatanich]

Figured it out, looks like it must have been a change in the Sass compiler. The Scss built fine the other day with WE but stopped with the upgrade. What was strange however was that unrelated files would fail to compile.

[ctolkien]

@am11 , in a new project... I'm unable to replicate any issues.... just seems to happen in our existing project.

I've tried the exact same thing in SASS - getting the same error " Something went wrong reaching..."

Again, it seems to work fine when not using "@import".

[am11]

@ctolkien, finally we have found the source of the similar issue that @blowsie reported here. The Strict Math option was disabled. You need to set it to true. To change the settings: Tools > Options > Web Essentials > LESS > Strict Math.

HTH

[am11]

I personally think this should be set to true by default. But as stated in their docs, this is a controversial matter: http://lesscss.org/usage/index.html#command-line-usage-strict-math

[ctolkien]

OK, setting Strict Math to true fixed the issue.

Unfortunately however, migrating to a persistent node process hasn't resolved the performance issues entirely. After a reboot and running a save several times (so everything is warm), this is the best case scenario I could get:

22/07/2014 11:41:15 AM: LESS: Compiling Soda.less
22/07/2014 11:41:17 AM: LESS: Soda.less compiled.
22/07/2014 11:41:20 AM: LESS: bootstrap.less compiled.

5 seconds. Much better than the ~10 it was previously, however gulp reports times of ~1ms.

This is perhaps for another ticket.

[am11]

@ctolkien, wait for next nightly we have fixed a handful of issues; related to performance as well.