5 ms·

They didn't debug anything. They just stuck a job queue on it and then rewrote part of it in Rust. That's the point of my comment. Using their code in the post

by orf 3y ago

They didn't debug anything. They just stuck a job queue on it and then rewrote part of it in Rust. That's the point of my comment. Using their code in the post (gist here[1]): $ pv lol.tsv | node lol.js 1.71GiB 0:00:07 [ 225MiB/s] [================================>] 100% Finished reading the file. Do we live in a world where engineers using a mature language like NodeJS with a pretty simple line-by-line file reading process and a string split think they hit a hard wall at 700kb/s, and the problem is not their bad code? 1. https://gist.github.com/orf/92bc26381e3dc00c8e96af262768591b https://gist.github.com/orf/92bc26381e3dc00c8e96af262768591b

nerdponx 3y ago
But is the code shitty? It looks like boring normal code to me. I'm not sure what point you're making here. The point of the article is that you can write really boring normal code in NodeJS and it's really slow, or you can write really boring normal code in Rust and it's a lot faster, and it's not that hard to use the latter to replace hot paths in the former.
- disgruntledphd2 3y ago
  It's probably all the memory allocations. I know that when I do work like this, creating a huge array up front and assigning to that typically results in massive speedups. I know basically nothing about Node though, so I could be completely wrong here.
  - Karrot_Kream 3y ago
    Yeah that's one way to do it. Another is to reuse structures where possible to not force reallocation. There's a bunch of ways.
  - xmcqdpt2 3y ago
    Node.js has a decent JIT, or so I've heard. I have experience with the HotSpot JVM. For the JVM this code would absolutely stop allocating on the heap past 10k iterations of the loop. "Split a string then grab the parts in a hot loop" is ground stakes for JIT compilers. If they are seeing any GC activity once the loop gets hot, something else is going very wrong.
    - disgruntledphd2 3y ago
      Yeah but if you're appending in a loop you'll need to allocate memory every time, right? Can a JIT avoid or rewrite this?
      - xmcqdpt2 3y ago
        Yep the Hotspot JIT will absolutely allocate on the stack and reuse memory allocations inside loops. It also has escape analysis and it will detect you don't need the array of strings after the iteration. In that case it usually just allocates the strings directly at their final locations on the record object.
        disgruntledphd2 3y ago
        Wow, that's super cool. Almost makes me wish i did stuff on the JVM (but until someone rewrites LAPACK for it, that seems unlikely).
- orf 3y ago
  > But is the code shitty? It looks like boring normal code to me. I'm not sure what point you're making here. 1. It should never be that stupidly slow 2. That should be obvious to anyone with a napkin and some math 3. Excessive allocations are highly unlikely to be the root cause of the slowness 4. Instead of debugging, they seemingly spent a lot of time putting it on Kubernetes, scaling it out to 25 instances and then finally rewriting a portion of it in Rust. The whole thing smells of confusion and multiple issues being conflated together, with the rewrite in rust fixing these by virtue of deletion. And if the root is was the GC, then some `--trace-gc` output in the post would be nice at the absolute minimum. Showing a for loop with a string split doesn't cut it.
  - maurice2k 3y ago
    Exactly. I'm not a super-expert in Node.js but worked a lot with it and also know a lot of internals. And I can definitely say: IO speed is pretty good. To be honest, I've never come across a language with bad IO speed and I've seen quite a lot of languages. I also did some benchmarks (like orf did) and speed is pretty good. Maybe Rust is a little bit better because it's nearer to syscalls than Node.js is but we're not talking about 760kb/s vs. 250mb/s. 200GB of logs and 25 instances means 8GB per instance. Not sure what they're doing, but processing of 8GB taking 3h is like doing it with my 80286 processor ;-) Even with inserting stuff in MongoDB this seems way to slow. And they only replaced the file IO part with Rust... I guess the records array is getting pretty huge which might result in swapping to disk. But then: don't let it happen, just flush it to the database or whatever. I believe the problem could have been solved with Node.js without issues.
