Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: add *timeNs properties to BigInt Stats objects #21387

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 67 additions & 4 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,8 @@ A `fs.Stats` object provides information about a file.
Objects returned from [`fs.stat()`][], [`fs.lstat()`][] and [`fs.fstat()`][] and
their synchronous counterparts are of this type.
If `bigint` in the `options` passed to those methods is true, the numeric values
will be `bigint` instead of `number`.
will be `bigint` instead of `number`, and the object will contain additional
nanosecond-precision properties suffixed with `Ns`.

```console
Stats {
Expand All @@ -539,7 +540,7 @@ Stats {
`bigint` version:

```console
Stats {
BigIntStats {
dev: 2114n,
ino: 48064969n,
mode: 33188n,
Expand All @@ -554,6 +555,10 @@ Stats {
mtimeMs: 1318289051000n,
ctimeMs: 1318289051000n,
birthtimeMs: 1318289051000n,
atimeNs: 1318289051000000000n,
mtimeNs: 1318289051000000000n,
ctimeNs: 1318289051000000000n,
birthtimeNs: 1318289051000000000n,
atime: Mon, 10 Oct 2011 23:24:11 GMT,
mtime: Mon, 10 Oct 2011 23:24:11 GMT,
ctime: Mon, 10 Oct 2011 23:24:11 GMT,
Expand Down Expand Up @@ -726,6 +731,54 @@ added: v8.1.0
The timestamp indicating the creation time of this file expressed in
milliseconds since the POSIX Epoch.

### stats.atimeNs
<!-- YAML
added: REPLACEME
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be changed, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'll only be available when this is released.

-->

* {bigint}

Only present when `bigint: true` is passed into the method that generates
the object.
The timestamp indicating the last time this file was accessed expressed in
nanoseconds since the POSIX Epoch.

### stats.mtimeNs
<!-- YAML
added: REPLACEME
-->

* {bigint}

Only present when `bigint: true` is passed into the method that generates
the object.
The timestamp indicating the last time this file was modified expressed in
nanoseconds since the POSIX Epoch.

### stats.ctimeNs
<!-- YAML
added: REPLACEME
-->

* {bigint}

Only present when `bigint: true` is passed into the method that generates
the object.
The timestamp indicating the last time the file status was changed expressed
in nanoseconds since the POSIX Epoch.

### stats.birthtimeNs
<!-- YAML
added: REPLACEME
-->

* {bigint}

Only present when `bigint: true` is passed into the method that generates
the object.
The timestamp indicating the creation time of this file expressed in
nanoseconds since the POSIX Epoch.

### stats.atime
<!-- YAML
added: v0.11.13
Expand Down Expand Up @@ -765,8 +818,17 @@ The timestamp indicating the creation time of this file.
### Stat Time Values

The `atimeMs`, `mtimeMs`, `ctimeMs`, `birthtimeMs` properties are
[numbers][MDN-Number] that hold the corresponding times in milliseconds. Their
precision is platform specific. `atime`, `mtime`, `ctime`, and `birthtime` are
numeric values that hold the corresponding times in milliseconds. Their
precision is platform specific. When `bigint: true` is passed into the
method that generates the object, the properties will be [bigints][],
otherwise they will be [numbers][MDN-Number].

The `atimeNs`, `mtimeNs`, `ctimeNs`, `birthtimeNs` properties are
[bigints][] that hold the corresponding times in nanoseconds. They are
only present when `bigint: true` is passed into the method that generates
the object. Their precision is platform specific.

`atime`, `mtime`, `ctime`, and `birthtime` are
[`Date`][MDN-Date] object alternate representations of the various times. The
`Date` and number values are not connected. Assigning a new number value, or
mutating the `Date` value, will not be reflected in the corresponding alternate
Expand Down Expand Up @@ -4976,6 +5038,7 @@ the file contents.
[`net.Socket`]: net.html#net_class_net_socket
[`stat()`]: fs.html#fs_fs_stat_path_options_callback
[`util.promisify()`]: util.html#util_util_promisify_original
[bigints]: https://tc39.github.io/proposal-bigint
[Caveats]: #fs_caveats
[Common System Errors]: errors.html#errors_common_system_errors
[FS Constants]: #fs_fs_constants_1
Expand Down
163 changes: 108 additions & 55 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const { Reflect } = primordials;
const { Object, Reflect } = primordials;

const { Buffer, kMaxLength } = require('buffer');
const {
Expand All @@ -16,7 +16,8 @@ const {
} = require('internal/errors');
const {
isUint8Array,
isDate
isDate,
isBigUint64Array
} = require('internal/util/types');
const { once } = require('internal/util');
const { toPathIfFileURL } = require('internal/url');
Expand Down Expand Up @@ -230,27 +231,9 @@ function preprocessSymlinkDestination(path, type, linkPath) {
}
}

function dateFromNumeric(num) {
return new Date(Number(num) + 0.5);
}

// Constructor for file stats.
function Stats(
dev,
mode,
nlink,
uid,
gid,
rdev,
blksize,
ino,
size,
blocks,
atim_msec,
mtim_msec,
ctim_msec,
birthtim_msec
) {
function StatsBase(dev, mode, nlink, uid, gid, rdev, blksize,
ino, size, blocks) {
this.dev = dev;
this.mode = mode;
this.nlink = nlink;
Expand All @@ -261,63 +244,132 @@ function Stats(
this.ino = ino;
this.size = size;
this.blocks = blocks;
this.atimeMs = atim_msec;
this.mtimeMs = mtim_msec;
this.ctimeMs = ctim_msec;
this.birthtimeMs = birthtim_msec;
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) {
if (isWindows && (property === S_IFIFO || property === S_IFBLK ||
property === S_IFSOCK)) {
return false; // Some types are not available on Windows
}
if (typeof this.mode === 'bigint') { // eslint-disable-line valid-typeof
return (this.mode & BigInt(S_IFMT)) === BigInt(property);
}
return (this.mode & S_IFMT) === property;
};

Stats.prototype.isDirectory = function() {
StatsBase.prototype.isDirectory = function() {
return this._checkModeProperty(S_IFDIR);
};

Stats.prototype.isFile = function() {
StatsBase.prototype.isFile = function() {
return this._checkModeProperty(S_IFREG);
};

Stats.prototype.isBlockDevice = function() {
StatsBase.prototype.isBlockDevice = function() {
return this._checkModeProperty(S_IFBLK);
};

Stats.prototype.isCharacterDevice = function() {
StatsBase.prototype.isCharacterDevice = function() {
return this._checkModeProperty(S_IFCHR);
};

Stats.prototype.isSymbolicLink = function() {
StatsBase.prototype.isSymbolicLink = function() {
return this._checkModeProperty(S_IFLNK);
};

Stats.prototype.isFIFO = function() {
StatsBase.prototype.isFIFO = function() {
return this._checkModeProperty(S_IFIFO);
};

Stats.prototype.isSocket = function() {
StatsBase.prototype.isSocket = function() {
return this._checkModeProperty(S_IFSOCK);
};

const kNsPerMsBigInt = 10n ** 6n;
const kNsPerSecBigInt = 10n ** 9n;
const kMsPerSec = 10 ** 3;
const kNsPerMs = 10 ** 6;
function msFromTimeSpec(sec, nsec) {
return sec * kMsPerSec + nsec / kNsPerMs;
}

function nsFromTimeSpecBigInt(sec, nsec) {
return sec * kNsPerSecBigInt + nsec;
}

function dateFromMs(ms) {
return new Date(Number(ms) + 0.5);
}

function BigIntStats(dev, mode, nlink, uid, gid, rdev, blksize,
ino, size, blocks,
atimeNs, mtimeNs, ctimeNs, birthtimeNs) {
StatsBase.call(this, dev, mode, nlink, uid, gid, rdev, blksize,
ino, size, blocks);

this.atimeMs = atimeNs / kNsPerMsBigInt;
this.mtimeMs = mtimeNs / kNsPerMsBigInt;
this.ctimeMs = ctimeNs / kNsPerMsBigInt;
this.birthtimeMs = birthtimeNs / kNsPerMsBigInt;
this.atimeNs = atimeNs;
this.mtimeNs = mtimeNs;
this.ctimeNs = ctimeNs;
this.birthtimeNs = birthtimeNs;
this.atime = dateFromMs(this.atimeMs);
this.mtime = dateFromMs(this.mtimeMs);
this.ctime = dateFromMs(this.ctimeMs);
this.birthtime = dateFromMs(this.birthtimeMs);
}

Object.setPrototypeOf(BigIntStats.prototype, StatsBase.prototype);
Object.setPrototypeOf(BigIntStats, StatsBase);

BigIntStats.prototype._checkModeProperty = function(property) {
if (isWindows && (property === S_IFIFO || property === S_IFBLK ||
property === S_IFSOCK)) {
return false; // Some types are not available on Windows
}
return (this.mode & BigInt(S_IFMT)) === BigInt(property);
};

function Stats(dev, mode, nlink, uid, gid, rdev, blksize,
ino, size, blocks,
atimeMs, mtimeMs, ctimeMs, birthtimeMs) {
StatsBase.call(this, dev, mode, nlink, uid, gid, rdev, blksize,
ino, size, blocks);
this.atimeMs = atimeMs;
this.mtimeMs = mtimeMs;
this.ctimeMs = ctimeMs;
this.birthtimeMs = birthtimeMs;
this.atime = dateFromMs(atimeMs);
this.mtime = dateFromMs(mtimeMs);
this.ctime = dateFromMs(ctimeMs);
this.birthtime = dateFromMs(birthtimeMs);
}

Object.setPrototypeOf(Stats.prototype, StatsBase.prototype);
Object.setPrototypeOf(Stats, StatsBase);

Stats.prototype._checkModeProperty = function(property) {
if (isWindows && (property === S_IFIFO || property === S_IFBLK ||
property === S_IFSOCK)) {
return false; // Some types are not available on Windows
}
return (this.mode & S_IFMT) === property;
};

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], // blksize
stats[7 + offset], stats[8 + offset],
stats[9 + offset], // blocks
stats[10 + offset], stats[11 + offset],
stats[12 + offset], stats[13 + offset]);
if (isBigUint64Array(stats)) {
return new BigIntStats(
stats[0 + offset], stats[1 + offset], stats[2 + offset],
stats[3 + offset], stats[4 + offset], stats[5 + offset],
stats[6 + offset], stats[7 + offset], stats[8 + offset],
stats[9 + offset],
nsFromTimeSpecBigInt(stats[10 + offset], stats[11 + offset]),
nsFromTimeSpecBigInt(stats[12 + offset], stats[13 + offset]),
nsFromTimeSpecBigInt(stats[14 + offset], stats[15 + offset]),
nsFromTimeSpecBigInt(stats[16 + offset], stats[17 + offset])
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else would a touch clearer

Copy link
Member Author

@joyeecheung joyeecheung Jun 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fishrock123 I think we prefer leaving out the else block for early returns? There used to a PR adding a lint rule for that although it didn't land due to the amount of churn.

return new Stats(
stats[0 + offset], stats[1 + offset], stats[2 + offset],
stats[3 + offset], stats[4 + offset], stats[5 + offset],
stats[6 + offset], stats[7 + offset], stats[8 + offset],
stats[9 + offset],
msFromTimeSpec(stats[10 + offset], stats[11 + offset]),
Copy link
Member Author

@joyeecheung joyeecheung Jun 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't pass a time spec into the Stats constructor as that would be a breaking change since Stats is public. We may try using some factory function to construct Stats internally from time specs later, and try adding the Ns properties to normal Stats later, though, but there will be some precision losses without bigint and that's not too different from multiplying the Ms ones with 1000

msFromTimeSpec(stats[12 + offset], stats[13 + offset]),
msFromTimeSpec(stats[14 + offset], stats[15 + offset]),
msFromTimeSpec(stats[16 + offset], stats[17 + offset])
);
}

function stringToFlags(flags) {
Expand Down Expand Up @@ -453,6 +505,7 @@ function warnOnNonPortableTemplate(template) {

module.exports = {
assertEncoding,
BigIntStats, // for testing
copyObject,
Dirent,
getDirents,
Expand Down
26 changes: 24 additions & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,32 @@ struct PackageConfig {
};
} // namespace loader

enum class FsStatsOffset {
kDev = 0,
kMode,
kNlink,
kUid,
kGid,
kRdev,
kBlkSize,
kIno,
kSize,
kBlocks,
kATimeSec,
kATimeNsec,
kMTimeSec,
kMTimeNsec,
kCTimeSec,
kCTimeNsec,
kBirthTimeSec,
kBirthTimeNsec,
kFsStatsFieldsNumber
};

// 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;
constexpr size_t kFsStatsBufferLength =
static_cast<size_t>(FsStatsOffset::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
Expand Down
11 changes: 7 additions & 4 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2197,10 +2197,13 @@ void Initialize(Local<Object> target,

env->SetMethod(target, "mkdtemp", Mkdtemp);

target->Set(context,
FIXED_ONE_BYTE_STRING(isolate, "kFsStatsFieldsNumber"),
Integer::New(isolate, kFsStatsFieldsNumber))
.Check();
target
->Set(context,
FIXED_ONE_BYTE_STRING(isolate, "kFsStatsFieldsNumber"),
Integer::New(
isolate,
static_cast<int32_t>(FsStatsOffset::kFsStatsFieldsNumber)))
.Check();

target->Set(context,
FIXED_ONE_BYTE_STRING(isolate, "statValues"),
Expand Down
Loading