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

precise_time_ns can overflow on Windows #17845

Closed
rusty-nail2 opened this issue Oct 7, 2014 · 7 comments · Fixed by #22788
Closed

precise_time_ns can overflow on Windows #17845

rusty-nail2 opened this issue Oct 7, 2014 · 7 comments · Fixed by #22788
Labels
O-windows Operating system: Windows

Comments

@rusty-nail2
Copy link

The last line of precise_time_ns uses a 64 bit multiplication that will overflow frequently.

Using floats would be the easy way out, but it would reduce precision to 53 bits.

@Gankra
Copy link
Contributor

Gankra commented Oct 7, 2014

As a datapoint, this is literally what microsoft itself recommends doing.

LARGE_INTEGER StartingTime, EndingTime, ElapsedMicroseconds;
LARGE_INTEGER Frequency;

QueryPerformanceFrequency(&Frequency); 
QueryPerformanceCounter(&StartingTime);

// Activity to be timed

QueryPerformanceCounter(&EndingTime);
ElapsedMicroseconds.QuadPart = EndingTime.QuadPart - StartingTime.QuadPart;


//
// We now have the elapsed number of ticks, along with the
// number of ticks-per-second. We use these values
// to convert to the number of elapsed microseconds.
// To guard against loss-of-precision, we convert
// to microseconds *before* dividing by ticks-per-second.
//

ElapsedMicroseconds.QuadPart *= 1000000;
ElapsedMicroseconds.QuadPart /= Frequency.QuadPart;

Where LARGE_INTEGER is a u64.

But we're multiplying by an extra 1000.

@rusty-nail2
Copy link
Author

In fairness to Microsoft, they're operating on a time difference computed mod 2^64, so as long as the interval length is reasonable (less than a few thousand hours for a typical tick rate) it can't overflow.

But precise_time_ns computes an absolute time rather than a difference, so overflows are guaranteed to happen at some point. The 1000x factor only makes it more noticeable.

@kmcallister kmcallister added O-windows Operating system: Windows A-libs labels Oct 7, 2014
@Gankra
Copy link
Contributor

Gankra commented Oct 8, 2014

Since we're dealing with absolute time (and therefore something large), it's probably not unreasonable to divide first, or maybe even partially multiply by 1000000000. Something like:

ticks * 1000 / ticks_per_s * 1000000

maybe? What are "typical" and "extreme" tick rates and absolute ticks like? An incredibly robust implementation could presumably branch on the size of ticks to determine the optimal split of the 1000000000.

@nodakai
Copy link
Contributor

nodakai commented Oct 9, 2014

The x86_64 architecture has the 64 bit MUL instruction whose result is effectively a 128 bit integer which never overflows. Likewise, the DIV instruction takes an effectively 128 bit integer as the dividened.

asm!("mulq $2; divq $3" : "={rax}"(d) : "{rax}"(a), "{rdi}"(b), "{rsi}"(c) : "%rdx" );

i386 offers (u32, u32) -> u64 MUL and (u64, u32) -> (u32, u32) DIV/MOD, so we should be able to implement 64 bit MUL and DIV without overflow.

Windows RT runs on ARMv7 (=~ Cortex-A). It has (u32, u32) -> u64 UMULL but lacks DIV and we have to call a division routine. Perhaps we can forget about it for now...

@vadimcn
Copy link
Contributor

vadimcn commented Nov 25, 2014

We could use techniques similar to division by a constant via multiplication and shifts.
Of course, in this case, we also have the 1000000000 multiplier, which together with the "magic constant" is going to overflow even a 128 bit register, but multiplication can be done "in parts".

@vadimcn
Copy link
Contributor

vadimcn commented Feb 24, 2015

Don't OSX and iOS have a similar problem here and here?
What is the typical value of info.numer? This seems to suggest that is can be large, just like on Windows.

@vadimcn
Copy link
Contributor

vadimcn commented Feb 24, 2015

@pcwalton ^

steveklabnik added a commit to steveklabnik/rust that referenced this issue Feb 26, 2015
…elix

which starts happening after ~2 hours of machine uptime.
Closes rust-lang#17845
Manishearth added a commit to Manishearth/rust that referenced this issue Feb 27, 2015
…elix

 which starts happening after ~2 hours of machine uptime.
Closes rust-lang#17845
lnicola pushed a commit to lnicola/rust that referenced this issue Aug 13, 2024
feat: Implement TAIT and fix ATPIT a bit

Closes rust-lang#16296 (Commented on the issue)

In rust-lang#16852, I implemented ATPIT, but as I didn't discern ATPIT and other non-assoc TAIT, I guess that it has been working for some TAITs.

As the definining usage of TAIT requires it should be appear in the Def body's type(const blocks' type annotations or functions' signatures), this can be done in simlilar way with ATPIT

And this PR also corrects some defining-usage resolution for ATPIT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows Operating system: Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants