1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fs: refactor stats array to be more generic

Browse Source 
- Pass kFsStatsFieldsLength between JS and C++ instead of using the
  magic number 14
- Pass the global stats array to the completion callback of
  asynchronous FSReqWrap similar to how the stats arrays are passed
  to the FSReqPromise resolvers
- Abstract the stats converter and take an offset to compute the
  old stats in fs.watchFile
- Use templates in node::FillStatsArray and FSReqPromise in preparation
  for BigInt intergration
- Put the global stat array filler in node_internals.h because it is
  shared by node_file.cc and node_stat_watcher.cc

PR-URL: https://github.com/nodejs/node/pull/19714
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Joyee Cheung 2018-03-31 04:02:57 +08:00
parent 30fe55e248
commit f7049a2006
No known key found for this signature in database
GPG Key ID: F586868AAD831D0C
9 changed files with 168 additions and 163 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
lib/fs.js
View File

@ -43,7 +43,7 @@ const {
} = errors.codes;
const { Readable, Writable } = require('stream');
const EventEmitter = require('events');
const { FSReqWrap, statValues } = binding;
const { FSReqWrap, statValues, kFsStatsFieldsLength } = binding;
const { FSEvent } = process.binding('fs_event_wrap');
const internalFS = require('internal/fs');
const { getPathFromURL } = require('internal/url');
@ -56,7 +56,8 @@ const {
nullCheck,
preprocessSymlinkDestination,
Stats,
statsFromValues,
getStatsFromBinding,
getStatsFromGlobalBinding,
stringToFlags,
stringToSymlinkType,
toUnixTimestamp,
@ -145,9 +146,9 @@ function makeStatsCallback(cb) {
throw new ERR_INVALID_CALLBACK();
}
return function(err) {
return function(err, stats) {
if (err) return cb(err);
cb(err, statsFromValues());
cb(err, getStatsFromBinding(stats));
};
}
@ -891,7 +892,7 @@ fs.fstatSync = function(fd) {
const ctx = { fd };
binding.fstat(fd, undefined, ctx);
handleErrorFromBinding(ctx);
return statsFromValues();
return getStatsFromGlobalBinding();
};
fs.lstatSync = function(path) {
@ -900,7 +901,7 @@ fs.lstatSync = function(path) {
const ctx = { path };
binding.lstat(pathModule.toNamespacedPath(path), undefined, ctx);
handleErrorFromBinding(ctx);
return statsFromValues();
return getStatsFromGlobalBinding();
};
fs.statSync = function(path) {
@ -909,7 +910,7 @@ fs.statSync = function(path) {
const ctx = { path };
binding.stat(pathModule.toNamespacedPath(path), undefined, ctx);
handleErrorFromBinding(ctx);
return statsFromValues();
return getStatsFromGlobalBinding();
};
fs.readlink = function(path, options, callback) {
@ -1420,15 +1421,6 @@ 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);
@ -1439,13 +1431,14 @@ function StatWatcher() {
// the sake of backwards compatibility
var oldStatus = -1;
this._handle.onchange = function(newStatus) {
this._handle.onchange = function(newStatus, stats) {
if (oldStatus === -1 &&
newStatus === -1 &&
statValues[2/* new nlink */] === statValues[16/* old nlink */]) return;
oldStatus = newStatus;
self.emit('change', statsFromValues(), statsFromPrevValues());
self.emit('change', getStatsFromBinding(stats),
getStatsFromBinding(stats, kFsStatsFieldsLength));
};
this._handle.onstop = function() {

15
lib/fs/promises.js
View File

@ -23,11 +23,11 @@ const { isUint8Array } = require('internal/util/types');
const {
copyObject,
getOptions,
getStatsFromBinding,
isUint32,
modeNum,
nullCheck,
preprocessSymlinkDestination,
statsFromValues,
stringToFlags,
stringToSymlinkType,
toUnixTimestamp,
@ -332,21 +332,24 @@ async function symlink(target, path, type_) {
async function fstat(handle) {
validateFileHandle(handle);
return statsFromValues(await binding.fstat(handle.fd, kUsePromises));
const result = await binding.fstat(handle.fd, kUsePromises);
return getStatsFromBinding(result);
}
async function lstat(path) {
path = getPathFromURL(path);
validatePath(path);
return statsFromValues(
await binding.lstat(pathModule.toNamespacedPath(path), kUsePromises));
const result = await binding.lstat(pathModule.toNamespacedPath(path),
kUsePromises);
return getStatsFromBinding(result);
}
async function stat(path) {
path = getPathFromURL(path);
validatePath(path);
return statsFromValues(
await binding.stat(pathModule.toNamespacedPath(path), kUsePromises));
const result = await binding.stat(pathModule.toNamespacedPath(path),
kUsePromises);
return getStatsFromBinding(result);
}
async function link(existingPath, newPath) {

32
lib/internal/fs.js
View File

@ -134,6 +134,10 @@ function preprocessSymlinkDestination(path, type, linkPath) {
}
}
function dateFromNumeric(num) {
return new Date(num + 0.5);
}
// Constructor for file stats.
function Stats(
dev,
@ -165,10 +169,10 @@ function Stats(
this.mtimeMs = mtim_msec;
this.ctimeMs = ctim_msec;
this.birthtimeMs = birthtim_msec;
this.atime = new Date(atim_msec + 0.5);
this.mtime = new Date(mtim_msec + 0.5);
this.ctime = new Date(ctim_msec + 0.5);
this.birthtime = new Date(birthtim_msec + 0.5);
this.atime = dateFromNumeric(atim_msec);
this.mtime = dateFromNumeric(mtim_msec);
this.ctime = dateFromNumeric(ctim_msec);
this.birthtime = dateFromNumeric(birthtim_msec);
}
Stats.prototype._checkModeProperty = function(property) {
@ -203,11 +207,18 @@ Stats.prototype.isSocket = function() {
return this._checkModeProperty(S_IFSOCK);
};
function statsFromValues(stats = statValues) {
return new Stats(stats[0], stats[1], stats[2], stats[3], stats[4], stats[5],
stats[6] < 0 ? undefined : stats[6], stats[7], stats[8],
stats[9] < 0 ? undefined : stats[9], stats[10], stats[11],
stats[12], stats[13]);
function getStatsFromBinding(stats, offset = 0) {
return new Stats(stats[0 + offset], stats[1 + offset], stats[2 + offset],
stats[3 + offset], stats[4 + offset], stats[5 + offset],
stats[6 + offset] < 0 ? undefined : stats[6 + offset],
stats[7 + offset], stats[8 + offset],
stats[9 + offset] < 0 ? undefined : stats[9 + offset],
stats[10 + offset], stats[11 + offset],
stats[12 + offset], stats[13 + offset]);
}
function getStatsFromGlobalBinding(offset = 0) {
return getStatsFromBinding(statValues, offset);
}
function stringToFlags(flags) {
@ -424,7 +435,8 @@ module.exports = {
nullCheck,
preprocessSymlinkDestination,
realpathCacheKey: Symbol('realpathCacheKey'),
statsFromValues,
getStatsFromBinding,
getStatsFromGlobalBinding,
stringToFlags,
stringToSymlinkType,
Stats,

2
src/env.cc
View File

@ -104,7 +104,7 @@ Environment::Environment(IsolateData* isolate_data,
#endif
handle_cleanup_waiting_(0),
http_parser_buffer_(nullptr),
fs_stats_field_array_(isolate_, kFsStatsFieldsLength),
fs_stats_field_array_(isolate_, kFsStatsFieldsLength * 2),
context_(context->GetIsolate(), context) {
// We'll be creating new objects so make sure we've entered the context.
v8::HandleScope handle_scope(isolate());

7
src/env.h
View File

@ -644,6 +644,10 @@ class Environment {
inline AliasedBuffer<double, v8::Float64Array>* fs_stats_field_array();
// stat fields contains twice the number of entries because `fs.StatWatcher`
// needs room to store data for *two* `fs.Stats` instances.
static const int kFsStatsFieldsLength = 14;
inline std::vector<std::unique_ptr<fs::FileHandleReadWrap>>&
file_handle_read_wrap_freelist();
@ -822,9 +826,6 @@ class Environment {
bool http_parser_buffer_in_use_ = false;
std::unique_ptr<http2::http2_state> http2_state_;
// stat fields contains twice the number of entries because `fs.StatWatcher`
// needs room to store data for *two* `fs.Stats` instances.
static const int kFsStatsFieldsLength = 2 * 14;
AliasedBuffer<double, v8::Float64Array> fs_stats_field_array_;
std::vector<std::unique_ptr<fs::FileHandleReadWrap>>

126
src/node_file.cc
View File

@ -45,42 +45,6 @@
namespace node {
void FillStatsArray(AliasedBuffer<double, v8::Float64Array>* fields_ptr,
const uv_stat_t* s, int offset) {
AliasedBuffer<double, v8::Float64Array>& fields = *fields_ptr;
fields[offset + 0] = s->st_dev;
fields[offset + 1] = s->st_mode;
fields[offset + 2] = s->st_nlink;
fields[offset + 3] = s->st_uid;
fields[offset + 4] = s->st_gid;
fields[offset + 5] = s->st_rdev;
#if defined(__POSIX__)
fields[offset + 6] = s->st_blksize;
#else
fields[offset + 6] = -1;
#endif
fields[offset + 7] = s->st_ino;
fields[offset + 8] = s->st_size;
#if defined(__POSIX__)
fields[offset + 9] = s->st_blocks;
#else
fields[offset + 9] = -1;
#endif
// Dates.
// NO-LINT because the fields are 'long' and we just want to cast to `unsigned`
#define X(idx, name) \
/* NOLINTNEXTLINE(runtime/int) */ \
fields[offset + idx] = ((unsigned long)(s->st_##name.tv_sec) * 1e3) + \
/* NOLINTNEXTLINE(runtime/int) */ \
((unsigned long)(s->st_##name.tv_nsec) / 1e6); \
X(10, atim)
X(11, mtim)
X(12, ctim)
X(13, birthtim)
#undef X
}
namespace fs {
using v8::Array;
@ -432,12 +396,9 @@ void FSReqWrap::Reject(Local<Value> reject) {
MakeCallback(env()->oncomplete_string(), 1, &reject);
}
void FSReqWrap::FillStatsArray(const uv_stat_t* stat) {
node::FillStatsArray(env()->fs_stats_field_array(), stat);
}
void FSReqWrap::ResolveStat() {
Resolve(Undefined(env()->isolate()));
void FSReqWrap::ResolveStat(const uv_stat_t* stat) {
node::FillGlobalStatsArray(env(), stat);
Resolve(env()->fs_stats_field_array()->GetJSArray());
}
void FSReqWrap::Resolve(Local<Value> value) {
@ -452,65 +413,12 @@ void FSReqWrap::SetReturnValue(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().SetUndefined();
}
void FSReqPromise::SetReturnValue(const FunctionCallbackInfo<Value>& args) {
Local<Context> context = env()->context();
args.GetReturnValue().Set(
object()->Get(context, env()->promise_string()).ToLocalChecked()
.As<Promise::Resolver>()->GetPromise());
}
void NewFSReqWrap(const FunctionCallbackInfo<Value>& args) {
CHECK(args.IsConstructCall());
Environment* env = Environment::GetCurrent(args.GetIsolate());
new FSReqWrap(env, args.This());
}
FSReqPromise::FSReqPromise(Environment* env)
: FSReqBase(env,
env->fsreqpromise_constructor_template()
->NewInstance(env->context()).ToLocalChecked(),
AsyncWrap::PROVIDER_FSREQPROMISE),
stats_field_array_(env->isolate(), 14) {
auto resolver = Promise::Resolver::New(env->context()).ToLocalChecked();
object()->Set(env->context(), env->promise_string(),
resolver).FromJust();
}
FSReqPromise::~FSReqPromise() {
// Validate that the promise was explicitly resolved or rejected.
CHECK(finished_);
}
void FSReqPromise::Reject(Local<Value> reject) {
finished_ = true;
HandleScope scope(env()->isolate());
InternalCallbackScope callback_scope(this);
Local<Value> value =
object()->Get(env()->context(),
env()->promise_string()).ToLocalChecked();
Local<Promise::Resolver> resolver = value.As<Promise::Resolver>();
resolver->Reject(env()->context(), reject).FromJust();
}
void FSReqPromise::FillStatsArray(const uv_stat_t* stat) {
node::FillStatsArray(&stats_field_array_, stat);
}
void FSReqPromise::ResolveStat() {
Resolve(stats_field_array_.GetJSArray());
}
void FSReqPromise::Resolve(Local<Value> value) {
finished_ = true;
HandleScope scope(env()->isolate());
InternalCallbackScope callback_scope(this);
Local<Value> val =
object()->Get(env()->context(),
env()->promise_string()).ToLocalChecked();
Local<Promise::Resolver> resolver = val.As<Promise::Resolver>();
resolver->Resolve(env()->context(), value).FromJust();
}
FSReqAfterScope::FSReqAfterScope(FSReqBase* wrap, uv_fs_t* req)
: wrap_(wrap),
req_(req),
@ -563,8 +471,7 @@ void AfterStat(uv_fs_t* req) {
FSReqAfterScope after(req_wrap, req);
if (after.Proceed()) {
req_wrap->FillStatsArray(&req->statbuf);
req_wrap->ResolveStat();
req_wrap->ResolveStat(&req->statbuf);
}
}
@ -751,7 +658,7 @@ inline FSReqBase* GetReqWrap(Environment* env, Local<Value> value) {
if (value->IsObject()) {
return Unwrap<FSReqBase>(value.As<Object>());
} else if (value->StrictEquals(env->fs_use_promises_symbol())) {
return new FSReqPromise(env);
return new FSReqPromise<double, v8::Float64Array>(env);
}
return nullptr;
}
@ -893,7 +800,7 @@ static void Stat(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
const int argc = args.Length();
CHECK_GE(argc, 1);
CHECK_GE(argc, 2);
BufferValue path(env->isolate(), args[0]);
CHECK_NE(*path, nullptr);
@ -907,8 +814,8 @@ static void Stat(const FunctionCallbackInfo<Value>& args) {
FSReqWrapSync req_wrap_sync;
int err = SyncCall(env, args[2], &req_wrap_sync, "stat", uv_fs_stat, *path);
if (err == 0) {
FillStatsArray(env->fs_stats_field_array(),
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr));
node::FillGlobalStatsArray(env,
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr));
}
}
}
@ -917,7 +824,7 @@ static void LStat(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
const int argc = args.Length();
CHECK_GE(argc, 1);
CHECK_GE(argc, 2);
BufferValue path(env->isolate(), args[0]);
CHECK_NE(*path, nullptr);
@ -932,8 +839,8 @@ static void LStat(const FunctionCallbackInfo<Value>& args) {
int err = SyncCall(env, args[2], &req_wrap_sync, "lstat", uv_fs_lstat,
*path);
if (err == 0) {
FillStatsArray(env->fs_stats_field_array(),
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr));
node::FillGlobalStatsArray(env,
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr));
}
}
}
@ -942,7 +849,7 @@ static void FStat(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
const int argc = args.Length();
CHECK_GE(argc, 1);
CHECK_GE(argc, 2);
CHECK(args[0]->IsInt32());
int fd = args[0].As<Int32>()->Value();
@ -956,8 +863,8 @@ static void FStat(const FunctionCallbackInfo<Value>& args) {
FSReqWrapSync req_wrap_sync;
int err = SyncCall(env, args[2], &req_wrap_sync, "fstat", uv_fs_fstat, fd);
if (err == 0) {
FillStatsArray(env->fs_stats_field_array(),
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr));
node::FillGlobalStatsArray(env,
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr));
}
}
}
@ -1908,6 +1815,11 @@ void Initialize(Local<Object> target,
env->SetMethod(target, "mkdtemp", Mkdtemp);
target->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "kFsStatsFieldsLength"),
Integer::New(env->isolate(), env->kFsStatsFieldsLength))
.FromJust();
target->Set(context,
FIXED_ONE_BYTE_STRING(env->isolate(), "statValues"),
env->fs_stats_field_array()->GetJSArray()).FromJust();

64
src/node_file.h
View File

@ -61,10 +61,9 @@ class FSReqBase : public ReqWrap<uv_fs_t> {
return buffer_;
}
virtual void FillStatsArray(const uv_stat_t* stat) = 0;
virtual void Reject(Local<Value> reject) = 0;
virtual void Resolve(Local<Value> value) = 0;
virtual void ResolveStat() = 0;
virtual void ResolveStat(const uv_stat_t* stat) = 0;
virtual void SetReturnValue(const FunctionCallbackInfo<Value>& args) = 0;
const char* syscall() const { return syscall_; }
@ -90,31 +89,72 @@ class FSReqWrap : public FSReqBase {
FSReqWrap(Environment* env, Local<Object> req)
: FSReqBase(env, req, AsyncWrap::PROVIDER_FSREQWRAP) { }
void FillStatsArray(const uv_stat_t* stat) override;
void Reject(Local<Value> reject) override;
void Resolve(Local<Value> value) override;
void ResolveStat() override;
void ResolveStat(const uv_stat_t* stat) override;
void SetReturnValue(const FunctionCallbackInfo<Value>& args) override;
private:
DISALLOW_COPY_AND_ASSIGN(FSReqWrap);
};
template <typename NativeT = double, typename V8T = v8::Float64Array>
class FSReqPromise : public FSReqBase {
public:
explicit FSReqPromise(Environment* env);
explicit FSReqPromise(Environment* env)
: FSReqBase(env,
env->fsreqpromise_constructor_template()
->NewInstance(env->context()).ToLocalChecked(),
AsyncWrap::PROVIDER_FSREQPROMISE),
stats_field_array_(env->isolate(), env->kFsStatsFieldsLength) {
auto resolver = Promise::Resolver::New(env->context()).ToLocalChecked();
object()->Set(env->context(), env->promise_string(),
resolver).FromJust();
}
~FSReqPromise() override;
~FSReqPromise() override {
// Validate that the promise was explicitly resolved or rejected.
CHECK(finished_);
}
void FillStatsArray(const uv_stat_t* stat) override;
void Reject(Local<Value> reject) override;
void Resolve(Local<Value> value) override;
void ResolveStat() override;
void SetReturnValue(const FunctionCallbackInfo<Value>& args) override;
void Reject(Local<Value> reject) override {
finished_ = true;
HandleScope scope(env()->isolate());
InternalCallbackScope callback_scope(this);
Local<Value> value =
object()->Get(env()->context(),
env()->promise_string()).ToLocalChecked();
Local<Promise::Resolver> resolver = value.As<Promise::Resolver>();
resolver->Reject(env()->context(), reject).FromJust();
}
void Resolve(Local<Value> value) override {
finished_ = true;
HandleScope scope(env()->isolate());
InternalCallbackScope callback_scope(this);
Local<Value> val =
object()->Get(env()->context(),
env()->promise_string()).ToLocalChecked();
Local<Promise::Resolver> resolver = val.As<Promise::Resolver>();
resolver->Resolve(env()->context(), value).FromJust();
}
void ResolveStat(const uv_stat_t* stat) override {
node::FillStatsArray(&stats_field_array_, stat);
Resolve(stats_field_array_.GetJSArray());
}
void SetReturnValue(const FunctionCallbackInfo<Value>& args) override {
Local<Value> val =
object()->Get(env()->context(),
env()->promise_string()).ToLocalChecked();
Local<Promise::Resolver> resolver = val.As<Promise::Resolver>();
args.GetReturnValue().Set(resolver->GetPromise());
}
private:
bool finished_ = false;
AliasedBuffer<double, v8::Float64Array> stats_field_array_;
AliasedBuffer<NativeT, V8T> stats_field_array_;
DISALLOW_COPY_AND_ASSIGN(FSReqPromise);
};

44
src/node_internals.h
View File

@ -307,8 +307,48 @@ v8::Maybe<bool> ProcessEmitDeprecationWarning(Environment* env,
const char* warning,
const char* deprecation_code);
void FillStatsArray(AliasedBuffer<double, v8::Float64Array>* fields_ptr,
const uv_stat_t* s, int offset = 0);
template <typename NativeT, typename V8T>
void FillStatsArray(AliasedBuffer<NativeT, V8T>* fields_ptr,
const uv_stat_t* s, int offset = 0) {
AliasedBuffer<NativeT, V8T>& fields = *fields_ptr;
fields[offset + 0] = s->st_dev;
fields[offset + 1] = s->st_mode;
fields[offset + 2] = s->st_nlink;
fields[offset + 3] = s->st_uid;
fields[offset + 4] = s->st_gid;
fields[offset + 5] = s->st_rdev;
#if defined(__POSIX__)
fields[offset + 6] = s->st_blksize;
#else
fields[offset + 6] = -1;
#endif
fields[offset + 7] = s->st_ino;
fields[offset + 8] = s->st_size;
#if defined(__POSIX__)
fields[offset + 9] = s->st_blocks;
#else
fields[offset + 9] = -1;
#endif
// Dates.
// NO-LINT because the fields are 'long' and we just want to cast to `unsigned`
#define X(idx, name) \
/* NOLINTNEXTLINE(runtime/int) */ \
fields[offset + idx] = ((unsigned long)(s->st_##name.tv_sec) * 1e3) + \
/* NOLINTNEXTLINE(runtime/int) */ \
((unsigned long)(s->st_##name.tv_nsec) / 1e6); \
X(10, atim)
X(11, mtim)
X(12, ctim)
X(13, birthtim)
#undef X
}
inline void FillGlobalStatsArray(Environment* env,
const uv_stat_t* s,
int offset = 0) {
node::FillStatsArray(env->fs_stats_field_array(), s, offset);
}
void SetupProcessObject(Environment* env,
int argc,

12
src/node_stat_watcher.cc
View File

@ -107,10 +107,14 @@ void StatWatcher::Callback(uv_fs_poll_t* handle,
HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());
FillStatsArray(env->fs_stats_field_array(), curr);
FillStatsArray(env->fs_stats_field_array(), prev, 14);
Local<Value> arg = Integer::New(env->isolate(), status);
wrap->MakeCallback(env->onchange_string(), 1, &arg);
node::FillGlobalStatsArray(env, curr);
node::FillGlobalStatsArray(env, prev, env->kFsStatsFieldsLength);
Local<Value> argv[2] {
Integer::New(env->isolate(), status),
env->fs_stats_field_array()->GetJSArray()
};
wrap->MakeCallback(env->onchange_string(), arraysize(argv), argv);
}