Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promisify async wrapper #688

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions benchmark/fs/lstat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Call fs.stat over and over again really fast.
// Then see how many times it got called.
// Yes, this is a silly benchmark. Most benchmarks are silly.

var path = require('path');
var common = require('../common.js');
var fs = require('fs');

var FILES = [
require.resolve('../../lib/assert.js'),
require.resolve('../../lib/console.js'),
require.resolve('../../lib/fs.js')
];

var VARIANTS = {
promise: createPromiseBasedTest(fs.lstat),
callback: createCallBackBasedTest(fs.lstat),
};

var bench = common.createBenchmark(main, {
dur: [5],
concurrent: [/*1, 10,*/ 100],
variant: Object.keys(VARIANTS)
});

function main(conf) {
var stat = VARIANTS[conf.variant];

if (stat == VARIANTS.promise && !fs.lstat('.')) {
bench.start();
bench.end(0);
return;
}

var calls = 0;
bench.start();
setTimeout(function() {
bench.end(calls);
}, +conf.dur * 1000);

var cur = +conf.concurrent;
while (cur--) run();

function run() {
var p = stat(next);
if (p) p.then(next);
}

function next() {
calls++;
run();
}
}

function createCallBackBasedTest(stat) {
return function runStatViaCallbacks(cb) {
stat(FILES[0], function(err, data) {
if (err) throw err;
second(data);
});

function second(data) {
stat(FILES[1], function(err, data) {
if (err) throw err;
third(data);
});
}

function third(data) {
stat(FILES[2], function(err, data) {
if (err) throw err;
cb(data);
});
}
};
}

function createPromiseBasedTest(stat) {
return function runStatViaPromises() {
return stat(FILES[0])
.then(function secondP(data) {
return stat(FILES[1]);
})
.then(function thirdP(data) {
return stat(FILES[2]);
});
}
}
88 changes: 88 additions & 0 deletions benchmark/fs/stat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Call fs.stat over and over again really fast.
// Then see how many times it got called.
// Yes, this is a silly benchmark. Most benchmarks are silly.

var path = require('path');
var common = require('../common.js');
var fs = require('fs');

var FILES = [
require.resolve('../../lib/assert.js'),
require.resolve('../../lib/console.js'),
require.resolve('../../lib/fs.js')
];

var VARIANTS = {
promise: createPromiseBasedTest(fs.stat),
callback: createCallBackBasedTest(fs.stat),
};

var bench = common.createBenchmark(main, {
dur: [5],
concurrent: [/*1, 10,*/ 100],
variant: Object.keys(VARIANTS)
});

function main(conf) {
var stat = VARIANTS[conf.variant];

if (stat == VARIANTS.promise && !fs.stat('.')) {
bench.start();
bench.end(0);
return;
}

var calls = 0;
bench.start();
setTimeout(function() {
bench.end(calls);
}, +conf.dur * 1000);

var cur = +conf.concurrent;
while (cur--) run();

function run() {
var p = stat(next);
if (p) p.then(next);
}

function next() {
calls++;
run();
}
}

function createCallBackBasedTest(stat) {
return function runStatViaCallbacks(cb) {
stat(FILES[0], function(err, data) {
if (err) throw err;
second(data);
});

function second(data) {
stat(FILES[1], function(err, data) {
if (err) throw err;
third(data);
});
}

function third(data) {
stat(FILES[2], function(err, data) {
if (err) throw err;
cb(data);
});
}
};
}

function createPromiseBasedTest(stat) {
return function runStatViaPromises() {
return stat(FILES[0])
.then(function secondP(data) {
return stat(FILES[1]);
})
.then(function thirdP(data) {
return stat(FILES[2]);
});
}
}
37 changes: 30 additions & 7 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,12 @@ function nullCheck(path, callback) {
var er = new Error('Path must be a string without null bytes.');
if (!callback)
throw er;
process.nextTick(function() {
callback(er);
});
if (callback.reject)
callback.reject(er);
else
process.nextTick(function() {
callback(er);
});
return false;
}
return true;
Expand Down Expand Up @@ -780,12 +783,32 @@ fs.lstat = function(path, callback) {
binding.lstat(pathModule._makeLong(path), req);
};

fs.stat = function(path, callback) {
callback = makeCallback(callback);
if (!nullCheck(path, callback)) return;
function makeOnComplete(cb) {
return function oncomplete() {
return cb.apply(null, arguments);
};
}

function Defer(callback) {
var req = new FSReqWrap();
req.oncomplete = callback;
if (util.isFunction(callback)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe switch to typeof callback === 'function'?

req.oncomplete = makeOnComplete(callback);
// Below causes stat/test to reduce from >58K to < 50K, why?
// req.oncomplete = function() { return callback.apply(null, arguments); };
return req;
}
req.promise = new Promise(function(resolve, reject) {
req.resolve = resolve;
req.reject = reject;
});
return req;
}

fs.stat = function(path, callback) {
var req = Defer(callback);
if (!nullCheck(path, req)) return req.promise;
binding.stat(pathModule._makeLong(path), req);
return req.promise;
};

fs.fstatSync = function(fd) {
Expand Down
2 changes: 2 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,10 @@ namespace node {
V(received_shutdown_string, "receivedShutdown") \
V(refresh_string, "refresh") \
V(regexp_string, "regexp") \
V(reject_string, "reject") \
V(rename_string, "rename") \
V(replacement_string, "replacement") \
V(resolve_string, "resolve") \
V(retry_string, "retry") \
V(rss_string, "rss") \
V(serial_string, "serial") \
Expand Down
2 changes: 1 addition & 1 deletion src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ static void After(uv_fs_t *req) {
}
}

req_wrap->MakeCallback(env->oncomplete_string(), argc, argv);
req_wrap->Fulfill(argc, argv);

uv_fs_req_cleanup(&req_wrap->req_);
delete req_wrap;
Expand Down
18 changes: 18 additions & 0 deletions src/req_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,24 @@ class ReqWrap : public AsyncWrap {
persistent().Reset();
}

inline v8::Handle<v8::Value> Fulfill(int argc, v8::Handle<v8::Value>* argv) {
// if (obj.oncomplete) .oncomplete(argv)
v8::Local<v8::Value> cb_v = object()->Get(env()->oncomplete_string());
if (cb_v->IsFunction()) {
return MakeCallback(cb_v.As<v8::Function>(), argc, argv);
}
// else
// if (argv[0]) obj.reject(argv[0])
// else obj.resolve(argv[1...])
if (argc == 0)
return MakeCallback(env()->resolve_string(), 0, NULL);

if (argv[0]->ToBoolean()->IsTrue()) // if(err) {
return MakeCallback(env()->reject_string(), 1, argv);

return MakeCallback(env()->resolve_string(), argc-1, argv+1);
}

// Call this after the req has been dispatched.
void Dispatched() {
req_.data = this;
Expand Down