Skip to content

src: use uv_gettimeofday() #27029

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.

Sign up for GitHub

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

Merged
cjihrig merged 2 commits into from
Apr 22, 2019
Merged

src: use uv_gettimeofday() #27029

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af35d40
report: use uv_gettimeofday for dumpEventTimeStamp
cjihrig Mar 31, 2019
8e1e994
src: use uv_gettimeofday() to get microseconds
cjihrig Apr 15, 2019
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
Next Next commit
report: use uv_gettimeofday for dumpEventTimeStamp 
dumpEventTimeStamp was not implemented on Windows, and did not
include any error checking. This commit adds Windows support
and error checking.

PR-URL: #27029
Reviewed-By: Refael Ackermann <refack@gmail.com>
  • Loading branch information
@cjihrig
cjihrig committed Apr 22, 2019
commit af35d4044fd32922ce784afa6da98520ffbbf872
11 changes: 7 additions & 4 deletions src/node_report.cc
View file
Open in desktop
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
Copy Markdown
Member

@bnoordhuis bnoordhuis Apr 1, 2019 •
edited
Loading

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
Copy Markdown
Contributor

@refack refack Apr 1, 2019

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
Copy Markdown
Contributor Author

@cjihrig cjihrig Apr 1, 2019

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
Copy Markdown
Contributor Author

@cjihrig cjihrig Apr 1, 2019

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
View file
Open in desktop
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
6 changes: 1 addition & 5 deletions test/common/report.js
View file
Open in desktop
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