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

Stop compensating for early Timer in libuv #57264

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jakobnissen
Copy link
Contributor

It appears to have been fixed on the libuv side, so now Julia's timers are one millisecond too slow.

Closes #57263

It appears to have been fixed on the libuv side, so now Julia's timers are
one millisecond too slow.
@Keno
Copy link
Member

Keno commented Feb 5, 2025

Do we speculate this is fixed or do we know that it is fixed? The reason I ask is because the argument is supposed to be a hard floor, so if it wasn't fixed, this would give incorrect behavior (timing out late is suboptimal but not incorrect according to the spec of the function).

@jakobnissen
Copy link
Contributor Author

I only speculate, after seeing on my computer that Julia's timer is 1 ms too long. I'll try to look into libuv and see if I can find anything.

@vtjnash
Copy link
Member

vtjnash commented Feb 5, 2025

It was introduced by 7310f55, I think as a workaround because of problems with the original implementation of sleep rounding timeouts down (471828a)?

@martinholters
Copy link
Member

It seems that from e5496e0 (#49937) on, the extra 1ms is no longer needed, at least on my system. So indeed, there might have happened a fix in libuv, somewhere between 1.44.2 and 1.48.0. Would still be good to identify the fix over there and verify it is robust and cross-platform.

@vtjnash
Copy link
Member

vtjnash commented Feb 6, 2025

  • win,unix: change execution order of timers (Trevor Norris)
    Perhaps? Nothing else mentions timers that I see

@inkydragon inkydragon added the external dependencies Involves LLVM, OpenBLAS, or other linked libraries label Feb 7, 2025
@martinholters
Copy link
Member

Bisecting libuv brought me to libuv/libuv@00357f8 (libuv/libuv#4026), which BTW is not contained in any libuv releases, yet. Someone else (@vtjnash ?) has to judge whether that's a proper, reliable fix for the timer-firing-early bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external dependencies Involves LLVM, OpenBLAS, or other linked libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timer is ~1ms too slow
5 participants