From 05032329f759bc412c8cfab3b2edf69144e62e51 Mon Sep 17 00:00:00 2001 From: Albert Siddhartha Slawinski Date: Wed, 13 Aug 2014 10:43:03 -0700 Subject: [PATCH] fs.stat third parameter controlling error creation In some environment (especially when coffescript is involved) module.js will spend a lot of time looking up nonexisting files. This is caused by the lengthy error creation. Those errors are then immediately caught. This change makes the lookup much faster, by avoiding the unnecessary error creation. module::statPath now uses the new second parameter of fs.statSync function of the fs.js. There is a similar change for the fs.stat, adding the third parameter. Apply suggestions from libuv pull request Thanks saghul: https://github.com/joyent/libuv/pull/1428 Fix the datastructures for async Conflicts: lib/module.js src/node_file.cc src/node_file.h --- lib/fs.js | 63 +++++++++++------- lib/module.js | 7 +- src/node_file.cc | 168 +++++++++++++++++++++++++++-------------------- src/node_file.h | 6 ++ 4 files changed, 143 insertions(+), 101 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 3301a6af8474..a5149d155388 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -100,14 +100,16 @@ function assertEncoding(encoding) { } } -function nullCheck(path, callback) { +function nullCheck(path, callback, throw_safe) { if (('' + path).indexOf('\u0000') !== -1) { - var er = new Error('Path must be a string without null bytes.'); - if (!callback) - throw er; - process.nextTick(function() { - callback(er); - }); + if (!throw_safe) { + var er = new Error('Path must be a string without null bytes.'); + if (!callback) + throw er; + process.nextTick(function() { + callback(er); + }); + } return false; } return true; @@ -181,21 +183,25 @@ fs.Stats.prototype.isSocket = function() { }; fs.exists = function(path, callback) { - if (!nullCheck(path, cb)) return; - binding.stat(pathModule._makeLong(path), cb); - function cb(err, stats) { - if (callback) callback(err ? false : true); + if (!nullCheck(path, undefined, true)) { + cb(true, false); + return; + } + binding.stat(pathModule._makeLong(path), cb, true); + function cb(err, result) { + if (callback) { + if (err) { + callback(false); + } else { + callback(!!result); + } + } } }; fs.existsSync = function(path) { - try { - nullCheck(path); - binding.stat(pathModule._makeLong(path)); - return true; - } catch (e) { - return false; - } + return (nullCheck(path, undefined, true)) && + (!!binding.stat(pathModule._makeLong(path), undefined, true)); }; fs.readFile = function(path, options, callback_) { @@ -701,10 +707,17 @@ fs.lstat = function(path, callback) { binding.lstat(pathModule._makeLong(path), callback); }; -fs.stat = function(path, callback) { +fs.stat = function(path, callback, throw_safe) { callback = makeCallback(callback); - if (!nullCheck(path, callback)) return; - binding.stat(pathModule._makeLong(path), callback); + if (!nullCheck(path, callback, throw_safe)) { + if (throw_safe) { + process.nextTick(function() { + callback(true, false); + }); + } + return; + } + binding.stat(pathModule._makeLong(path), callback, throw_safe); }; fs.fstatSync = function(fd) { @@ -716,9 +729,11 @@ fs.lstatSync = function(path) { return binding.lstat(pathModule._makeLong(path)); }; -fs.statSync = function(path) { - nullCheck(path); - return binding.stat(pathModule._makeLong(path)); +fs.statSync = function(path, throw_safe) { + if (!nullCheck(path, undefined, throw_safe)) { + return false; + } + return binding.stat(pathModule._makeLong(path), undefined, throw_safe); }; fs.readlink = function(path, callback) { diff --git a/lib/module.js b/lib/module.js index 564f6c49d6cb..ff41a68231f3 100644 --- a/lib/module.js +++ b/lib/module.js @@ -82,10 +82,9 @@ var debug = Module._debug; // -> a/index. function statPath(path) { - try { - return fs.statSync(path); - } catch (ex) {} - return false; + var fs = NativeModule.require('fs'); + + return fs.statSync(path, true); } // check if the directory is a package.json dir diff --git a/src/node_file.cc b/src/node_file.cc index d62fbe2ab9d9..9c9c1917fdca 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -66,13 +66,13 @@ using v8::Value; #define THROW_BAD_ARGS TYPE_ERROR("Bad argument") -class FSReqWrap: public ReqWrap { +class FSReqWrap: public ReqWrap { public: void* operator new(size_t size) { return new char[size]; } void* operator new(size_t size, char* storage) { return storage; } FSReqWrap(Environment* env, const char* syscall, char* data = NULL) - : ReqWrap(env, Object::New(env->isolate())), + : ReqWrap(env, Object::New(env->isolate())), syscall_(syscall), data_(data), dest_len_(0) { @@ -116,13 +116,15 @@ static inline bool IsInt64(double x) { static void After(uv_fs_t *req) { FSReqWrap* req_wrap = static_cast(req->data); - assert(&req_wrap->req_ == req); + assert(&req_wrap->req_.req == req); req_wrap->ReleaseEarly(); // Free memory that's no longer used now. Environment* env = req_wrap->env(); HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); + node_fs_t* req_node = (node_fs_t*) req; + // there is always at least one argument. "error" int argc = 1; @@ -131,8 +133,12 @@ static void After(uv_fs_t *req) { Local argv[2]; if (req->result < 0) { - // If the request doesn't have a path parameter set. - if (req->path == NULL) { + if (req_node->throw_safe == 1) { + argc = 2; + argv[0] = True(env->isolate()); + argv[1] = False(env->isolate()); + } else if (req->path == NULL) { + // If the request doesn't have a path parameter set. argv[0] = UVException(req->result, NULL, req_wrap->syscall()); } else if ((req->result == UV_EEXIST || req->result == UV_ENOTEMPTY || @@ -235,7 +241,7 @@ static void After(uv_fs_t *req) { req_wrap->MakeCallback(env->oncomplete_string(), argc, argv); - uv_fs_req_cleanup(&req_wrap->req_); + uv_fs_req_cleanup(&req_wrap->req_.req); delete req_wrap; } @@ -243,15 +249,15 @@ static void After(uv_fs_t *req) { // For async calls FSReqWrap is used. struct fs_req_wrap { fs_req_wrap() {} - ~fs_req_wrap() { uv_fs_req_cleanup(&req); } + ~fs_req_wrap() { uv_fs_req_cleanup(&req.req); } // Ensure that copy ctor and assignment operator are not used. fs_req_wrap(const fs_req_wrap& req); fs_req_wrap& operator=(const fs_req_wrap& req); - uv_fs_t req; + node_fs_t req; }; -#define ASYNC_DEST_CALL(func, callback, dest_path, ...) \ +#define ASYNC_DEST_CALL(func, callback, dest_path, throw_safe_val, ...) \ Environment* env = Environment::GetCurrent(args.GetIsolate()); \ FSReqWrap* req_wrap; \ char* dest_str = (dest_path); \ @@ -264,31 +270,36 @@ struct fs_req_wrap { dest_str, \ dest_len + 1); \ } \ + req_wrap->req_.throw_safe = throw_safe_val; \ int err = uv_fs_ ## func(env->event_loop() , \ - &req_wrap->req_, \ + &req_wrap->req_.req, \ __VA_ARGS__, \ After); \ req_wrap->object()->Set(env->oncomplete_string(), callback); \ req_wrap->Dispatched(); \ if (err < 0) { \ - uv_fs_t* req = &req_wrap->req_; \ + uv_fs_t* req = &req_wrap->req_.req; \ req->result = err; \ req->path = NULL; \ After(req); \ } \ args.GetReturnValue().Set(req_wrap->persistent()); -#define ASYNC_CALL(func, callback, ...) \ - ASYNC_DEST_CALL(func, callback, NULL, __VA_ARGS__) \ +#define ASYNC_CALL(func, callback, throw_safe_val, ...) \ + ASYNC_DEST_CALL(func, callback, NULL, throw_safe_val, __VA_ARGS__) \ -#define SYNC_DEST_CALL(func, path, dest, ...) \ +#define SYNC_DEST_CALL(func, path, dest, throw_safe_val, ...) \ fs_req_wrap req_wrap; \ + req_wrap.req.throw_safe = throw_safe_val; \ int err = uv_fs_ ## func(env->event_loop(), \ - &req_wrap.req, \ + &req_wrap.req.req, \ __VA_ARGS__, \ NULL); \ if (err < 0) { \ - if (dest != NULL && \ + if (req_wrap.req.throw_safe == 1) { \ + args.GetReturnValue().Set(False(env->isolate())); \ + return; \ + } else if (dest != NULL && \ (err == UV_EEXIST || \ err == UV_ENOTEMPTY || \ err == UV_EPERM)) { \ @@ -298,10 +309,10 @@ struct fs_req_wrap { } \ } \ -#define SYNC_CALL(func, path, ...) \ - SYNC_DEST_CALL(func, path, NULL, __VA_ARGS__) \ +#define SYNC_CALL(func, path, throw_safe_val, ...) \ + SYNC_DEST_CALL(func, path, NULL, throw_safe_val, __VA_ARGS__) \ -#define SYNC_REQ req_wrap.req +#define SYNC_REQ req_wrap.req.req #define SYNC_RESULT err @@ -317,9 +328,9 @@ static void Close(const FunctionCallbackInfo& args) { int fd = args[0]->Int32Value(); if (args[1]->IsFunction()) { - ASYNC_CALL(close, args[1], fd) + ASYNC_CALL(close, args[1], 0, fd) } else { - SYNC_CALL(close, 0, fd) + SYNC_CALL(close, 0, 0, fd) } } @@ -432,12 +443,22 @@ static void Stat(const FunctionCallbackInfo& args) { node::Utf8Value path(args[0]); - if (args[1]->IsFunction()) { - ASYNC_CALL(stat, args[1], *path) + if (args[2]->IsTrue()) { + if (args[1]->IsFunction()) { + ASYNC_CALL(stat, args[1], 1, *path) + } else { + SYNC_CALL(stat, *path, 1, *path) + args.GetReturnValue().Set( + BuildStatsObject(env, static_cast(SYNC_REQ.ptr))); + } } else { - SYNC_CALL(stat, *path, *path) - args.GetReturnValue().Set( - BuildStatsObject(env, static_cast(SYNC_REQ.ptr))); + if (args[1]->IsFunction()) { + ASYNC_CALL(stat, args[1], 0, *path) + } else { + SYNC_CALL(stat, *path, 0, *path) + args.GetReturnValue().Set( + BuildStatsObject(env, static_cast(SYNC_REQ.ptr))); + } } } @@ -453,9 +474,9 @@ static void LStat(const FunctionCallbackInfo& args) { node::Utf8Value path(args[0]); if (args[1]->IsFunction()) { - ASYNC_CALL(lstat, args[1], *path) + ASYNC_CALL(lstat, args[1], 0, *path) } else { - SYNC_CALL(lstat, *path, *path) + SYNC_CALL(lstat, *path, 0, *path) args.GetReturnValue().Set( BuildStatsObject(env, static_cast(SYNC_REQ.ptr))); } @@ -472,9 +493,9 @@ static void FStat(const FunctionCallbackInfo& args) { int fd = args[0]->Int32Value(); if (args[1]->IsFunction()) { - ASYNC_CALL(fstat, args[1], fd) + ASYNC_CALL(fstat, args[1], 0, fd) } else { - SYNC_CALL(fstat, 0, fd) + SYNC_CALL(fstat, 0, 0, fd) args.GetReturnValue().Set( BuildStatsObject(env, static_cast(SYNC_REQ.ptr))); } @@ -510,9 +531,9 @@ static void Symlink(const FunctionCallbackInfo& args) { } if (args[3]->IsFunction()) { - ASYNC_DEST_CALL(symlink, args[3], *dest, *dest, *path, flags) + ASYNC_DEST_CALL(symlink, args[3], *dest, 0, *dest, *path, flags) } else { - SYNC_DEST_CALL(symlink, *path, *dest, *dest, *path, flags) + SYNC_DEST_CALL(symlink, *path, *dest, 0, *dest, *path, flags) } } @@ -534,9 +555,9 @@ static void Link(const FunctionCallbackInfo& args) { node::Utf8Value new_path(args[1]); if (args[2]->IsFunction()) { - ASYNC_DEST_CALL(link, args[2], *new_path, *orig_path, *new_path) + ASYNC_DEST_CALL(link, args[2], *new_path, 0, *orig_path, *new_path) } else { - SYNC_DEST_CALL(link, *orig_path, *new_path, *orig_path, *new_path) + SYNC_DEST_CALL(link, *orig_path, *new_path, 0, *orig_path, *new_path) } } @@ -552,9 +573,9 @@ static void ReadLink(const FunctionCallbackInfo& args) { node::Utf8Value path(args[0]); if (args[1]->IsFunction()) { - ASYNC_CALL(readlink, args[1], *path) + ASYNC_CALL(readlink, args[1], 0, *path) } else { - SYNC_CALL(readlink, *path, *path) + SYNC_CALL(readlink, *path, 0, *path) const char* link_path = static_cast(SYNC_REQ.ptr); Local rc = String::NewFromUtf8(env->isolate(), link_path); args.GetReturnValue().Set(rc); @@ -579,9 +600,9 @@ static void Rename(const FunctionCallbackInfo& args) { node::Utf8Value new_path(args[1]); if (args[2]->IsFunction()) { - ASYNC_DEST_CALL(rename, args[2], *new_path, *old_path, *new_path) + ASYNC_DEST_CALL(rename, args[2], *new_path, 0, *old_path, *new_path) } else { - SYNC_DEST_CALL(rename, *old_path, *new_path, *old_path, *new_path) + SYNC_DEST_CALL(rename, *old_path, *new_path, 0, *old_path, *new_path) } } @@ -599,9 +620,9 @@ static void FTruncate(const FunctionCallbackInfo& args) { int64_t len = GET_TRUNCATE_LENGTH(args[1]); if (args[2]->IsFunction()) { - ASYNC_CALL(ftruncate, args[2], fd, len) + ASYNC_CALL(ftruncate, args[2], 0, fd, len) } else { - SYNC_CALL(ftruncate, 0, fd, len) + SYNC_CALL(ftruncate, 0, 0, fd, len) } } @@ -616,9 +637,9 @@ static void Fdatasync(const FunctionCallbackInfo& args) { int fd = args[0]->Int32Value(); if (args[1]->IsFunction()) { - ASYNC_CALL(fdatasync, args[1], fd) + ASYNC_CALL(fdatasync, args[1], 0, fd) } else { - SYNC_CALL(fdatasync, 0, fd) + SYNC_CALL(fdatasync, 0, 0, fd) } } @@ -633,9 +654,9 @@ static void Fsync(const FunctionCallbackInfo& args) { int fd = args[0]->Int32Value(); if (args[1]->IsFunction()) { - ASYNC_CALL(fsync, args[1], fd) + ASYNC_CALL(fsync, args[1], 0, fd) } else { - SYNC_CALL(fsync, 0, fd) + SYNC_CALL(fsync, 0, 0, fd) } } @@ -651,9 +672,9 @@ static void Unlink(const FunctionCallbackInfo& args) { node::Utf8Value path(args[0]); if (args[1]->IsFunction()) { - ASYNC_CALL(unlink, args[1], *path) + ASYNC_CALL(unlink, args[1], 0, *path) } else { - SYNC_CALL(unlink, *path, *path) + SYNC_CALL(unlink, *path, 0, *path) } } @@ -669,9 +690,9 @@ static void RMDir(const FunctionCallbackInfo& args) { node::Utf8Value path(args[0]); if (args[1]->IsFunction()) { - ASYNC_CALL(rmdir, args[1], *path) + ASYNC_CALL(rmdir, args[1], 0, *path) } else { - SYNC_CALL(rmdir, *path, *path) + SYNC_CALL(rmdir, *path, 0, *path) } } @@ -687,9 +708,9 @@ static void MKDir(const FunctionCallbackInfo& args) { int mode = static_cast(args[1]->Int32Value()); if (args[2]->IsFunction()) { - ASYNC_CALL(mkdir, args[2], *path, mode) + ASYNC_CALL(mkdir, args[2], 0, *path, mode) } else { - SYNC_CALL(mkdir, *path, *path, mode) + SYNC_CALL(mkdir, *path, 0, *path, mode) } } @@ -705,9 +726,9 @@ static void ReadDir(const FunctionCallbackInfo& args) { node::Utf8Value path(args[0]); if (args[1]->IsFunction()) { - ASYNC_CALL(readdir, args[1], *path, 0 /*flags*/) + ASYNC_CALL(readdir, args[1], 0, *path, 0 /*flags*/) } else { - SYNC_CALL(readdir, *path, *path, 0 /*flags*/) + SYNC_CALL(readdir, *path, 0, *path, 0 /*flags*/) assert(SYNC_REQ.result >= 0); char* namebuf = static_cast(SYNC_REQ.ptr); @@ -752,9 +773,9 @@ static void Open(const FunctionCallbackInfo& args) { int mode = static_cast(args[2]->Int32Value()); if (args[3]->IsFunction()) { - ASYNC_CALL(open, args[3], *path, flags, mode) + ASYNC_CALL(open, args[3], 0, *path, flags, mode) } else { - SYNC_CALL(open, *path, *path, flags, mode) + SYNC_CALL(open, *path, 0, *path, flags, mode) args.GetReturnValue().Set(SYNC_RESULT); } } @@ -799,11 +820,11 @@ static void WriteBuffer(const FunctionCallbackInfo& args) { uv_buf_t uvbuf = uv_buf_init(const_cast(buf), len); if (cb->IsFunction()) { - ASYNC_CALL(write, cb, fd, &uvbuf, 1, pos) + ASYNC_CALL(write, cb, 0, fd, &uvbuf, 1, pos) return; } - SYNC_CALL(write, NULL, fd, &uvbuf, 1, pos) + SYNC_CALL(write, NULL, 0, fd, &uvbuf, 1, pos) args.GetReturnValue().Set(SYNC_RESULT); } @@ -850,15 +871,16 @@ static void WriteString(const FunctionCallbackInfo& args) { uv_buf_t uvbuf = uv_buf_init(const_cast(buf), len); if (!cb->IsFunction()) { - SYNC_CALL(write, NULL, fd, &uvbuf, 1, pos) + SYNC_CALL(write, NULL, 0, fd, &uvbuf, 1, pos) if (must_free) delete[] buf; return args.GetReturnValue().Set(SYNC_RESULT); } FSReqWrap* req_wrap = new FSReqWrap(env, "write", must_free ? buf : NULL); + req_wrap->req_.throw_safe = 0; int err = uv_fs_write(env->event_loop(), - &req_wrap->req_, + &req_wrap->req_.req, fd, &uvbuf, 1, @@ -867,7 +889,7 @@ static void WriteString(const FunctionCallbackInfo& args) { req_wrap->object()->Set(env->oncomplete_string(), cb); req_wrap->Dispatched(); if (err < 0) { - uv_fs_t* req = &req_wrap->req_; + uv_fs_t* req = &req_wrap->req_.req; req->result = err; req->path = NULL; After(req); @@ -932,9 +954,9 @@ static void Read(const FunctionCallbackInfo& args) { cb = args[5]; if (cb->IsFunction()) { - ASYNC_CALL(read, cb, fd, &uvbuf, 1, pos); + ASYNC_CALL(read, cb, 0, fd, &uvbuf, 1, pos); } else { - SYNC_CALL(read, 0, fd, &uvbuf, 1, pos) + SYNC_CALL(read, 0, 0, fd, &uvbuf, 1, pos) args.GetReturnValue().Set(SYNC_RESULT); } } @@ -954,9 +976,9 @@ static void Chmod(const FunctionCallbackInfo& args) { int mode = static_cast(args[1]->Int32Value()); if (args[2]->IsFunction()) { - ASYNC_CALL(chmod, args[2], *path, mode); + ASYNC_CALL(chmod, args[2], 0, *path, mode); } else { - SYNC_CALL(chmod, *path, *path, mode); + SYNC_CALL(chmod, *path, 0, *path, mode); } } @@ -975,9 +997,9 @@ static void FChmod(const FunctionCallbackInfo& args) { int mode = static_cast(args[1]->Int32Value()); if (args[2]->IsFunction()) { - ASYNC_CALL(fchmod, args[2], fd, mode); + ASYNC_CALL(fchmod, args[2], 0, fd, mode); } else { - SYNC_CALL(fchmod, 0, fd, mode); + SYNC_CALL(fchmod, 0, 0, fd, mode); } } @@ -1008,9 +1030,9 @@ static void Chown(const FunctionCallbackInfo& args) { uv_gid_t gid = static_cast(args[2]->Uint32Value()); if (args[3]->IsFunction()) { - ASYNC_CALL(chown, args[3], *path, uid, gid); + ASYNC_CALL(chown, args[3], 0, *path, uid, gid); } else { - SYNC_CALL(chown, *path, *path, uid, gid); + SYNC_CALL(chown, *path, 0, *path, uid, gid); } } @@ -1041,9 +1063,9 @@ static void FChown(const FunctionCallbackInfo& args) { uv_gid_t gid = static_cast(args[2]->Uint32Value()); if (args[3]->IsFunction()) { - ASYNC_CALL(fchown, args[3], fd, uid, gid); + ASYNC_CALL(fchown, args[3], 0, fd, uid, gid); } else { - SYNC_CALL(fchown, 0, fd, uid, gid); + SYNC_CALL(fchown, 0, 0, fd, uid, gid); } } @@ -1071,9 +1093,9 @@ static void UTimes(const FunctionCallbackInfo& args) { const double mtime = static_cast(args[2]->NumberValue()); if (args[3]->IsFunction()) { - ASYNC_CALL(utime, args[3], *path, atime, mtime); + ASYNC_CALL(utime, args[3], 0, *path, atime, mtime); } else { - SYNC_CALL(utime, *path, *path, atime, mtime); + SYNC_CALL(utime, *path, 0, *path, atime, mtime); } } @@ -1100,9 +1122,9 @@ static void FUTimes(const FunctionCallbackInfo& args) { const double mtime = static_cast(args[2]->NumberValue()); if (args[3]->IsFunction()) { - ASYNC_CALL(futime, args[3], fd, atime, mtime); + ASYNC_CALL(futime, args[3], 0, fd, atime, mtime); } else { - SYNC_CALL(futime, 0, fd, atime, mtime); + SYNC_CALL(futime, 0, 0, fd, atime, mtime); } } diff --git a/src/node_file.h b/src/node_file.h index dc5deedb0a1c..1bb8d1ff68e2 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -27,6 +27,12 @@ namespace node { +typedef struct node_fs_s { + uv_fs_t req; + int throw_safe; + void* data; +} node_fs_t; + void InitFs(v8::Handle target); } // namespace node