Support context passing in low-level async APIs

[petkaantonov]

The current low-level async APIs have performance-hostile design as each level that passes a callback requires the allocation of a closre (which is in this case the allocation of 2 objects: JSFunction object and a Context object).

For example:

fs.doSomething = function(arg, callback) {
  var req = new FSReqWrap();
  req.oncomplete = callback;
  binding.doSomething(arg, req);
};

110% of the time callback has some context it needs around once it's called, so it can never be a static function that is only allocated once per program. Also any higher level fs abstraction is going to allocate a closure for its own context and so on. These closures are relatively expensive objects, more expensive than e.g. a Promise object.

With context passing:

fs.doSomething = function(arg, callback) {
  var req = new FSReqWrap();
  req.oncomplete = callback;
  // When the request completes, it would call req.oncomplete.call(req.context, err, result);
  req.context = ... 
  binding.doSomething(arg, req);
}

This would enable extremely performant promise-wrappers (given user-land promises):

fs.doSomethingPromise = function(arg) {
  var promise = new Promise(INTERNAL);
  var req = new FSReqWrap();
  req.oncomplete = promise._resolveFromNodeBack;
  req.context = promise;
  binding.doSomething(arg, req);
  return promise;
};

The above code literally allocates less objects than a fs.doSomething(1, function closure(){}).

But it would also enable high level fs functions that are build on top of low level fs functions to allocate less closures.

Another example of context-passing include most of the ES5 array methods.

[benjamingr]

The points raised here are good and interesting - I'm wondering if you can make a benchmark to back them up?

Also, to be clear - the suggestion here is to allow low level methods to take a this parameter right?

[petkaantonov]

What exactly do you want me to benchmark? That allocating a closure is slower than not allocating it?

[benjamingr]

@petkaantonov code using a node API (for example the filesystem API) performing better (in a real use case) with context passed than without it.

[vkurchatkin]

As far as I understand you are proposing that instead of

var whatever  = ....

fs.someFunction(function(err, res) {
   console.log(whatever);
});

people should do

var whatever  = ....

fs.someFunction(function(err, res, context) {
   var whatever = context;
   console.log(whatever);
}, whatever);

[petkaantonov]

Not in public facing API... let's take a look at readFile for instance.. it allocates 7 closures when it could simply allocate 1 FileReadingContext and pass that around. You really misunderstood me, this is an internal optimization at lowest level, not some api change.

[vkurchatkin]

Well, adding context support to FSReqWrap is simple. But all functions used inside readFile are public, so we basically expose this API to users as well

[petkaantonov]

Well you don't need to rewrite anything, supporting it in FSReqWrap (and equivalent in other modules) is already enough for me. I am just saying that iojs itself could get also a perf win from this for many methods if it wants. You don't need to touch readFile at all. And if you were to refactor readFile you would obviously call internal methods that accept context if you didn't want to expose context passing to users.

Here's an example how readFile could be implemented were this supported by FSReqWrap: https://gist.github.com/petkaantonov/0b23d1c18404baf65dec , it reduces object allocations in successful case from 9 (6 JSFunctions+3 Contexts) to 1 (JSObject).

[mscdex]

👍

[vkurchatkin]

Ok, it seems like we already have everything to do this:

var binding = process.binding('fs');
var open = binding.open;
var FSReqWrap = binding.FSReqWrap;
var constants = process.binding('constants');


var req = new FSReqWrap();
req.oncomplete = oncomplete;
req.context = 'whatever';

open('test.js', constants.O_RDONLY, 0o666, req);

function oncomplete(err, fd) {

  if (err)
    throw err;

  console.log(fd);
  console.log(this.context)

}

[petkaantonov]

Yea that works for me, thanks

[vkurchatkin]

I tried this approach out and can't see significant changes in benchmarks

before

fs/readfile.js dur=5 len=1024 concurrent=1: 22961.59914
fs/readfile.js dur=5 len=1024 concurrent=10: 28115.44754
fs/readfile.js dur=5 len=16777216 concurrent=1: 160.76778
fs/readfile.js dur=5 len=16777216 concurrent=10: 325.55294

after

fs/readfile.js dur=5 len=1024 concurrent=1: 23862.05061
fs/readfile.js dur=5 len=1024 concurrent=10: 27368.07561
fs/readfile.js dur=5 len=16777216 concurrent=1: 160.06920
fs/readfile.js dur=5 len=16777216 concurrent=10: 326.91924

[petkaantonov]

Those benchmarks measure file reading speed which is not affected, to measure the call speed you probably want to read a ton of unique short files like 16 bytes long or such. E.g. create different 1000 files of 16 bytes length and then call readFile on them in parallel...

[vkurchatkin]

nope, same thing

[petkaantonov]

@vkurchatkin I'll construct a relevant benchmark on the weekend when I have more time for this