Skip to content

Commit c0a9a83

Browse files
refackrvagg
refack
authored and
rvagg
committed
src: refactor FillStatsArray
PR-URL: #23793 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
1 parent b6004b3 commit c0a9a83

File tree

7 files changed

+112
-81
lines changed

7 files changed

+112
-81
lines changed

‎lib/internal/fs/watchers.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
const errors = require('internal/errors');
44
const {
5-
kFsStatsFieldsLength,
5+
kFsStatsFieldsNumber,
66
StatWatcher: _StatWatcher
77
} = process.binding('fs');
88
const { FSEvent } = internalBinding('fs_event_wrap');
@@ -48,7 +48,7 @@ function onchange(newStatus, stats) {
4848

4949
self[kOldStatus] = newStatus;
5050
self.emit('change', getStatsFromBinding(stats),
51-
getStatsFromBinding(stats, kFsStatsFieldsLength));
51+
getStatsFromBinding(stats, kFsStatsFieldsNumber));
5252
}
5353

5454
// FIXME(joyeecheung): this method is not documented.

‎src/env.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,8 @@ Environment::Environment(IsolateData* isolate_data,
169169
trace_category_state_(isolate_, kTraceCategoryCount),
170170
stream_base_state_(isolate_, StreamBase::kNumStreamBaseStateFields),
171171
http_parser_buffer_(nullptr),
172-
fs_stats_field_array_(isolate_, kFsStatsFieldsLength * 2),
173-
fs_stats_field_bigint_array_(isolate_, kFsStatsFieldsLength * 2),
172+
fs_stats_field_array_(isolate_, kFsStatsBufferLength),
173+
fs_stats_field_bigint_array_(isolate_, kFsStatsBufferLength),
174174
context_(context->GetIsolate(), context) {
175175
// We'll be creating new objects so make sure we've entered the context.
176176
v8::HandleScope handle_scope(isolate());

‎src/env.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ struct PackageConfig {
8282
};
8383
} // namespace loader
8484

85+
// Stat fields buffers contain twice the number of entries in an uv_stat_t
86+
// because `fs.StatWatcher` needs room to store 2 `fs.Stats` instances.
87+
constexpr size_t kFsStatsFieldsNumber = 14;
88+
constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
89+
8590
// PER_ISOLATE_* macros: We have a lot of per-isolate properties
8691
// and adding and maintaining their getters and setters by hand would be
8792
// difficult so let's make the preprocessor generate them for us.
@@ -712,10 +717,6 @@ class Environment {
712717
inline AliasedBuffer<uint64_t, v8::BigUint64Array>*
713718
fs_stats_field_bigint_array();
714719

715-
// stat fields contains twice the number of entries because `fs.StatWatcher`
716-
// needs room to store data for *two* `fs.Stats` instances.
717-
static const int kFsStatsFieldsLength = 14;
718-
719720
inline std::vector<std::unique_ptr<fs::FileHandleReadWrap>>&
720721
file_handle_read_wrap_freelist();
721722

‎src/node_file.cc‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ void FSReqCallback::Reject(Local<Value> reject) {
426426
}
427427

428428
void FSReqCallback::ResolveStat(const uv_stat_t* stat) {
429-
Resolve(node::FillGlobalStatsArray(env(), stat, use_bigint()));
429+
Resolve(FillGlobalStatsArray(env(), use_bigint(), stat));
430430
}
431431

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

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

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

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

21522152
target->Set(context,
2153-
FIXED_ONE_BYTE_STRING(isolate, "kFsStatsFieldsLength"),
2154-
Integer::New(isolate, env->kFsStatsFieldsLength))
2153+
FIXED_ONE_BYTE_STRING(isolate, "kFsStatsFieldsNumber"),
2154+
Integer::New(isolate, kFsStatsFieldsNumber))
21552155
.FromJust();
21562156

21572157
target->Set(context,

‎src/node_file.h‎

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,92 @@ class FSReqCallback : public FSReqBase {
148148
DISALLOW_COPY_AND_ASSIGN(FSReqCallback);
149149
};
150150

151+
// Wordaround a GCC4.9 bug that C++14 N3652 was not implemented
152+
// Refs: https://www.gnu.org/software/gcc/projects/cxx-status.html#cxx14
153+
// Refs: https://isocpp.org/files/papers/N3652.html
154+
#if __cpp_constexpr < 201304
155+
# define constexpr inline
156+
#endif
157+
158+
template <typename NativeT,
159+
// SFINAE limit NativeT to arithmetic types
160+
typename = std::enable_if<std::is_arithmetic<NativeT>::value>>
161+
constexpr NativeT ToNative(uv_timespec_t ts) {
162+
// This template has exactly two specializations below.
163+
static_assert(std::is_arithmetic<NativeT>::value == false, "Not implemented");
164+
UNREACHABLE();
165+
}
166+
167+
template <>
168+
constexpr double ToNative(uv_timespec_t ts) {
169+
// We need to do a static_cast since the original FS values are ulong.
170+
/* NOLINTNEXTLINE(runtime/int) */
171+
const auto u_sec = static_cast<unsigned long>(ts.tv_sec);
172+
const double full_sec = u_sec * 1000.0;
173+
/* NOLINTNEXTLINE(runtime/int) */
174+
const auto u_nsec = static_cast<unsigned long>(ts.tv_nsec);
175+
const double full_nsec = u_nsec / 1000'000.0;
176+
return full_sec + full_nsec;
177+
}
178+
179+
template <>
180+
constexpr uint64_t ToNative(uv_timespec_t ts) {
181+
// We need to do a static_cast since the original FS values are ulong.
182+
/* NOLINTNEXTLINE(runtime/int) */
183+
const auto u_sec = static_cast<unsigned long>(ts.tv_sec);
184+
const auto full_sec = static_cast<uint64_t>(u_sec) * 1000UL;
185+
/* NOLINTNEXTLINE(runtime/int) */
186+
const auto u_nsec = static_cast<unsigned long>(ts.tv_nsec);
187+
const auto full_nsec = static_cast<uint64_t>(u_nsec) / 1000'000UL;
188+
return full_sec + full_nsec;
189+
}
190+
191+
#undef constexpr // end N3652 bug workaround
192+
193+
template <typename NativeT, typename V8T>
194+
constexpr void FillStatsArray(AliasedBuffer<NativeT, V8T>* fields,
195+
const uv_stat_t* s, const size_t offset = 0) {
196+
fields->SetValue(offset + 0, s->st_dev);
197+
fields->SetValue(offset + 1, s->st_mode);
198+
fields->SetValue(offset + 2, s->st_nlink);
199+
fields->SetValue(offset + 3, s->st_uid);
200+
fields->SetValue(offset + 4, s->st_gid);
201+
fields->SetValue(offset + 5, s->st_rdev);
202+
#if defined(__POSIX__)
203+
fields->SetValue(offset + 6, s->st_blksize);
204+
#else
205+
fields->SetValue(offset + 6, 0);
206+
#endif
207+
fields->SetValue(offset + 7, s->st_ino);
208+
fields->SetValue(offset + 8, s->st_size);
209+
#if defined(__POSIX__)
210+
fields->SetValue(offset + 9, s->st_blocks);
211+
#else
212+
fields->SetValue(offset + 9, 0);
213+
#endif
214+
// Dates.
215+
fields->SetValue(offset + 10, ToNative<NativeT>(s->st_atim));
216+
fields->SetValue(offset + 11, ToNative<NativeT>(s->st_mtim));
217+
fields->SetValue(offset + 12, ToNative<NativeT>(s->st_ctim));
218+
fields->SetValue(offset + 13, ToNative<NativeT>(s->st_birthtim));
219+
}
220+
221+
inline Local<Value> FillGlobalStatsArray(Environment* env,
222+
const bool use_bigint,
223+
const uv_stat_t* s,
224+
const bool second = false) {
225+
const ptrdiff_t offset = second ? kFsStatsFieldsNumber : 0;
226+
if (use_bigint) {
227+
auto* const arr = env->fs_stats_field_bigint_array();
228+
FillStatsArray(arr, s, offset);
229+
return arr->GetJSArray();
230+
} else {
231+
auto* const arr = env->fs_stats_field_array();
232+
FillStatsArray(arr, s, offset);
233+
return arr->GetJSArray();
234+
}
235+
}
236+
151237
template <typename NativeT = double, typename V8T = v8::Float64Array>
152238
class FSReqPromise : public FSReqBase {
153239
public:
@@ -157,7 +243,7 @@ class FSReqPromise : public FSReqBase {
157243
->NewInstance(env->context()).ToLocalChecked(),
158244
AsyncWrap::PROVIDER_FSREQPROMISE,
159245
use_bigint),
160-
stats_field_array_(env->isolate(), env->kFsStatsFieldsLength) {
246+
stats_field_array_(env->isolate(), kFsStatsFieldsNumber) {
161247
auto resolver = Promise::Resolver::New(env->context()).ToLocalChecked();
162248
object()->Set(env->context(), env->promise_string(),
163249
resolver).FromJust();
@@ -191,7 +277,8 @@ class FSReqPromise : public FSReqBase {
191277
}
192278

193279
void ResolveStat(const uv_stat_t* stat) override {
194-
Resolve(node::FillStatsArray(&stats_field_array_, stat));
280+
FillStatsArray(&stats_field_array_, stat);
281+
Resolve(stats_field_array_.GetJSArray());
195282
}
196283

197284
void SetReturnValue(const FunctionCallbackInfo<Value>& args) override {

‎src/node_internals.h‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
#include "uv.h"
3333
#include "v8.h"
3434
#include "tracing/trace_event.h"
35-
#include "node_perf_common.h"
3635
#include "node_api.h"
3736

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

274-
template <typename NativeT, typename V8T>
275-
v8::Local<v8::Value> FillStatsArray(AliasedBuffer<NativeT, V8T>* fields_ptr,
276-
const uv_stat_t* s, int offset = 0) {
277-
AliasedBuffer<NativeT, V8T>& fields = *fields_ptr;
278-
fields[offset + 0] = s->st_dev;
279-
fields[offset + 1] = s->st_mode;
280-
fields[offset + 2] = s->st_nlink;
281-
fields[offset + 3] = s->st_uid;
282-
fields[offset + 4] = s->st_gid;
283-
fields[offset + 5] = s->st_rdev;
284-
#if defined(__POSIX__)
285-
fields[offset + 6] = s->st_blksize;
286-
#else
287-
fields[offset + 6] = 0;
288-
#endif
289-
fields[offset + 7] = s->st_ino;
290-
fields[offset + 8] = s->st_size;
291-
#if defined(__POSIX__)
292-
fields[offset + 9] = s->st_blocks;
293-
#else
294-
fields[offset + 9] = 0;
295-
#endif
296-
// Dates.
297-
// NO-LINT because the fields are 'long' and we just want to cast to `unsigned`
298-
#define X(idx, name) \
299-
/* NOLINTNEXTLINE(runtime/int) */ \
300-
fields[offset + idx] = ((unsigned long)(s->st_##name.tv_sec) * 1e3) + \
301-
/* NOLINTNEXTLINE(runtime/int) */ \
302-
((unsigned long)(s->st_##name.tv_nsec) / 1e6); \
303-
304-
X(10, atim)
305-
X(11, mtim)
306-
X(12, ctim)
307-
X(13, birthtim)
308-
#undef X
309-
310-
return fields_ptr->GetJSArray();
311-
}
312-
313-
inline v8::Local<v8::Value> FillGlobalStatsArray(Environment* env,
314-
const uv_stat_t* s,
315-
bool use_bigint = false,
316-
int offset = 0) {
317-
if (use_bigint) {
318-
return node::FillStatsArray(
319-
env->fs_stats_field_bigint_array(), s, offset);
320-
} else {
321-
return node::FillStatsArray(env->fs_stats_field_array(), s, offset);
322-
}
323-
}
324-
325273
void SetupBootstrapObject(Environment* env,
326274
v8::Local<v8::Object> bootstrapper);
327275
void SetupProcessObject(Environment* env,

‎src/node_stat_watcher.cc‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
#include "node_stat_watcher.h"
2323
#include "node_internals.h"
2424
#include "async_wrap-inl.h"
25-
#include "env-inl.h"
26-
#include "util-inl.h"
25+
#include "env.h"
26+
#include "node_file.h"
2727

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

83-
Local<Value> arr = node::FillGlobalStatsArray(env, curr,
84-
wrap->use_bigint_);
85-
node::FillGlobalStatsArray(env, prev, wrap->use_bigint_,
86-
env->kFsStatsFieldsLength);
83+
Local<Value> arr = fs::FillGlobalStatsArray(env, wrap->use_bigint_, curr);
84+
USE(fs::FillGlobalStatsArray(env, wrap->use_bigint_, prev, true));
8785

88-
Local<Value> argv[2] {
89-
Integer::New(env->isolate(), status),
90-
arr
91-
};
86+
Local<Value> argv[2] = { Integer::New(env->isolate(), status), arr };
9287
wrap->MakeCallback(env->onchange_string(), arraysize(argv), argv);
9388
}
9489

0 commit comments

Comments
 (0)