This repository has been archived by the owner. It is now read-only.

Fix uv_hrtime() for win32 #864

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants
@witchu

witchu commented Jul 25, 2013

sometimes the calculation less than expected.

@Nodejs-Jenkins

This comment has been minimized.

Show comment Hide comment
@Nodejs-Jenkins

Nodejs-Jenkins Jul 25, 2013

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit witchu/libuv@1a39914af186f5db73ba8507d60da5d3bb3e39dc has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Commit witchu/libuv@f4207f1b0684f5d0b81039cae49c25a5c91905c6 has the following error(s):

  • First line of commit message must be no longer than 50 characters

The following commiters were not found in the CLA:

  • Witchu Promjunyakul

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit witchu/libuv@1a39914af186f5db73ba8507d60da5d3bb3e39dc has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Commit witchu/libuv@f4207f1b0684f5d0b81039cae49c25a5c91905c6 has the following error(s):

  • First line of commit message must be no longer than 50 characters

The following commiters were not found in the CLA:

  • Witchu Promjunyakul

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

- (((uint64_t) counter.HighPart * NANOSEC / hrtime_frequency_)
- << 32);
+ (((uint64_t) counter.HighPart * NANOSEC / hrtime_frequency_) << 32) +
+ (((uint64_t) counter.HighPart * NANOSEC % hrtime_frequency_) * 4294967296 / hrtime_frequency_);

This comment has been minimized.

Show comment Hide comment
@bnoordhuis

bnoordhuis Jul 25, 2013

Contributor

Long line, wrap at 80 columns. Secondly, for the sake of consistency, is there a reason to multiply rather than shift? Thirdly, for the sake of clarity, it'd be better to cache the common part of the calculation in a a variable.

@bnoordhuis

bnoordhuis Jul 25, 2013

Contributor

Long line, wrap at 80 columns. Secondly, for the sake of consistency, is there a reason to multiply rather than shift? Thirdly, for the sake of clarity, it'd be better to cache the common part of the calculation in a a variable.

@piscisaureus

This comment has been minimized.

Show comment Hide comment
@piscisaureus

piscisaureus Sep 7, 2013

Member

Of course if there is a bug in uv_hrtime() it's important and we shouldn't leave this pending for so long. However I am very curious what values cause the computation to overflow.

@witchu Are you seeing problems with this on a real-world system? If so, could you share the performance counter and frequency values you're seeing? Also, out of curiosity, what kind of hardware is it?

Member

piscisaureus commented Sep 7, 2013

Of course if there is a bug in uv_hrtime() it's important and we shouldn't leave this pending for so long. However I am very curious what values cause the computation to overflow.

@witchu Are you seeing problems with this on a real-world system? If so, could you share the performance counter and frequency values you're seeing? Also, out of curiosity, what kind of hardware is it?

@witchu

This comment has been minimized.

Show comment Hide comment
@witchu

witchu Sep 8, 2013

I found the problem when it run on my windows server. The performance
frequency go very high.
On Sep 8, 2013 1:47 AM, "Bert Belder" notifications@github.com wrote:

Of course if there is a bug in uv_hrtime() it's important and we shouldn't
leave this pending for so long. However I am very curious what values cause
the computation to overflow.

@witchu https://github.com/witchu Are you seeing problems with this in
a real-world system? If so, could you share the performance counter and
-frequency you're seeing? Also, out of curiosity, what kind of hardware is
it?


Reply to this email directly or view it on GitHubhttps://github.com/joyent/libuv/pull/864#issuecomment-24007240
.

witchu commented Sep 8, 2013

I found the problem when it run on my windows server. The performance
frequency go very high.
On Sep 8, 2013 1:47 AM, "Bert Belder" notifications@github.com wrote:

Of course if there is a bug in uv_hrtime() it's important and we shouldn't
leave this pending for so long. However I am very curious what values cause
the computation to overflow.

@witchu https://github.com/witchu Are you seeing problems with this in
a real-world system? If so, could you share the performance counter and
-frequency you're seeing? Also, out of curiosity, what kind of hardware is
it?


Reply to this email directly or view it on GitHubhttps://github.com/joyent/libuv/pull/864#issuecomment-24007240
.

@saghul

This comment has been minimized.

Show comment Hide comment
@saghul

saghul Feb 12, 2014

Contributor

@piscisaureus is this still relevant?

Contributor

saghul commented Feb 12, 2014

@piscisaureus is this still relevant?

@saghul

This comment has been minimized.

Show comment Hide comment
@saghul

saghul Jun 19, 2014

Contributor

No follow-up, closing.

Contributor

saghul commented Jun 19, 2014

No follow-up, closing.

@saghul saghul closed this Jun 19, 2014

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.