From fc381a3f51b0c9dc8487a2730308365a2534788d Mon Sep 17 00:00:00 2001 From: Daniel Pihlstrom Date: Sun, 23 Apr 2017 07:55:35 +0200 Subject: [PATCH 1/9] lib,src: round fs stat times to nearest ms --- lib/fs.js | 8 ++++---- src/node_file.cc | 9 ++++++--- test/parallel/test-fs-utimes.js | 26 +++++++++++++++++--------- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index ac1f90109a9eda..e899e8bc1c0f9a 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -196,10 +196,10 @@ function Stats( this.ino = ino; this.size = size; this.blocks = blocks; - this.atime = new Date(atim_msec); - this.mtime = new Date(mtim_msec); - this.ctime = new Date(ctim_msec); - this.birthtime = new Date(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); } fs.Stats = Stats; diff --git a/src/node_file.cc b/src/node_file.cc index 4a0b1527d6aca4..9e8d0572aada8d 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -71,6 +71,9 @@ using v8::Value; #define GET_OFFSET(a) ((a)->IsNumber() ? (a)->IntegerValue() : -1) +#define MILLISEC_PER_SEC 1000 +#define NANOSEC_PER_MILLISEC 1000000 + class FSReqWrap: public ReqWrap { public: enum Ownership { COPY, MOVE }; @@ -475,9 +478,9 @@ void FillStatsArray(double* fields, const uv_stat_t* s) { fields[9] = -1; #endif // Dates. -#define X(idx, name) \ - fields[idx] = (static_cast(s->st_##name.tv_sec) * 1000) + \ - (static_cast(s->st_##name.tv_nsec / 1000000)); \ +#define X(idx, name) \ + fields[idx] = (static_cast(s->st_##name.tv_sec) * MILLISEC_PER_SEC) + \ + (static_cast(s->st_##name.tv_nsec) / NANOSEC_PER_MILLISEC);\ X(10, atim) X(11, mtim) diff --git a/test/parallel/test-fs-utimes.js b/test/parallel/test-fs-utimes.js index 456f09b8041cb6..25cea2a198a582 100644 --- a/test/parallel/test-fs-utimes.js +++ b/test/parallel/test-fs-utimes.js @@ -39,12 +39,18 @@ function stat_resource(resource) { } function check_mtime(resource, mtime) { - mtime = fs._toUnixTimestamp(mtime); const stats = stat_resource(resource); - const real_mtime = fs._toUnixTimestamp(stats.mtime); - // check up to single-second precision - // sub-second precision is OS and fs dependant - return mtime - real_mtime < 2; + if (process.platform === 'win32') { + // check ms precision on windows. + return Math.floor(mtime) === Math.floor(stats.mtime); + } else { + mtime = fs._toUnixTimestamp(mtime); + const real_mtime = fs._toUnixTimestamp(stats.mtime); + + // check up to single-second precision + // sub-second precision is OS and fs dependant + return mtime - real_mtime < 2; + } } function expect_errno(syscall, resource, err, errno) { @@ -143,15 +149,17 @@ function testIt(atime, mtime, callback) { const stats = fs.statSync(__filename); // run tests -const runTest = common.mustCall(testIt, 5); +const runTest = common.mustCall(testIt, 6); runTest(new Date('1982-09-10 13:37'), new Date('1982-09-10 13:37'), function() { runTest(new Date(), new Date(), function() { runTest(123456.789, 123456.789, function() { runTest(stats.mtime, stats.mtime, function() { - runTest('123456', -1, common.mustCall(function() { - // done - })); + runTest('123456', -1, function() { + runTest(1491674378008, 1491674378008, common.mustCall(function() { + // done + })); + }); }); }); }); From 4e113b9d7cdd0c7771d1a237cf73b8b45c71774b Mon Sep 17 00:00:00 2001 From: Daniel Pihlstrom Date: Thu, 4 May 2017 11:29:01 +0200 Subject: [PATCH 2/9] use common iswindows in test --- test/parallel/test-fs-utimes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-fs-utimes.js b/test/parallel/test-fs-utimes.js index 25cea2a198a582..3c6cd05dc27919 100644 --- a/test/parallel/test-fs-utimes.js +++ b/test/parallel/test-fs-utimes.js @@ -40,7 +40,7 @@ function stat_resource(resource) { function check_mtime(resource, mtime) { const stats = stat_resource(resource); - if (process.platform === 'win32') { + if (common.isWindows) { // check ms precision on windows. return Math.floor(mtime) === Math.floor(stats.mtime); } else { From a474c1a8ba206ed1f69b2d9c447e3577bdfab0fc Mon Sep 17 00:00:00 2001 From: Daniel Pihlstrom Date: Thu, 4 May 2017 11:34:05 +0200 Subject: [PATCH 3/9] scientific notation for stat times --- src/node_file.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index 9e8d0572aada8d..51d5fa3012a1c4 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -71,9 +71,6 @@ using v8::Value; #define GET_OFFSET(a) ((a)->IsNumber() ? (a)->IntegerValue() : -1) -#define MILLISEC_PER_SEC 1000 -#define NANOSEC_PER_MILLISEC 1000000 - class FSReqWrap: public ReqWrap { public: enum Ownership { COPY, MOVE }; @@ -479,8 +476,8 @@ void FillStatsArray(double* fields, const uv_stat_t* s) { #endif // Dates. #define X(idx, name) \ - fields[idx] = (static_cast(s->st_##name.tv_sec) * MILLISEC_PER_SEC) + \ - (static_cast(s->st_##name.tv_nsec) / NANOSEC_PER_MILLISEC);\ + fields[idx] = (s->st_##name.tv_sec * 1e3) + \ + (s->st_##name.tv_nsec / 1e6); \ X(10, atim) X(11, mtim) From 3e5afc1ab969d11801f2ea5b9bc3d38fc1674649 Mon Sep 17 00:00:00 2001 From: Daniel Pihlstrom Date: Thu, 4 May 2017 16:58:43 +0200 Subject: [PATCH 4/9] test - use tounixtimestamp instead of floor --- src/node_file.cc | 2 +- test/parallel/test-fs-utimes.js | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index 51d5fa3012a1c4..8d611e23532a80 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -475,7 +475,7 @@ void FillStatsArray(double* fields, const uv_stat_t* s) { fields[9] = -1; #endif // Dates. -#define X(idx, name) \ +#define X(idx, name) \ fields[idx] = (s->st_##name.tv_sec * 1e3) + \ (s->st_##name.tv_nsec / 1e6); \ diff --git a/test/parallel/test-fs-utimes.js b/test/parallel/test-fs-utimes.js index 3c6cd05dc27919..6c051afdb953f1 100644 --- a/test/parallel/test-fs-utimes.js +++ b/test/parallel/test-fs-utimes.js @@ -40,13 +40,12 @@ function stat_resource(resource) { function check_mtime(resource, mtime) { const stats = stat_resource(resource); + mtime = fs._toUnixTimestamp(mtime); + const real_mtime = fs._toUnixTimestamp(stats.mtime); if (common.isWindows) { // check ms precision on windows. - return Math.floor(mtime) === Math.floor(stats.mtime); + return mtime === real_mtime; } else { - mtime = fs._toUnixTimestamp(mtime); - const real_mtime = fs._toUnixTimestamp(stats.mtime); - // check up to single-second precision // sub-second precision is OS and fs dependant return mtime - real_mtime < 2; From 519150d05fbe39e33f63d551ecc0288cfb94ba7c Mon Sep 17 00:00:00 2001 From: Daniel Pihlstrom Date: Fri, 5 May 2017 03:08:05 +0200 Subject: [PATCH 5/9] swap lines back, no reason for why i swapped them to begin with --- test/parallel/test-fs-utimes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-fs-utimes.js b/test/parallel/test-fs-utimes.js index 6c051afdb953f1..b7ee5f3af8261a 100644 --- a/test/parallel/test-fs-utimes.js +++ b/test/parallel/test-fs-utimes.js @@ -39,8 +39,8 @@ function stat_resource(resource) { } function check_mtime(resource, mtime) { - const stats = stat_resource(resource); mtime = fs._toUnixTimestamp(mtime); + const stats = stat_resource(resource); const real_mtime = fs._toUnixTimestamp(stats.mtime); if (common.isWindows) { // check ms precision on windows. From a6268a80bc33549535c0e60b5dc38fc17ebc1c2f Mon Sep 17 00:00:00 2001 From: Daniel Pihlstrom Date: Fri, 5 May 2017 03:13:22 +0200 Subject: [PATCH 6/9] use string to create new test case to fix x86 overflow --- test/parallel/test-fs-utimes.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-fs-utimes.js b/test/parallel/test-fs-utimes.js index b7ee5f3af8261a..cffedca04e613a 100644 --- a/test/parallel/test-fs-utimes.js +++ b/test/parallel/test-fs-utimes.js @@ -155,9 +155,13 @@ runTest(new Date('1982-09-10 13:37'), new Date('1982-09-10 13:37'), function() { runTest(123456.789, 123456.789, function() { runTest(stats.mtime, stats.mtime, function() { runTest('123456', -1, function() { - runTest(1491674378008, 1491674378008, common.mustCall(function() { - // done - })); + runTest( + new Date('2017-04-08T17:59:38.008Z'), + new Date('2017-04-08T17:59:38.008Z'), + common.mustCall(function() { + // done + }) + ); }); }); }); From ac3bfa5acc84b29a154e690cfb65548c1e4bfa1a Mon Sep 17 00:00:00 2001 From: Daniel Pihlstrom Date: Fri, 5 May 2017 22:10:04 +0200 Subject: [PATCH 7/9] use (Date.now()/1000) instead of -1 to test utimes --- test/parallel/test-fs-utimes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-fs-utimes.js b/test/parallel/test-fs-utimes.js index cffedca04e613a..51714df9d1e5ce 100644 --- a/test/parallel/test-fs-utimes.js +++ b/test/parallel/test-fs-utimes.js @@ -154,7 +154,7 @@ runTest(new Date('1982-09-10 13:37'), new Date('1982-09-10 13:37'), function() { runTest(new Date(), new Date(), function() { runTest(123456.789, 123456.789, function() { runTest(stats.mtime, stats.mtime, function() { - runTest('123456', -1, function() { + runTest('123456', (Date.now()/1000), function() { runTest( new Date('2017-04-08T17:59:38.008Z'), new Date('2017-04-08T17:59:38.008Z'), From bad1f6f21d05f2988d65db2133bfa8b631893d97 Mon Sep 17 00:00:00 2001 From: Daniel Pihlstrom Date: Fri, 5 May 2017 22:37:04 +0200 Subject: [PATCH 8/9] restore -1 test and allow up to 2 seconds diff in test results --- test/parallel/test-fs-utimes.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/test/parallel/test-fs-utimes.js b/test/parallel/test-fs-utimes.js index 51714df9d1e5ce..f5eaf282f59ad0 100644 --- a/test/parallel/test-fs-utimes.js +++ b/test/parallel/test-fs-utimes.js @@ -42,14 +42,9 @@ function check_mtime(resource, mtime) { mtime = fs._toUnixTimestamp(mtime); const stats = stat_resource(resource); const real_mtime = fs._toUnixTimestamp(stats.mtime); - if (common.isWindows) { - // check ms precision on windows. - return mtime === real_mtime; - } else { - // check up to single-second precision - // sub-second precision is OS and fs dependant - return mtime - real_mtime < 2; - } + // check up to single-second precision + // sub-second precision is OS and fs dependant + return mtime - real_mtime < 2; } function expect_errno(syscall, resource, err, errno) { @@ -154,7 +149,7 @@ runTest(new Date('1982-09-10 13:37'), new Date('1982-09-10 13:37'), function() { runTest(new Date(), new Date(), function() { runTest(123456.789, 123456.789, function() { runTest(stats.mtime, stats.mtime, function() { - runTest('123456', (Date.now()/1000), function() { + runTest('123456', -1, function() { runTest( new Date('2017-04-08T17:59:38.008Z'), new Date('2017-04-08T17:59:38.008Z'), From 32c1639d21ce35d6860a87b5f33024e828fc95e7 Mon Sep 17 00:00:00 2001 From: Daniel Pihlstrom Date: Thu, 25 May 2017 12:46:38 +0200 Subject: [PATCH 9/9] add comment explaining +0.5 --- lib/fs.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/fs.js b/lib/fs.js index c50788a03dbd68..99ff5c1843e200 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -206,6 +206,8 @@ fs.Stats = Stats; // defining the properties in this fashion (explicitly with no loop or factory) // has been shown to be the most performant on V8 contemp. // Ref: https://github.com/nodejs/node/pull/12818 +// +0.5 is added to the Dates to protect values from being rounded down +// Ref: https://github.com/nodejs/node/pull/12607 Object.defineProperties(Stats.prototype, { atime: { configurable: true,