5 ms·

const readFilePromise = (...args) => new Promise((resolve, reject) => readFile(...args, (err, result) => err ? reject(err) : resolve(result))) How can the nat

const readFilePromise = (...args) => new Promise((resolve, reject) => readFile(...args, (err, result) => err ? reject(err) : resolve(result))) How can the native implementation be much more complex than that, beyond maybe some argument assertions? That will take almost the exact same amount of time as readFile, the additional time cost is a small constant value.

orf 6y ago
On a SSD reading files is fast. However read() is a blocking operation that could take forever - reading some /dev/ device that never returns any data, or a slower medium that takes hundreds of milliseconds per read. That’s a big no-no. So it gets dispatched to a dedicated blocking-io threadpool with limited workers which does the blocking operation without interrupting the event loop. I’m guessing as disk speeds have increased the relative overhead of dispatching the task to a thread pool has also increased. At least, that’s how I think it works based on some half-remembered blog post. Edit: yep - http://docs.libuv.org/en/v1.x/design.html#file-i-o http://docs.libuv.org/en/v1.x/design.html#file-i-o
- beaconstudios 6y ago
  I can fully believe that there's some cleverness going on with node file I/O - but there should be no meaningful difference in performance between returning the result via a Promise, versus a callback.
  - orf 6y ago
    It depends - I would assume the callback pattern is much easier to JIT than the promise pattern and requires less objects to be allocated, but you’re right. That’s a huge difference. You’d need to benchmark another non-io bound function to find how much overhead comes from promises themselves.
    - beaconstudios 6y ago
      Not enough to add 40% overhead to a 1mb file read, I can tell you that much.
      - javajosh 6y ago
        It depends. If disks got so fast that function call overhead starts to dominate, then you might see an effect like this. But I have to imagine reading from an SSD is still 1000x slower (at least) than a function invocation in Javascript.
        beaconstudios 6y ago
        I don't see that basically ever being the case.
  - SavantIdiot 6y ago
    The most recent comment is insightful: "My guess is that the promise version is idling when IO capacity is available due to scheduling? Is it implemented in an event driven way internally, and if it is (for example using epoll) is the epoll event able to interrupt and cause the promise to continue to be evaluated or does it have to wait for the vm to get around to scheduling it again?" It didn't occur to me that the async promise is rescheduled. That right there is a concern. The OP also points out buffer concatenation, which is an odd choice. Reading the file into memory in one go should start with an stat and then a single buffer alloc. But that is probably naive.
- hinkley 6y ago
  Network filesystems, compressed filesystems, and of course the ever-dreaded on-access virus scanners on Windows that 'break' one open source project after another. Who knew that opening the same file 10 times is slow? Everybody. Everybody knew that. Looking at you, svn team.
- TheCoelacanth 6y ago
  But these are both non-blocking APIs. readFileSync is the blocking version.
  - tynorf 6y ago
    I believe orf is talking about the read(2) syscall, which is blocking.
