Hacker News new | ask | show | jobs
by IsaacSchlueter 5188 days ago
First of all, writing bug reports to Hacker News is usually a bit like complaining about the government at a bus stop. It's fun, and you'll get lots of agreement from crazy people, and maybe start a little riot, but writing your representative is a better way to effect change.

We have two mailing lists, and an issue tracker on github. If someone hadn't emailed this to me, I probably would not have seen it.

There are three problems I see with your test.

1) It serves the file twice, since you don't filter out the /favicon.ico url

2) you're converting the file contents to a string, which needs to be converted back into raw bytes to send.

3) your gist is in JavaScript, but I see that you're actually running CoffeeScript. I'm not sure what kind of wrapper CoffeeScript is adding to the equation.

I ran the same test serving the ChangeLog file from node's source folder with node v0.6.15 (to be released tomorrow) https://raw.github.com/joyent/node/v0.6/ChangeLog

Here's the source code of the server.js: https://gist.github.com/2338851

It did use more CPU than nginx, but not 100x as much. Requesting the /buffer url made it spike up to about 0.2% cpu usage. Requesting the / url made it spike up to about 0.3%. Even when I disabled the /favicon.ico check, its behavior is nowhere near what you're seeing.

When I ran it with coffee script, it actually uses this JavaScript: https://gist.github.com/2338874. So, there's an extra Function.call in there, but otherwise, it's pretty much identical. My coffeescript version is here: https://gist.github.com/2338879

So, something is odd with your situation, and from your test, it's not clear what. I'd love to get more details.

On the more general note, clearly we in the node world are perfectly fine with using nginx to serve static assets. Qv. the nodejs.org home page. But node's performance here should be within an order of magnitude, or something is broken. This is not expected or normal performance that you're seeing. We run the "serve a big string over http" benchmark quite often, and so this was very surprising to me.

1 comments

Aha, I take back some of what I said. ChangeLog is not big enough, that's all.

When running with a 16MB file, there's some more CPU spiking, but only if it's served as a single write.

Investigating further, I found that there's a bug in my gists, and I'm testing res.url instead of req.url, so I was making all the exact same mistakes in your test! Hah!

With this test, we see it spike up to 10% for a big string, but a big buffer still stays well below 1%. https://gist.github.com/2339010

I did it in coffeescript to just rule that out as a possible source of work.

The moral of the story is that Buffer<->string conversion is WAY too expensive in Node, and leads to situations where the easy approach leads to poor performance. That's a problem. Thankfully, it's one we already are aware of, and plan to fix, and it's easy to work around by just using a buffer rather than doing extra conversions.

hi isaac,

thanks for responding, it's nice of you to acknowledge it as a bug, and say you're working on it.

> First of all, writing bug reports to Hacker News is usually a bit like complaining about the government at a bus stop.

just a little note on this one, i've made clear in my original writing this is not hate and i sent an email to substack, and ryan to some others i know. i guess i will send you an email next time as well.

not filing as a bug: you are right, i should have done that.

Buffer<->string conversion is WAY too expensive in Node: yes it is, hope you can make this way better, because we don't want to hit cpu each time we pull data from a e.g. database.

writing it to hackernews/stackoverflow: it's because i'm asking for solutions outside of nodejs domain, varnish,nginx or any other way this issue can be worked around. i'm sure you understand. and this was quite helpful as we're now deploying a few varnish and nginx servers to mitigate this problem.

Sorry if my response was a bit overly strong. I know you're a good guy and we all just want these things to work well. I wasn't offended, just a bit confused by the results that weren't matching what I was seeing.

Of course, poking a bit further showed that converting a 16MB string to a buffer actually is quite expensive. You can avoid this by using buffers directly when you're serving files (or better yet, just pipe a fs.ReadStream directly into the response, so it'll stream it as it reads from disk, and buffer as little as possible.)