Skip to content

Commit

Permalink
src: refactor FillStatsArray
Browse files Browse the repository at this point in the history
PR-URL: #23793
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
refack authored and rvagg committed Nov 28, 2018
1 parent b6004b3 commit c0a9a83
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 81 deletions.
4 changes: 2 additions & 2 deletions lib/internal/fs/watchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const errors = require('internal/errors');
const {
kFsStatsFieldsLength,
kFsStatsFieldsNumber,
StatWatcher: _StatWatcher
} = process.binding('fs');
const { FSEvent } = internalBinding('fs_event_wrap');
Expand Down Expand Up @@ -48,7 +48,7 @@ function onchange(newStatus, stats) {

self[kOldStatus] = newStatus;
self.emit('change', getStatsFromBinding(stats),
getStatsFromBinding(stats, kFsStatsFieldsLength));
getStatsFromBinding(stats, kFsStatsFieldsNumber));
}

// FIXME(joyeecheung): this method is not documented.
Expand Down
4 changes: 2 additions & 2 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ Environment::Environment(IsolateData* isolate_data,
trace_category_state_(isolate_, kTraceCategoryCount),
stream_base_state_(isolate_, StreamBase::kNumStreamBaseStateFields),
http_parser_buffer_(nullptr),
fs_stats_field_array_(isolate_, kFsStatsFieldsLength * 2),
fs_stats_field_bigint_array_(isolate_, kFsStatsFieldsLength * 2),
fs_stats_field_array_(isolate_, kFsStatsBufferLength),
fs_stats_field_bigint_array_(isolate_, kFsStatsBufferLength),
context_(context->GetIsolate(), context) {
// We'll be creating new objects so make sure we've entered the context.
v8::HandleScope handle_scope(isolate());
Expand Down
9 changes: 5 additions & 4 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ struct PackageConfig {
};
} // namespace loader

// Stat fields buffers contain twice the number of entries in an uv_stat_t
// because `fs.StatWatcher` needs room to store 2 `fs.Stats` instances.
constexpr size_t kFsStatsFieldsNumber = 14;
constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;

// PER_ISOLATE_* macros: We have a lot of per-isolate properties
// and adding and maintaining their getters and setters by hand would be
// difficult so let's make the preprocessor generate them for us.
Expand Down Expand Up @@ -712,10 +717,6 @@ class Environment {
inline AliasedBuffer<uint64_t, v8::BigUint64Array>*
fs_stats_field_bigint_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();

Expand Down
18 changes: 9 additions & 9 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ void FSReqCallback::Reject(Local<Value> reject) {
}

void FSReqCallback::ResolveStat(const uv_stat_t* stat) {
Resolve(node::FillGlobalStatsArray(env(), stat, use_bigint()));
Resolve(FillGlobalStatsArray(env(), use_bigint(), stat));
}

void FSReqCallback::Resolve(Local<Value> value) {
Expand Down Expand Up @@ -899,8 +899,8 @@ static void Stat(const FunctionCallbackInfo<Value>& args) {
return; // error info is in ctx
}

Local<Value> arr = node::FillGlobalStatsArray(env,
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr), use_bigint);
Local<Value> arr = FillGlobalStatsArray(env, use_bigint,
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr));
args.GetReturnValue().Set(arr);
}
}
Expand Down Expand Up @@ -930,8 +930,8 @@ static void LStat(const FunctionCallbackInfo<Value>& args) {
return; // error info is in ctx
}

Local<Value> arr = node::FillGlobalStatsArray(env,
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr), use_bigint);
Local<Value> arr = FillGlobalStatsArray(env, use_bigint,
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr));
args.GetReturnValue().Set(arr);
}
}
Expand Down Expand Up @@ -960,8 +960,8 @@ static void FStat(const FunctionCallbackInfo<Value>& args) {
return; // error info is in ctx
}