Jarred 6y ago
I agree in principle, but for some reason, the underlying implementation for fs.promises is completely separate. I think this is where fs.promises.readFile reads the actual file https://github.com/nodejs/node/blob/master/lib/internal/fs/promises.js#L319-L339 https://github.com/nodejs/node/blob/master/lib/internal/fs/p..., with many layers of indirection before that fs.readFile is this one function: https://github.com/nodejs/node/blob/master/lib/fs.js#L327-L360 https://github.com/nodejs/node/blob/master/lib/fs.js#L327-L3...
- wcarss 6y ago
  I'm brand new to the code, but I don't think it is as simple as you state. The 'one function' you linked to just passes a ReadFileContext into binding.open[1]. The ReadFileContext appears to have its own read that uses its own allocated Buffers[2] under some circumstances, but under others they are mysteriously already there[3]. Then binding.open refers to an internalBinding of fs[4], which I'm unsure of, is that js or C++ code? This[5] suggests C++ is involved, but there is also an open in lib/fs.js[6] with that signature, though it also just calls binding.open at the end with the same signature, so I think it's off into C++ by then. At that point, I lose the trail. Maybe C++ is even doing the allocating. This is not easy code to grok in its full extent. 1 - https://github.com/nodejs/node/blob/master/lib/fs.js#L356 https://github.com/nodejs/node/blob/master/lib/fs.js#L356 2 - https://github.com/nodejs/node/blob/master/lib/internal/fs/read_file_context.js#L103 https://github.com/nodejs/node/blob/master/lib/internal/fs/r... 3 - https://github.com/nodejs/node/blob/master/lib/internal/fs/read_file_context.js#L108 https://github.com/nodejs/node/blob/master/lib/internal/fs/r... 4 - https://github.com/nodejs/node/blob/master/lib/fs.js#L74 https://github.com/nodejs/node/blob/master/lib/fs.js#L74 5 - https://github.com/nodejs/node/blob/3b2863db12818778671713b5c30363f788912aae/lib/internal/bootstrap/loaders.js#L21 https://github.com/nodejs/node/blob/3b2863db12818778671713b5... 6 - https://github.com/nodejs/node/blob/master/lib/fs.js#L472 https://github.com/nodejs/node/blob/master/lib/fs.js#L472 edit: I was curious so I kept reading, It seems like open is probably C++ here[7], which makes an AsyncCall or SyncCall to "open", which are just wrappers around syscalls[8]. Still not sure about other places the buffer comes from, though. 7 - https://github.com/nodejs/node/blob/master/src/node_file.cc#L1709 https://github.com/nodejs/node/blob/master/src/node_file.cc#... 8 - https://github.com/nodejs/node/blob/606df7c4e79324b9725bfcfe019a8b75bfa04c3f/src/node_file-inl.h#L267 https://github.com/nodejs/node/blob/606df7c4e79324b9725bfcfe...
idbehold 6y ago
I believe the difference here is that these newly introduced promise wrappers allow you to pass an AbortSignal instances to them. Wrapping the old callback style methods would not allow you to "abort" in the middle of reading a file. You could respond to a passed in AbortSignal instance in the simple `readFilePromise` function you provided by simply rejecting early, but it wouldn't free up the underlying file descriptor until after the entire file had been read.
- idbehold 6y ago
  Actually I was completely wrong. They added support for consuming an AbortSignal in the old callback style methods as well. Now I'm at a loss as to why they even have two different implementations instead of just doing something like: fs.promises.readFile = util.promisify(fs.readFile);

orf 6y ago

On a SSD reading files is fast. However read() is a blocking operation that could take forever - reading some /dev/ device that never returns any data, or a slower medium that takes hundreds of milliseconds per read. That’s a big no-no. So it gets dispatched to a dedicated blocking-io threadpool with limited workers which does the blocking operation without interrupting the event loop. I’m guessing as disk speeds have increased the relative overhead of dispatching the task to a thread pool has also increased. At least, that’s how I think it works based on some half-remembered blog post. Edit: yep - http://docs.libuv.org/en/v1.x/design.html#file-i-o http://docs.libuv.org/en/v1.x/design.html#file-i-o

beaconstudios 6y ago
I can fully believe that there's some cleverness going on with node file I/O - but there should be no meaningful difference in performance between returning the result via a Promise, versus a callback.
- orf 6y ago
  It depends - I would assume the callback pattern is much easier to JIT than the promise pattern and requires less objects to be allocated, but you’re right. That’s a huge difference. You’d need to benchmark another non-io bound function to find how much overhead comes from promises themselves.
  - beaconstudios 6y ago
    Not enough to add 40% overhead to a 1mb file read, I can tell you that much.
    - javajosh 6y ago
      It depends. If disks got so fast that function call overhead starts to dominate, then you might see an effect like this. But I have to imagine reading from an SSD is still 1000x slower (at least) than a function invocation in Javascript.
      - beaconstudios 6y ago
        I don't see that basically ever being the case.
- SavantIdiot 6y ago
  The most recent comment is insightful: "My guess is that the promise version is idling when IO capacity is available due to scheduling? Is it implemented in an event driven way internally, and if it is (for example using epoll) is the epoll event able to interrupt and cause the promise to continue to be evaluated or does it have to wait for the vm to get around to scheduling it again?" It didn't occur to me that the async promise is rescheduled. That right there is a concern. The OP also points out buffer concatenation, which is an odd choice. Reading the file into memory in one go should start with an stat and then a single buffer alloc. But that is probably naive.
