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.

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

Jump to bottom

win, util: rearrange uv_hrtime #2866

Closed
wants to merge 1 commit into from
Closed

win, util: rearrange uv_hrtime #2866

wants to merge 1 commit into from

Conversation

bzoz
Copy link
Member

@bzoz bzoz commented May 25, 2020

Rearrange math operations in uv_hrtime. This is a workaround for a probable compiler bug in VS2019.

Fixes: #1633

Big thanks to @saschanaz for substantial help in solving this issue.

@bzoz
win, util: rearrange uv_hrtime
d775340 
Rearrange math operations in uv_hrtime. This is a workaround for a
probable compiler bug in VS2019.

Fixes: libuv#1633
@bzoz
Copy link
Member Author

bzoz commented May 25, 2020

CI: https://ci.nodejs.org/view/libuv/job/libuv-test-commit/1918/

@addaleax addaleax mentioned this pull request May 25, 2020
Closed
@jebej
Copy link

jebej commented May 25, 2020

Would it also work doing just a single division?

double scaled_interval = (double) scale / hrtime_frequency_;
double result = (double) counter.QuadPart * scaled_interval;

@bzoz
Copy link
Member Author

bzoz commented May 25, 2020

hrtime_frequency_ is usually a multiple of scale, so scaled_freq ends up being an integer.

bnoordhuis
bnoordhuis approved these changes May 25, 2020
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

Well, that's one cause I never would have expected... LGTM and lets get this released.

bzoz added a commit that referenced this pull request May 25, 2020
@bzoz
win, util: rearrange uv_hrtime
7967448 
Rearrange math operations in uv_hrtime. This is a workaround for a
probable compiler bug in VS2019.

Fixes: #1633

PR-URL: #2866
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@bzoz
Copy link
Member Author

bzoz commented May 25, 2020

Landed in 7967448

@bzoz bzoz closed this May 25, 2020
@bzoz
Copy link
Member Author

bzoz commented Jun 15, 2020

@cjihrig this and #2882 fix some nasty bugs in libuv on Windows, could we make a release soon?

@richardlau richardlau mentioned this pull request Jul 31, 2021
Closed
JeffroMF pushed a commit to JeffroMF/libuv that referenced this pull request May 16, 2022
@bzoz @JeffroMF
win, util: rearrange uv_hrtime
36f49db 
Rearrange math operations in uv_hrtime. This is a workaround for a
probable compiler bug in VS2019.

Fixes: libuv#1633

PR-URL: libuv#2866
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@vtjnash vtjnash mentioned this pull request Dec 6, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@addaleax addaleax addaleax approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Assertion failed: new_time >= loop->time
4 participants
@bzoz @jebej @bnoordhuis @addaleax