- mrighele 3y ago
  The point the parent is making is that the boring normal code in NodeJS is not slow, at least not as presented in the article. The bottleneck is probably somewhere else. Allocation is usually not slow in a garbage collected language. Deallocation can be, but that is usually easy to check (for example with the --trace-gc flag, or running with profiling enabled and checking the results with chrome devtools). It would have been nice if they shared such analysis. With the limited information provided in the article, there are other points that I would check before going as far as writing a module in another language. For example > records.push({ They are collecting all the parsed rows in a single array. This is going to consume memory proportional to the size of the file, potentially in the same order of magnitude (depends on how much information is being collected and how much discarded).
- ryanjshaw 3y ago
  The low throughput is definitely a code smell. In my experience, a common mistake in handling files too large for memory is to read the data faster than you process it, consequently running out of memory. For example, maybe they have a loop reading lines and doing a bunch of async DB writes: your process will read at disk speed and write at database speed (relatively slower), and you'll run out of memory due to async tasks/context queuing up in memory. Perhaps this is why it worked to split the file into 8GB units X 25 servers: it didn't blow their memory budget per server.
bhaak 3y ago
Try it with something a little bigger than what you used. Then you would have seen that the GC kicks in and performance goes down after about 2GB of processed data. With my first try with bigger data it blew up with an out of heap memory error. The article mentions them having memory problems as well. They probably operated at the upper levels of available memory and even got into swapping and then 700kb/s is not surprising. So yes, this absolutely looks like a memory allocation issue.
- orf 3y ago
  Yes, of course the unbounded array will cause you to run out of memory. That’s… not news, and it absolutely isn’t a “memory allocation issue” in the sense they are using it in the post. Try modifying it to clear the array (a DB flush, if you will) every 100k lines.
  - bhaak 3y ago
    That's what you get if you use the (simplified) example code that is in the article. It's also what you can derive from their description of the problem. Now we don't have any Rust code to compare so we can't say how it's different. But everything points towards a memory allocation issue that is just not their in simple Rust code.
  - tomhallett 3y ago
    Following you in the abstract, but still a bit confused. * "unbounded array" - are you referring to `records`? * If so, is the memory of `records` too large? Or is it that `records[i].pathname` has a reference to `fields[7]`, AND v8 will retain all copies of `fields` until the loop is done? Do you think this code would be more GC-friendly? ``` let count = 0; const MAX = 1000; const flushedRecords = []; for await (const line of readlineStream) { const fields = line.split('\t'); const httpStatus = Number(fields[5]); if (httpStatus < 200 || httpStatus > 299) continue; records.push({ pathname: fields[7], referrer: fields[8], // ... }); if(count == MAX || isLastLine(line)){ flushedRecords.push([...records]); records = []; count = 0; } else { count++; } } ``` Or were you thinking something totally different?
nicoburns 3y ago
> They just stuck a job queue on it and then rewrote part of it in Rust IMO that's not an inappropriate response here. You can make this code in Node, but it's super-awkward trying to reduce string allocations in JavaScript. Much easier to rewrite in Rust where such things will be fast even with the naive code. For code such as this you can likely do a 1:1 port of the Node code and it will be 10x faster. Which is much better than getting yourself in twists trying to make Node fast at tasks it's not good at.
- orf 3y ago
  I took the code from their post, the one they highlighted as the issue, string allocations and all, and ran it. It was 1000x as fast as their self-reported runtime. Does this really, really not scream “there is another issue” to you? Do you really think, even with string allocations in a hot loop, NodeJS with its very optimised runtime would struggle to hit 1mb/s?
- LudwigNagasena 3y ago
  Naive Rust code is super-awkward compared to naive JS code. I doubt one can do 1:1 port of anything more complex than Hello World (even FizzBuzz may get you tripped up).
  - __s 3y ago
    Not sure what you mean by naive rust, but rust has good webassembly integration. To give you some context, here's a post I made about rewriting 10k lines of js into rust: https://www.reddit.com/r/rust/comments/k3jy5g/i_rewrote_10k_lines_of_js_into_rust_over_the_last https://www.reddit.com/r/rust/comments/k3jy5g/i_rewrote_10k_... (I went on to rewrite the server from nodejs to rust too)
  - nicoburns 3y ago
    YMMV, but my Rust code ends up looking pretty much identical to my TypeScript code most of the time. And especially for code like this where there are no complex lifetimes.
  - jplusequalt 3y ago

by orf 3y ago

nerdponx 3y ago

But is the code shitty? It looks like boring normal code to me. I'm not sure what point you're making here. The point of the article is that you can write really boring normal code in NodeJS and it's really slow, or you can write really boring normal code in Rust and it's a lot faster, and it's not that hard to use the latter to replace hot paths in the former.

disgruntledphd2 3y ago
It's probably all the memory allocations. I know that when I do work like this, creating a huge array up front and assigning to that typically results in massive speedups. I know basically nothing about Node though, so I could be completely wrong here.
- Karrot_Kream 3y ago
  Yeah that's one way to do it. Another is to reuse structures where possible to not force reallocation. There's a bunch of ways.
- xmcqdpt2 3y ago
  Node.js has a decent JIT, or so I've heard. I have experience with the HotSpot JVM. For the JVM this code would absolutely stop allocating on the heap past 10k iterations of the loop. "Split a string then grab the parts in a hot loop" is ground stakes for JIT compilers. If they are seeing any GC activity once the loop gets hot, something else is going very wrong.
  - disgruntledphd2 3y ago
    Yeah but if you're appending in a loop you'll need to allocate memory every time, right? Can a JIT avoid or rewrite this?
    - xmcqdpt2 3y ago
      Yep the Hotspot JIT will absolutely allocate on the stack and reuse memory allocations inside loops. It also has escape analysis and it will detect you don't need the array of strings after the iteration. In that case it usually just allocates the strings directly at their final locations on the record object.
      - disgruntledphd2 3y ago
        Wow, that's super cool. Almost makes me wish i did stuff on the JVM (but until someone rewrites LAPACK for it, that seems unlikely).
orf 3y ago
> But is the code shitty? It looks like boring normal code to me. I'm not sure what point you're making here. 1. It should never be that stupidly slow 2. That should be obvious to anyone with a napkin and some math 3. Excessive allocations are highly unlikely to be the root cause of the slowness 4. Instead of debugging, they seemingly spent a lot of time putting it on Kubernetes, scaling it out to 25 instances and then finally rewriting a portion of it in Rust. The whole thing smells of confusion and multiple issues being conflated together, with the rewrite in rust fixing these by virtue of deletion. And if the root is was the GC, then some `--trace-gc` output in the post would be nice at the absolute minimum. Showing a for loop with a string split doesn't cut it.
- maurice2k 3y ago
  Exactly. I'm not a super-expert in Node.js but worked a lot with it and also know a lot of internals. And I can definitely say: IO speed is pretty good. To be honest, I've never come across a language with bad IO speed and I've seen quite a lot of languages. I also did some benchmarks (like orf did) and speed is pretty good. Maybe Rust is a little bit better because it's nearer to syscalls than Node.js is but we're not talking about 760kb/s vs. 250mb/s. 200GB of logs and 25 instances means 8GB per instance. Not sure what they're doing, but processing of 8GB taking 3h is like doing it with my 80286 processor ;-) Even with inserting stuff in MongoDB this seems way to slow. And they only replaced the file IO part with Rust... I guess the records array is getting pretty huge which might result in swapping to disk. But then: don't let it happen, just flush it to the database or whatever. I believe the problem could have been solved with Node.js without issues.
mrighele 3y ago
The point the parent is making is that the boring normal code in NodeJS is not slow, at least not as presented in the article. The bottleneck is probably somewhere else. Allocation is usually not slow in a garbage collected language. Deallocation can be, but that is usually easy to check (for example with the --trace-gc flag, or running with profiling enabled and checking the results with chrome devtools). It would have been nice if they shared such analysis. With the limited information provided in the article, there are other points that I would check before going as far as writing a module in another language. For example > records.push({ They are collecting all the parsed rows in a single array. This is going to consume memory proportional to the size of the file, potentially in the same order of magnitude (depends on how much information is being collected and how much discarded).
ryanjshaw 3y ago
The low throughput is definitely a code smell. In my experience, a common mistake in handling files too large for memory is to read the data faster than you process it, consequently running out of memory. For example, maybe they have a loop reading lines and doing a bunch of async DB writes: your process will read at disk speed and write at database speed (relatively slower), and you'll run out of memory due to async tasks/context queuing up in memory. Perhaps this is why it worked to split the file into 8GB units X 25 servers: it didn't blow their memory budget per server.

bhaak 3y ago

Try it with something a little bigger than what you used. Then you would have seen that the GC kicks in and performance goes down after about 2GB of processed data. With my first try with bigger data it blew up with an out of heap memory error. The article mentions them having memory problems as well. They probably operated at the upper levels of available memory and even got into swapping and then 700kb/s is not surprising. So yes, this absolutely looks like a memory allocation issue.

orf 3y ago
Yes, of course the unbounded array will cause you to run out of memory. That’s… not news, and it absolutely isn’t a “memory allocation issue” in the sense they are using it in the post. Try modifying it to clear the array (a DB flush, if you will) every 100k lines.
- bhaak 3y ago
  That's what you get if you use the (simplified) example code that is in the article. It's also what you can derive from their description of the problem. Now we don't have any Rust code to compare so we can't say how it's different. But everything points towards a memory allocation issue that is just not their in simple Rust code.
- tomhallett 3y ago
  Following you in the abstract, but still a bit confused. * "unbounded array" - are you referring to `records`? * If so, is the memory of `records` too large? Or is it that `records[i].pathname` has a reference to `fields[7]`, AND v8 will retain all copies of `fields` until the loop is done? Do you think this code would be more GC-friendly? ``` let count = 0; const MAX = 1000; const flushedRecords = []; for await (const line of readlineStream) { const fields = line.split('\t'); const httpStatus = Number(fields[5]); if (httpStatus < 200 || httpStatus > 299) continue; records.push({ pathname: fields[7], referrer: fields[8], // ... }); if(count == MAX || isLastLine(line)){ flushedRecords.push([...records]); records = []; count = 0; } else { count++; } } ``` Or were you thinking something totally different?

nicoburns 3y ago

> They just stuck a job queue on it and then rewrote part of it in Rust IMO that's not an inappropriate response here. You can make this code in Node, but it's super-awkward trying to reduce string allocations in JavaScript. Much easier to rewrite in Rust where such things will be fast even with the naive code. For code such as this you can likely do a 1:1 port of the Node code and it will be 10x faster. Which is much better than getting yourself in twists trying to make Node fast at tasks it's not good at.

orf 3y ago
I took the code from their post, the one they highlighted as the issue, string allocations and all, and ran it. It was 1000x as fast as their self-reported runtime. Does this really, really not scream “there is another issue” to you? Do you really think, even with string allocations in a hot loop, NodeJS with its very optimised runtime would struggle to hit 1mb/s?
LudwigNagasena 3y ago
Naive Rust code is super-awkward compared to naive JS code. I doubt one can do 1:1 port of anything more complex than Hello World (even FizzBuzz may get you tripped up).
- __s 3y ago
  Not sure what you mean by naive rust, but rust has good webassembly integration. To give you some context, here's a post I made about rewriting 10k lines of js into rust: https://www.reddit.com/r/rust/comments/k3jy5g/i_rewrote_10k_lines_of_js_into_rust_over_the_last https://www.reddit.com/r/rust/comments/k3jy5g/i_rewrote_10k_... (I went on to rewrite the server from nodejs to rust too)
- nicoburns 3y ago
  YMMV, but my Rust code ends up looking pretty much identical to my TypeScript code most of the time. And especially for code like this where there are no complex lifetimes.
- jplusequalt 3y ago