hinkley 6y ago
Network filesystems, compressed filesystems, and of course the ever-dreaded on-access virus scanners on Windows that 'break' one open source project after another. Who knew that opening the same file 10 times is slow? Everybody. Everybody knew that. Looking at you, svn team.
TheCoelacanth 6y ago
But these are both non-blocking APIs. readFileSync is the blocking version.
- tynorf 6y ago
  I believe orf is talking about the read(2) syscall, which is blocking.

Jarred 6y ago

I agree in principle, but for some reason, the underlying implementation for fs.promises is completely separate. I think this is where fs.promises.readFile reads the actual file https://github.com/nodejs/node/blob/master/lib/internal/fs/promises.js#L319-L339 https://github.com/nodejs/node/blob/master/lib/internal/fs/p..., with many layers of indirection before that fs.readFile is this one function: https://github.com/nodejs/node/blob/master/lib/fs.js#L327-L360 https://github.com/nodejs/node/blob/master/lib/fs.js#L327-L3...

wcarss 6y ago
I'm brand new to the code, but I don't think it is as simple as you state. The 'one function' you linked to just passes a ReadFileContext into binding.open[1]. The ReadFileContext appears to have its own read that uses its own allocated Buffers[2] under some circumstances, but under others they are mysteriously already there[3]. Then binding.open refers to an internalBinding of fs[4], which I'm unsure of, is that js or C++ code? This[5] suggests C++ is involved, but there is also an open in lib/fs.js[6] with that signature, though it also just calls binding.open at the end with the same signature, so I think it's off into C++ by then. At that point, I lose the trail. Maybe C++ is even doing the allocating. This is not easy code to grok in its full extent. 1 - https://github.com/nodejs/node/blob/master/lib/fs.js#L356 https://github.com/nodejs/node/blob/master/lib/fs.js#L356 2 - https://github.com/nodejs/node/blob/master/lib/internal/fs/read_file_context.js#L103 https://github.com/nodejs/node/blob/master/lib/internal/fs/r... 3 - https://github.com/nodejs/node/blob/master/lib/internal/fs/read_file_context.js#L108 https://github.com/nodejs/node/blob/master/lib/internal/fs/r... 4 - https://github.com/nodejs/node/blob/master/lib/fs.js#L74 https://github.com/nodejs/node/blob/master/lib/fs.js#L74 5 - https://github.com/nodejs/node/blob/3b2863db12818778671713b5c30363f788912aae/lib/internal/bootstrap/loaders.js#L21 https://github.com/nodejs/node/blob/3b2863db12818778671713b5... 6 - https://github.com/nodejs/node/blob/master/lib/fs.js#L472 https://github.com/nodejs/node/blob/master/lib/fs.js#L472 edit: I was curious so I kept reading, It seems like open is probably C++ here[7], which makes an AsyncCall or SyncCall to "open", which are just wrappers around syscalls[8]. Still not sure about other places the buffer comes from, though. 7 - https://github.com/nodejs/node/blob/master/src/node_file.cc#L1709 https://github.com/nodejs/node/blob/master/src/node_file.cc#... 8 - https://github.com/nodejs/node/blob/606df7c4e79324b9725bfcfe019a8b75bfa04c3f/src/node_file-inl.h#L267 https://github.com/nodejs/node/blob/606df7c4e79324b9725bfcfe...

idbehold 6y ago

I believe the difference here is that these newly introduced promise wrappers allow you to pass an AbortSignal instances to them. Wrapping the old callback style methods would not allow you to "abort" in the middle of reading a file. You could respond to a passed in AbortSignal instance in the simple `readFilePromise` function you provided by simply rejecting early, but it wouldn't free up the underlying file descriptor until after the entire file had been read.

idbehold 6y ago
Actually I was completely wrong. They added support for consuming an AbortSignal in the old callback style methods as well. Now I'm at a loss as to why they even have two different implementations instead of just doing something like: fs.promises.readFile = util.promisify(fs.readFile);