From 6a5ab5d550719f416683ec0d588461b8bc9a8787 Mon Sep 17 00:00:00 2001 From: Brian White Date: Sat, 11 Mar 2017 19:41:20 -0500 Subject: [PATCH] fs: include more fs.stat*() optimizations Including: * Move async *stat() functions to FillStatsArray() now used by the sync *stat() functions * Avoid creating fs.Stats instances for implicit async/sync *stat() calls used in various fs functions * Store reference to Float64Array data on C++ side for easier/faster access, instead of passing from JS to C++ on every async/sync *stat() call PR-URL: https://github.com/nodejs/node/pull/11665 Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig --- benchmark/fs/bench-stat.js | 22 ++- benchmark/fs/bench-statSync.js | 37 ++--- benchmark/fs/readFileSync.js | 17 +++ lib/fs.js | 153 +++++++++++++-------- src/env-inl.h | 10 ++ src/env.h | 6 +- src/node_file.cc | 189 ++++++-------------------- src/node_internals.h | 2 +- src/node_stat_watcher.cc | 11 +- test/parallel/test-fs-sync-fd-leak.js | 2 +- 10 files changed, 205 insertions(+), 244 deletions(-) create mode 100644 benchmark/fs/readFileSync.js diff --git a/benchmark/fs/bench-stat.js b/benchmark/fs/bench-stat.js index c0db00e27deee6..149b4f3d3bee92 100644 --- a/benchmark/fs/bench-stat.js +++ b/benchmark/fs/bench-stat.js @@ -4,20 +4,30 @@ const common = require('../common'); const fs = require('fs'); const bench = common.createBenchmark(main, { - n: [1e4], - kind: ['lstat', 'stat'] + n: [20e4], + kind: ['fstat', 'lstat', 'stat'] }); function main(conf) { const n = conf.n >>> 0; + const kind = conf.kind; + var arg; + if (kind === 'fstat') + arg = fs.openSync(__filename, 'r'); + else + arg = __filename; bench.start(); (function r(cntr, fn) { - if (cntr-- <= 0) - return bench.end(n); - fn(__filename, function() { + if (cntr-- <= 0) { + bench.end(n); + if (kind === 'fstat') + fs.closeSync(arg); + return; + } + fn(arg, function() { r(cntr, fn); }); - }(n, fs[conf.kind])); + }(n, fs[kind])); } diff --git a/benchmark/fs/bench-statSync.js b/benchmark/fs/bench-statSync.js index 4bc2ecd65a3624..57e03df3b04951 100644 --- a/benchmark/fs/bench-statSync.js +++ b/benchmark/fs/bench-statSync.js @@ -4,36 +4,23 @@ const common = require('../common'); const fs = require('fs'); const bench = common.createBenchmark(main, { - n: [1e4], + n: [1e6], kind: ['fstatSync', 'lstatSync', 'statSync'] }); function main(conf) { const n = conf.n >>> 0; - var fn; - var i; - switch (conf.kind) { - case 'statSync': - case 'lstatSync': - fn = fs[conf.kind]; - bench.start(); - for (i = 0; i < n; i++) { - fn(__filename); - } - bench.end(n); - break; - case 'fstatSync': - fn = fs.fstatSync; - const fd = fs.openSync(__filename, 'r'); - bench.start(); - for (i = 0; i < n; i++) { - fn(fd); - } - bench.end(n); - fs.closeSync(fd); - break; - default: - throw new Error('Invalid kind argument'); + const kind = conf.kind; + const arg = (kind === 'fstatSync' ? fs.openSync(__filename, 'r') : __dirname); + const fn = fs[conf.kind]; + + bench.start(); + for (var i = 0; i < n; i++) { + fn(arg); } + bench.end(n); + + if (kind === 'fstat') + fs.closeSync(arg); } diff --git a/benchmark/fs/readFileSync.js b/benchmark/fs/readFileSync.js new file mode 100644 index 00000000000000..7c8f5d240d1da9 --- /dev/null +++ b/benchmark/fs/readFileSync.js @@ -0,0 +1,17 @@ +'use strict'; + +var common = require('../common.js'); +var fs = require('fs'); + +var bench = common.createBenchmark(main, { + n: [60e4] +}); + +function main(conf) { + var n = +conf.n; + + bench.start(); + for (var i = 0; i < n; ++i) + fs.readFileSync(__filename); + bench.end(n); +} diff --git a/lib/fs.js b/lib/fs.js index d8ae0b09ba414e..7b50412ea76fa9 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -25,6 +25,7 @@ 'use strict'; const constants = process.binding('constants').fs; +const { S_IFMT, S_IFREG, S_IFLNK } = constants; const util = require('util'); const pathModule = require('path'); const { isUint8Array } = process.binding('util'); @@ -135,6 +136,24 @@ function makeCallback(cb) { }; } +// Special case of `makeCallback()` that is specific to async `*stat()` calls as +// an optimization, since the data passed back to the callback needs to be +// transformed anyway. +function makeStatsCallback(cb) { + if (cb === undefined) { + return rethrow(); + } + + if (typeof cb !== 'function') { + throw new TypeError('"callback" argument must be a function'); + } + + return function(err) { + if (err) return cb(err); + cb(err, statsFromValues()); + }; +} + function nullCheck(path, callback) { if (('' + path).indexOf('\u0000') !== -1) { var er = new Error('Path must be a string without null bytes'); @@ -151,7 +170,7 @@ function isFd(path) { return (path >>> 0) === path; } -// Static method to set the stats properties on a Stats object. +// Constructor for file stats. function Stats( dev, mode, @@ -184,41 +203,49 @@ function Stats( } fs.Stats = Stats; -// Create a C++ binding to the function which creates a Stats object. -binding.FSInitialize(fs.Stats); - -fs.Stats.prototype._checkModeProperty = function(property) { - return ((this.mode & constants.S_IFMT) === property); +Stats.prototype._checkModeProperty = function(property) { + return ((this.mode & S_IFMT) === property); }; -fs.Stats.prototype.isDirectory = function() { +Stats.prototype.isDirectory = function() { return this._checkModeProperty(constants.S_IFDIR); }; -fs.Stats.prototype.isFile = function() { - return this._checkModeProperty(constants.S_IFREG); +Stats.prototype.isFile = function() { + return this._checkModeProperty(S_IFREG); }; -fs.Stats.prototype.isBlockDevice = function() { +Stats.prototype.isBlockDevice = function() { return this._checkModeProperty(constants.S_IFBLK); }; -fs.Stats.prototype.isCharacterDevice = function() { +Stats.prototype.isCharacterDevice = function() { return this._checkModeProperty(constants.S_IFCHR); }; -fs.Stats.prototype.isSymbolicLink = function() { - return this._checkModeProperty(constants.S_IFLNK); +Stats.prototype.isSymbolicLink = function() { + return this._checkModeProperty(S_IFLNK); }; -fs.Stats.prototype.isFIFO = function() { +Stats.prototype.isFIFO = function() { return this._checkModeProperty(constants.S_IFIFO); }; -fs.Stats.prototype.isSocket = function() { +Stats.prototype.isSocket = function() { return this._checkModeProperty(constants.S_IFSOCK); }; +const statValues = binding.getStatValues(); + +function statsFromValues() { + return new Stats(statValues[0], statValues[1], statValues[2], statValues[3], + statValues[4], statValues[5], + statValues[6] < 0 ? undefined : statValues[6], statValues[7], + statValues[8], statValues[9] < 0 ? undefined : statValues[9], + statValues[10], statValues[11], statValues[12], + statValues[13]); +} + // Don't allow mode to accidentally be overwritten. ['F_OK', 'R_OK', 'W_OK', 'X_OK'].forEach(function(key) { Object.defineProperty(fs, key, { @@ -275,7 +302,7 @@ fs.exists = function(path, callback) { var req = new FSReqWrap(); req.oncomplete = cb; binding.stat(pathModule._makeLong(path), req); - function cb(err, stats) { + function cb(err) { if (callback) callback(err ? false : true); } }; @@ -284,7 +311,7 @@ fs.existsSync = function(path) { try { handleError((path = getPathFromURL(path))); nullCheck(path); - binding.stat(pathModule._makeLong(path), statValues); + binding.stat(pathModule._makeLong(path)); return true; } catch (e) { return false; @@ -387,13 +414,19 @@ function readFileAfterOpen(err, fd) { binding.fstat(fd, req); } -function readFileAfterStat(err, st) { +function readFileAfterStat(err) { var context = this.context; if (err) return context.close(err); - var size = context.size = st.isFile() ? st.size : 0; + // Use stats array directly to avoid creating an fs.Stats instance just for + // our internal use. + var size; + if ((statValues[1/*mode*/] & S_IFMT) === S_IFREG) + size = context.size = statValues[8/*size*/]; + else + size = context.size = 0; if (size === 0) { context.buffers = []; @@ -467,14 +500,13 @@ function tryToString(buf, encoding, callback) { function tryStatSync(fd, isUserFd) { var threw = true; - var st; try { - st = fs.fstatSync(fd); + binding.fstat(fd); threw = false; } finally { if (threw && !isUserFd) fs.closeSync(fd); } - return st; + return !threw; } function tryCreateBuffer(size, fd, isUserFd) { @@ -506,8 +538,13 @@ fs.readFileSync = function(path, options) { var isUserFd = isFd(path); // file descriptor ownership var fd = isUserFd ? path : fs.openSync(path, options.flag || 'r', 0o666); - var st = tryStatSync(fd, isUserFd); - var size = st.isFile() ? st.size : 0; + // Use stats array directly to avoid creating an fs.Stats instance just for + // our internal use. + var size; + if (tryStatSync(fd, isUserFd) && (statValues[1/*mode*/] & S_IFMT) === S_IFREG) + size = statValues[8/*size*/]; + else + size = 0; var pos = 0; var buffer; // single buffer with file data var buffers; // list for when size is unknown @@ -854,12 +891,12 @@ fs.readdirSync = function(path, options) { fs.fstat = function(fd, callback) { var req = new FSReqWrap(); - req.oncomplete = makeCallback(callback); + req.oncomplete = makeStatsCallback(callback); binding.fstat(fd, req); }; fs.lstat = function(path, callback) { - callback = makeCallback(callback); + callback = makeStatsCallback(callback); if (handleError((path = getPathFromURL(path)), callback)) return; if (!nullCheck(path, callback)) return; @@ -869,7 +906,7 @@ fs.lstat = function(path, callback) { }; fs.stat = function(path, callback) { - callback = makeCallback(callback); + callback = makeStatsCallback(callback); if (handleError((path = getPathFromURL(path)), callback)) return; if (!nullCheck(path, callback)) return; @@ -878,32 +915,22 @@ fs.stat = function(path, callback) { binding.stat(pathModule._makeLong(path), req); }; -const statValues = new Float64Array(14); -function statsFromValues() { - return new Stats(statValues[0], statValues[1], statValues[2], statValues[3], - statValues[4], statValues[5], - statValues[6] < 0 ? undefined : statValues[6], statValues[7], - statValues[8], statValues[9] < 0 ? undefined : statValues[9], - statValues[10], statValues[11], statValues[12], - statValues[13]); -} - fs.fstatSync = function(fd) { - binding.fstat(fd, statValues); + binding.fstat(fd); return statsFromValues(); }; fs.lstatSync = function(path) { handleError((path = getPathFromURL(path))); nullCheck(path); - binding.lstat(pathModule._makeLong(path), statValues); + binding.lstat(pathModule._makeLong(path)); return statsFromValues(); }; fs.statSync = function(path) { handleError((path = getPathFromURL(path))); nullCheck(path); - binding.stat(pathModule._makeLong(path), statValues); + binding.stat(pathModule._makeLong(path)); return statsFromValues(); }; @@ -1380,6 +1407,15 @@ function emitStop(self) { self.emit('stop'); } +function statsFromPrevValues() { + return new Stats(statValues[14], statValues[15], statValues[16], + statValues[17], statValues[18], statValues[19], + statValues[20] < 0 ? undefined : statValues[20], + statValues[21], statValues[22], + statValues[23] < 0 ? undefined : statValues[23], + statValues[24], statValues[25], statValues[26], + statValues[27]); +} function StatWatcher() { EventEmitter.call(this); @@ -1390,13 +1426,13 @@ function StatWatcher() { // the sake of backwards compatibility var oldStatus = -1; - this._handle.onchange = function(current, previous, newStatus) { + this._handle.onchange = function(newStatus) { if (oldStatus === -1 && newStatus === -1 && - current.nlink === previous.nlink) return; + statValues[2/*new nlink*/] === statValues[16/*old nlink*/]) return; oldStatus = newStatus; - self.emit('change', current, previous); + self.emit('change', statsFromValues(), statsFromPrevValues()); }; this._handle.onstop = function() { @@ -1544,7 +1580,7 @@ fs.realpathSync = function realpathSync(p, options) { // On windows, check that the root exists. On unix there is no need. if (isWindows && !knownHard[base]) { - fs.lstatSync(base); + binding.lstat(pathModule._makeLong(base)); knownHard[base] = true; } @@ -1576,8 +1612,12 @@ fs.realpathSync = function realpathSync(p, options) { if (maybeCachedResolved) { resolvedLink = maybeCachedResolved; } else { - var stat = fs.lstatSync(base); - if (!stat.isSymbolicLink()) { + // Use stats array directly to avoid creating an fs.Stats instance just + // for our internal use. + + binding.lstat(pathModule._makeLong(base)); + + if ((statValues[1/*mode*/] & S_IFMT) !== S_IFLNK) { knownHard[base] = true; if (cache) cache.set(base, base); continue; @@ -1588,14 +1628,16 @@ fs.realpathSync = function realpathSync(p, options) { var linkTarget = null; var id; if (!isWindows) { - id = `${stat.dev.toString(32)}:${stat.ino.toString(32)}`; + var dev = statValues[0/*dev*/].toString(32); + var ino = statValues[7/*ino*/].toString(32); + id = `${dev}:${ino}`; if (seenLinks.hasOwnProperty(id)) { linkTarget = seenLinks[id]; } } if (linkTarget === null) { - fs.statSync(base); - linkTarget = fs.readlinkSync(base); + binding.stat(pathModule._makeLong(base)); + linkTarget = binding.readlink(pathModule._makeLong(base)); } resolvedLink = pathModule.resolve(previous, linkTarget); @@ -1614,7 +1656,7 @@ fs.realpathSync = function realpathSync(p, options) { // On windows, check that the root exists. On unix there is no need. if (isWindows && !knownHard[base]) { - fs.lstatSync(base); + binding.lstat(pathModule._makeLong(base)); knownHard[base] = true; } } @@ -1694,11 +1736,14 @@ fs.realpath = function realpath(p, options, callback) { return fs.lstat(base, gotStat); } - function gotStat(err, stat) { + function gotStat(err) { if (err) return callback(err); + // Use stats array directly to avoid creating an fs.Stats instance just for + // our internal use. + // if not a symlink, skip to the next path part - if (!stat.isSymbolicLink()) { + if ((statValues[1/*mode*/] & S_IFMT) !== S_IFLNK) { knownHard[base] = true; return process.nextTick(LOOP); } @@ -1708,7 +1753,9 @@ fs.realpath = function realpath(p, options, callback) { // dev/ino always return 0 on windows, so skip the check. let id; if (!isWindows) { - id = `${stat.dev.toString(32)}:${stat.ino.toString(32)}`; + var dev = statValues[0/*ino*/].toString(32); + var ino = statValues[7/*ino*/].toString(32); + id = `${dev}:${ino}`; if (seenLinks.hasOwnProperty(id)) { return gotTarget(null, seenLinks[id], base); } diff --git a/src/env-inl.h b/src/env-inl.h index 06a9474c9d3775..978f8ca819dec1 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -199,6 +199,7 @@ inline Environment::Environment(IsolateData* isolate_data, #endif handle_cleanup_waiting_(0), http_parser_buffer_(nullptr), + fs_stats_field_array_(nullptr), context_(context->GetIsolate(), context) { // We'll be creating new objects so make sure we've entered the context. v8::HandleScope handle_scope(isolate()); @@ -380,6 +381,15 @@ inline void Environment::set_http_parser_buffer(char* buffer) { http_parser_buffer_ = buffer; } +inline double* Environment::fs_stats_field_array() const { + return fs_stats_field_array_; +} + +inline void Environment::set_fs_stats_field_array(double* fields) { + CHECK_EQ(fs_stats_field_array_, nullptr); // Should be set only once. + fs_stats_field_array_ = fields; +} + inline Environment* Environment::from_cares_timer_handle(uv_timer_t* handle) { return ContainerOf(&Environment::cares_timer_handle_, handle); } diff --git a/src/env.h b/src/env.h index 06cc517435af52..abb0e6d0e5a8a5 100644 --- a/src/env.h +++ b/src/env.h @@ -253,7 +253,6 @@ namespace node { V(context, v8::Context) \ V(domain_array, v8::Array) \ V(domains_stack_array, v8::Array) \ - V(fs_stats_constructor_function, v8::Function) \ V(generic_internal_field_template, v8::ObjectTemplate) \ V(jsstream_constructor_template, v8::FunctionTemplate) \ V(module_load_list_array, v8::Array) \ @@ -492,6 +491,9 @@ class Environment { inline char* http_parser_buffer() const; inline void set_http_parser_buffer(char* buffer); + inline double* fs_stats_field_array() const; + inline void set_fs_stats_field_array(double* fields); + inline void ThrowError(const char* errmsg); inline void ThrowTypeError(const char* errmsg); inline void ThrowRangeError(const char* errmsg); @@ -600,6 +602,8 @@ class Environment { char* http_parser_buffer_; + double* fs_stats_field_array_; + #define V(PropertyName, TypeName) \ v8::Persistent PropertyName ## _; ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) diff --git a/src/node_file.cc b/src/node_file.cc index 8e738fb8bfa8d3..9c180833fb926f 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -50,7 +50,6 @@ namespace node { using v8::Array; using v8::ArrayBuffer; using v8::Context; -using v8::EscapableHandleScope; using v8::Float64Array; using v8::Function; using v8::FunctionCallbackInfo; @@ -211,6 +210,14 @@ static void After(uv_fs_t *req) { argc = 1; break; + case UV_FS_STAT: + case UV_FS_LSTAT: + case UV_FS_FSTAT: + argc = 1; + FillStatsArray(env->fs_stats_field_array(), + static_cast(req->ptr)); + break; + case UV_FS_UTIME: case UV_FS_FUTIME: argc = 0; @@ -224,13 +231,6 @@ static void After(uv_fs_t *req) { argv[1] = Integer::New(env->isolate(), req->result); break; - case UV_FS_STAT: - case UV_FS_LSTAT: - case UV_FS_FSTAT: - argv[1] = BuildStatsObject(env, - static_cast(req->ptr)); - break; - case UV_FS_MKDTEMP: link = StringBytes::Encode(env->isolate(), static_cast(req->path), @@ -441,116 +441,6 @@ static void Close(const FunctionCallbackInfo& args) { } -Local BuildStatsObject(Environment* env, const uv_stat_t* s) { - EscapableHandleScope handle_scope(env->isolate()); - - // If you hit this assertion, you forgot to enter the v8::Context first. - CHECK_EQ(env->context(), env->isolate()->GetCurrentContext()); - - // The code below is very nasty-looking but it prevents a segmentation fault - // when people run JS code like the snippet below. It's apparently more - // common than you would expect, several people have reported this crash... - // - // function crash() { - // fs.statSync('.'); - // crash(); - // } - // - // We need to check the return value of Number::New() and Date::New() - // and make sure that we bail out when V8 returns an empty handle. - - // Unsigned integers. It does not actually seem to be specified whether - // uid and gid are unsigned or not, but in practice they are unsigned, - // and Node’s (F)Chown functions do check their arguments for unsignedness. -#define X(name) \ - Local name = Integer::NewFromUnsigned(env->isolate(), s->st_##name); \ - if (name.IsEmpty()) \ - return Local(); \ - - X(uid) - X(gid) -# if defined(__POSIX__) - X(blksize) -# else - Local blksize = Undefined(env->isolate()); -# endif -#undef X - - // Integers. -#define X(name) \ - Local name = Integer::New(env->isolate(), s->st_##name); \ - if (name.IsEmpty()) \ - return Local(); \ - - X(dev) - X(mode) - X(nlink) - X(rdev) -#undef X - - // Numbers. -#define X(name) \ - Local name = Number::New(env->isolate(), \ - static_cast(s->st_##name)); \ - if (name.IsEmpty()) \ - return Local(); \ - - X(ino) - X(size) -# if defined(__POSIX__) - X(blocks) -# else - Local blocks = Undefined(env->isolate()); -# endif -#undef X - - // Dates. -#define X(name) \ - Local name##_msec = \ - Number::New(env->isolate(), \ - (static_cast(s->st_##name.tv_sec) * 1000) + \ - (static_cast(s->st_##name.tv_nsec / 1000000))); \ - \ - if (name##_msec.IsEmpty()) \ - return Local(); \ - - X(atim) - X(mtim) - X(ctim) - X(birthtim) -#undef X - - // Pass stats as the first argument, this is the object we are modifying. - Local argv[] = { - dev, - mode, - nlink, - uid, - gid, - rdev, - blksize, - ino, - size, - blocks, - atim_msec, - mtim_msec, - ctim_msec, - birthtim_msec - }; - - // Call out to JavaScript to create the stats object. - Local stats = - env->fs_stats_constructor_function()->NewInstance( - env->context(), - arraysize(argv), - argv).FromMaybe(Local()); - - if (stats.IsEmpty()) - return handle_scope.Escape(Local()); - - return handle_scope.Escape(stats); -} - void FillStatsArray(double* fields, const uv_stat_t* s) { fields[0] = s->st_dev; fields[1] = s->st_mode; @@ -666,15 +556,12 @@ static void Stat(const FunctionCallbackInfo& args) { BufferValue path(env->isolate(), args[0]); ASSERT_PATH(path) - if (args[1]->IsFloat64Array()) { - Local array = args[1].As(); - CHECK_EQ(array->Length(), 14); - Local ab = array->Buffer(); - double* fields = static_cast(ab->GetContents().Data()); - SYNC_CALL(stat, *path, *path) - FillStatsArray(fields, static_cast(SYNC_REQ.ptr)); - } else if (args[1]->IsObject()) { + if (args[1]->IsObject()) { ASYNC_CALL(stat, args[1], UTF8, *path) + } else { + SYNC_CALL(stat, *path, *path) + FillStatsArray(env->fs_stats_field_array(), + static_cast(SYNC_REQ.ptr)); } } @@ -687,15 +574,12 @@ static void LStat(const FunctionCallbackInfo& args) { BufferValue path(env->isolate(), args[0]); ASSERT_PATH(path) - if (args[1]->IsFloat64Array()) { - Local array = args[1].As(); - CHECK_EQ(array->Length(), 14); - Local ab = array->Buffer(); - double* fields = static_cast(ab->GetContents().Data()); - SYNC_CALL(lstat, *path, *path) - FillStatsArray(fields, static_cast(SYNC_REQ.ptr)); - } else if (args[1]->IsObject()) { + if (args[1]->IsObject()) { ASYNC_CALL(lstat, args[1], UTF8, *path) + } else { + SYNC_CALL(lstat, *path, *path) + FillStatsArray(env->fs_stats_field_array(), + static_cast(SYNC_REQ.ptr)); } } @@ -709,15 +593,12 @@ static void FStat(const FunctionCallbackInfo& args) { int fd = args[0]->Int32Value(); - if (args[1]->IsFloat64Array()) { - Local array = args[1].As(); - CHECK_EQ(array->Length(), 14); - Local ab = array->Buffer(); - double* fields = static_cast(ab->GetContents().Data()); - SYNC_CALL(fstat, 0, fd) - FillStatsArray(fields, static_cast(SYNC_REQ.ptr)); - } else if (args[1]->IsObject()) { + if (args[1]->IsObject()) { ASYNC_CALL(fstat, args[1], UTF8, fd) + } else { + SYNC_CALL(fstat, nullptr, fd) + FillStatsArray(env->fs_stats_field_array(), + static_cast(SYNC_REQ.ptr)); } } @@ -1496,12 +1377,20 @@ static void Mkdtemp(const FunctionCallbackInfo& args) { } } -void FSInitialize(const FunctionCallbackInfo& args) { - Local stats_constructor = args[0].As(); - CHECK(stats_constructor->IsFunction()); - +void GetStatValues(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - env->set_fs_stats_constructor_function(stats_constructor); + double* fields = env->fs_stats_field_array(); + if (fields == nullptr) { + // stat fields contains twice the number of entries because `fs.StatWatcher` + // needs room to store data for *two* `fs.Stats` instances. + fields = new double[2 * 14]; + env->set_fs_stats_field_array(fields); + } + Local ab = ArrayBuffer::New(env->isolate(), + fields, + sizeof(double) * 2 * 14); + Local fields_array = Float64Array::New(ab, 0, 2 * 14); + args.GetReturnValue().Set(fields_array); } void InitFs(Local target, @@ -1510,10 +1399,6 @@ void InitFs(Local target, void* priv) { Environment* env = Environment::GetCurrent(context); - // Function which creates a new Stats object. - target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "FSInitialize"), - env->NewFunctionTemplate(FSInitialize)->GetFunction()); - env->SetMethod(target, "access", Access); env->SetMethod(target, "close", Close); env->SetMethod(target, "open", Open); @@ -1552,6 +1437,8 @@ void InitFs(Local target, env->SetMethod(target, "mkdtemp", Mkdtemp); + env->SetMethod(target, "getStatValues", GetStatValues); + StatWatcher::Initialize(env, target); // Create FunctionTemplate for FSReqWrap diff --git a/src/node_internals.h b/src/node_internals.h index ff1f1cd11dba4e..0ee694a2284707 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -162,7 +162,7 @@ NO_RETURN void FatalError(const char* location, const char* message); void ProcessEmitWarning(Environment* env, const char* fmt, ...); -v8::Local BuildStatsObject(Environment* env, const uv_stat_t* s); +void FillStatsArray(double* fields, const uv_stat_t* s); void SetupProcessObject(Environment* env, int argc, diff --git a/src/node_stat_watcher.cc b/src/node_stat_watcher.cc index 9289efcf3e5a3d..9eeed77476be56 100644 --- a/src/node_stat_watcher.cc +++ b/src/node_stat_watcher.cc @@ -86,12 +86,11 @@ void StatWatcher::Callback(uv_fs_poll_t* handle, Environment* env = wrap->env(); HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); - Local argv[] = { - BuildStatsObject(env, curr), - BuildStatsObject(env, prev), - Integer::New(env->isolate(), status) - }; - wrap->MakeCallback(env->onchange_string(), arraysize(argv), argv); + + FillStatsArray(env->fs_stats_field_array(), curr); + FillStatsArray(env->fs_stats_field_array() + 14, prev); + Local arg = Integer::New(env->isolate(), status); + wrap->MakeCallback(env->onchange_string(), 1, &arg); } diff --git a/test/parallel/test-fs-sync-fd-leak.js b/test/parallel/test-fs-sync-fd-leak.js index 10b09481ddefb9..7e785ea3a2a82b 100644 --- a/test/parallel/test-fs-sync-fd-leak.js +++ b/test/parallel/test-fs-sync-fd-leak.js @@ -39,7 +39,7 @@ fs.writeSync = function() { throw new Error('BAM'); }; -fs.fstatSync = function() { +process.binding('fs').fstat = function() { throw new Error('BAM'); };