Local<Value> arr = node::FillGlobalStatsArray(env,
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr), use_bigint);
Local<Value> arr = FillGlobalStatsArray(env, use_bigint,
static_cast<const uv_stat_t*>(req_wrap_sync.req.ptr));
args.GetReturnValue().Set(arr);
}
}
Expand Down Expand Up @@ -2150,8 +2150,8 @@ void Initialize(Local<Object> target,
env->SetMethod(target, "mkdtemp", Mkdtemp);

target->Set(context,
FIXED_ONE_BYTE_STRING(isolate, "kFsStatsFieldsLength"),
Integer::New(isolate, env->kFsStatsFieldsLength))
FIXED_ONE_BYTE_STRING(isolate, "kFsStatsFieldsNumber"),
Integer::New(isolate, kFsStatsFieldsNumber))
.FromJust();

target->Set(context,
Expand Down
91 changes: 89 additions & 2 deletions src/node_file.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,92 @@ class FSReqCallback : public FSReqBase {
DISALLOW_COPY_AND_ASSIGN(FSReqCallback);
};

// Wordaround a GCC4.9 bug that C++14 N3652 was not implemented
// Refs: https://www.gnu.org/software/gcc/projects/cxx-status.html#cxx14
// Refs: https://isocpp.org/files/papers/N3652.html
#if __cpp_constexpr < 201304
# define constexpr inline
#endif

template <typename NativeT,
// SFINAE limit NativeT to arithmetic types
typename = std::enable_if<std::is_arithmetic<NativeT>::value>>
constexpr NativeT ToNative(uv_timespec_t ts) {
// This template has exactly two specializations below.
static_assert(std::is_arithmetic<NativeT>::value == false, "Not implemented");
UNREACHABLE();
}

template <>
constexpr double ToNative(uv_timespec_t ts) {
// We need to do a static_cast since the original FS values are ulong.
/* NOLINTNEXTLINE(runtime/int) */
const auto u_sec = static_cast<unsigned long>(ts.tv_sec);
const double full_sec = u_sec * 1000.0;
/* NOLINTNEXTLINE(runtime/int) */
const auto u_nsec = static_cast<unsigned long>(ts.tv_nsec);
const double full_nsec = u_nsec / 1000'000.0;
return full_sec + full_nsec;
}

template <>
constexpr uint64_t ToNative(uv_timespec_t ts) {
// We need to do a static_cast since the original FS values are ulong.
/* NOLINTNEXTLINE(runtime/int) */
const auto u_sec = static_cast<unsigned long>(ts.tv_sec);
const auto full_sec = static_cast<uint64_t>(u_sec) * 1000UL;
/* NOLINTNEXTLINE(runtime/int) */
const auto u_nsec = static_cast<unsigned long>(ts.tv_nsec);
const auto full_nsec = static_cast<uint64_t>(u_nsec) / 1000'000UL;
return full_sec + full_nsec;
}

#undef constexpr // end N3652 bug workaround

template <typename NativeT, typename V8T>
constexpr void FillStatsArray(AliasedBuffer<NativeT, V8T>* fields,
const uv_stat_t* s, const size_t offset = 0) {
fields->SetValue(offset + 0, s->st_dev);
fields->SetValue(offset + 1, s->st_mode);
fields->SetValue(offset + 2, s->st_nlink);
fields->SetValue(offset + 3, s->st_uid);
fields->SetValue(offset + 4, s->st_gid);
fields->SetValue(offset + 5, s->st_rdev);
#if defined(__POSIX__)
fields->SetValue(offset + 6, s->st_blksize);
#else
fields->SetValue(offset + 6, 0);
#endif
fields->SetValue(offset + 7, s->st_ino);
fields->SetValue(offset + 8, s->st_size);
#if defined(__POSIX__)
fields->SetValue(offset + 9, s->st_blocks);
#else
fields->SetValue(offset + 9, 0);
#endif
// Dates.
fields->SetValue(offset + 10, ToNative<NativeT>(s->st_atim));
fields->SetValue(offset + 11, ToNative<NativeT>(s->st_mtim));
fields->SetValue(offset + 12, ToNative<NativeT>(s->st_ctim));
fields->SetValue(offset + 13, ToNative<NativeT>(s->st_birthtim));
}

inline Local<Value> FillGlobalStatsArray(Environment* env,
const bool use_bigint,
const uv_stat_t* s,
const bool second = false) {
const ptrdiff_t offset = second ? kFsStatsFieldsNumber : 0;
if (use_bigint) {
auto* const arr = env->fs_stats_field_bigint_array();
FillStatsArray(arr, s, offset);
return arr->GetJSArray();
} else {
auto* const arr = env->fs_stats_field_array();
FillStatsArray(arr, s, offset);
return arr->GetJSArray();
}
}

template <typename NativeT = double, typename V8T = v8::Float64Array>
class FSReqPromise : public FSReqBase {
public:
Expand All @@ -157,7 +243,7 @@ class FSReqPromise : public FSReqBase {
->NewInstance(env->context()).ToLocalChecked(),
AsyncWrap::PROVIDER_FSREQPROMISE,
use_bigint),
stats_field_array_(env->isolate(), env->kFsStatsFieldsLength) {
stats_field_array_(env->isolate(), kFsStatsFieldsNumber) {
auto resolver = Promise::Resolver::New(env->context()).ToLocalChecked();
object()->Set(env->context(), env->promise_string(),
resolver).FromJust();
Expand Down Expand Up @@ -191,7 +277,8 @@ class FSReqPromise : public FSReqBase {
}

void ResolveStat(const uv_stat_t* stat) override {
Resolve(node::FillStatsArray(&stats_field_array_, stat));
FillStatsArray(&stats_field_array_, stat);
Resolve(stats_field_array_.GetJSArray());
}

void SetReturnValue(const FunctionCallbackInfo<Value>& args) override {
Expand Down
52 changes: 0 additions & 52 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#include "uv.h"
#include "v8.h"
#include "tracing/trace_event.h"
#include "node_perf_common.h"
#include "node_api.h"

#include <stdint.h>
Expand Down Expand Up @@ -271,57 +270,6 @@ v8::Maybe<bool> ProcessEmitDeprecationWarning(Environment* env,
const char* warning,
const char* deprecation_code);

template <typename NativeT, typename V8T>
v8::Local<v8::Value> 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] = 0;
#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] = 0;
#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

return fields_ptr->GetJSArray();
}

inline v8::Local<v8::Value> FillGlobalStatsArray(Environment* env,
const uv_stat_t* s,
bool use_bigint = false,
int offset = 0) {
if (use_bigint) {
return node::FillStatsArray(
env->fs_stats_field_bigint_array(), s, offset);
} else {
return node::FillStatsArray(env->fs_stats_field_array(), s, offset);
}
}

void SetupBootstrapObject(Environment* env,
v8::Local<v8::Object> bootstrapper);
void SetupProcessObject(Environment* env,
Expand Down
15 changes: 5 additions & 10 deletions src/node_stat_watcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
#include "node_stat_watcher.h"
#include "node_internals.h"
#include "async_wrap-inl.h"
#include "env-inl.h"
#include "util-inl.h"
#include "env.h"
#include "node_file.h"

#include <string.h>
#include <stdlib.h>
Expand Down Expand Up @@ -80,15 +80,10 @@ void StatWatcher::Callback(uv_fs_poll_t* handle,
HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());

Local<Value> arr = node::FillGlobalStatsArray(env, curr,
wrap->use_bigint_);
node::FillGlobalStatsArray(env, prev, wrap->use_bigint_,
env->kFsStatsFieldsLength);
Local<Value> arr = fs::FillGlobalStatsArray(env, wrap->use_bigint_, curr);
USE(fs::FillGlobalStatsArray(env, wrap->use_bigint_, prev, true));

Local<Value> argv[2] {
Integer::New(env->isolate(), status),
arr
};
Local<Value> argv[2] = { Integer::New(env->isolate(), status), arr };
wrap->MakeCallback(env->onchange_string(), arraysize(argv), argv);
}

Expand Down

0 comments on commit c0a9a83

Please sign in to comment.