measureme icon indicating copy to clipboard operation
measureme copied to clipboard

stack_collapse unit tests fail on Windows

Open michaelwoerister opened this issue 4 years ago • 3 comments

For some reason the unit tests in stack_collapse.rs fail on Windows (at least for me locally), even though they don't seem to contain anything platform specific.

cc @wesleywiser

michaelwoerister avatar Aug 26 '21 09:08 michaelwoerister

Turns out the issue is that Windows SystemTime does not support time intervals smaller than 100 nanoseconds. The tests are using intervals much smaller than that and when we add a duration to system times that are smaller than 100 nanoseconds, you'll end up not actually adding any time (i.e., the system time you get after the addition is the same as the one before).

You can see where we divide nanoseconds into 100 nanosecond chunks in stdlib here.

To fix this we need to use a time mechanism that has more precision than 100 nanosecond intervals.

rylev avatar Sep 10 '21 13:09 rylev

@eddyb mentioned that a fix for this should also take care of the case when we are not actually dealing with timestamps, such as when recording instructions counts.

michaelwoerister avatar Oct 06 '21 10:10 michaelwoerister

For generalized handling of counters, we've been serializing this as counter.units in the JSON: https://github.com/rust-lang/measureme/blob/1d904b780fb77b28d93a37c064a7544970e5064f/measureme/src/counters.rs#L148-L158

So we should be able to pretty-print an u64 that measures nanoseconds, as the appropriate unit for a maximum number of "significant digits". It's just going to be annoying to thread that state through.

eddyb avatar Oct 06 '21 10:10 eddyb