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

src: use uv_gettimeofday() #27029

Merged
merged 2 commits into from
Apr 22, 2019
Merged
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
11 changes: 7 additions & 4 deletions src/node_report.cc
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,14 @@ static void WriteNodeReport(Isolate* isolate,
tm_struct.tm_min,
tm_struct.tm_sec);
writer.json_keyvalue("dumpEventTime", timebuf);
struct timeval ts;
gettimeofday(&ts, nullptr);
writer.json_keyvalue("dumpEventTimeStamp",
std::to_string(ts.tv_sec * 1000 + ts.tv_usec / 1000));
#endif

uv_timeval64_t ts;
if (uv_gettimeofday(&ts) == 0) {
writer.json_keyvalue("dumpEventTimeStamp",
std::to_string(ts.tv_sec * 1000 + ts.tv_usec / 1000));
Copy link
Member

@bnoordhuis bnoordhuis Apr 1, 2019

Choose a reason for hiding this comment

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

This might overflow when tv_sec is a 32 bits long.

(I kind of regret that we didn't use int64_t for uv_timeval_t.tv_sec...)

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe there's still time to change uv_gettimeofday to uint64_t uv_gettimeofday() (or unsigned long long int) and always return microseconds? Seems like uv_timeval_t is a little "legacy" for this API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's not a bad idea. Since uv_gettimeofday() hasn't been released yet, there is time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can discuss at libuv/libuv#2243.

}

// Report native process ID
writer.json_keyvalue("processId", pid);

Expand Down
5 changes: 1 addition & 4 deletions src/node_report.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@
#include "uv.h"
#include "v8.h"

#ifdef _WIN32
#include <time.h>
#else
#include <sys/time.h>
#ifndef _WIN32
#include <sys/types.h>
#include <unistd.h>
#endif
Expand Down
21 changes: 4 additions & 17 deletions src/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ static std::atomic_int seq = {0}; // Sequence number for diagnostic filenames.

namespace node {

// Microseconds in a second, as a float.
#define MICROS_PER_SEC 1e6

using v8::ArrayBufferView;
using v8::Isolate;
using v8::Local;
Expand Down Expand Up @@ -166,20 +163,10 @@ void ThrowErrStringTooLong(Isolate* isolate) {
}

double GetCurrentTimeInMicroseconds() {
#ifdef _WIN32
// The difference between the Unix Epoch and the Windows Epoch in 100-ns ticks.
#define TICKS_TO_UNIX_EPOCH 116444736000000000LL
FILETIME ft;
GetSystemTimeAsFileTime(&ft);
uint64_t filetime_int =
static_cast<uint64_t>(ft.dwHighDateTime) << 32 | ft.dwLowDateTime;
// FILETIME is measured in terms of 100 ns. Convert that to 1 us (1000 ns).
return (filetime_int - TICKS_TO_UNIX_EPOCH) / 10.;
#else
struct timeval tp;
gettimeofday(&tp, nullptr);
return MICROS_PER_SEC * tp.tv_sec + tp.tv_usec;
#endif
constexpr double kMicrosecondsPerSecond = 1e6;
uv_timeval64_t tv;
CHECK_EQ(0, uv_gettimeofday(&tv));
return kMicrosecondsPerSecond * tv.tv_sec + tv.tv_usec;
}

int WriteFileSync(const char* path, uv_buf_t buf) {
Expand Down
6 changes: 1 addition & 5 deletions test/common/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,7 @@ function _validateContent(data) {
assert(typeof header.filename === 'string' || header.filename === null);
assert.notStrictEqual(new Date(header.dumpEventTime).toString(),
'Invalid Date');
if (isWindows)
assert.strictEqual(header.dumpEventTimeStamp, undefined);
else
assert(String(+header.dumpEventTimeStamp), header.dumpEventTimeStamp);

assert(String(+header.dumpEventTimeStamp), header.dumpEventTimeStamp);
assert(Number.isSafeInteger(header.processId));
assert.strictEqual(typeof header.cwd, 'string');
assert(Array.isArray(header.commandLine));
Expand Down