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

Fixes benchmark / stress test numbers #1

Closed
wants to merge 2 commits into from

Conversation

joelgallant
Copy link
Contributor

You'll notice that here, they're using num_nanoseconds, which is different than you have it here.

subsec_nanos only returns, as you might guess, sub-seconds (only the fractional part).

This made me think that image captures were happening 100s of times faster than they actually were.

@pedrosland
Copy link
Owner

Hi Joel. Thanks for your contribution. I completely forgot that I was supposed to fix up that code.

I suppose that CI doesn't build the examples or something because this change doesn't compile for me. I need an as f64 to convert the u64 to f64. Does it compile for you? If not, do you want to fix that so I can merge?

Here's my compiler output.

error[E0277]: cannot add `f64` to `u64`
  --> examples/stress.rs:32:43
   |
32 |     let runtime_secs = duration.as_secs() + (duration.subsec_nanos() as f64 / 1_000_000_000.0);
   |                                           ^ no implementation for `u64 + f64`
   |
   = help: the trait `std::ops::Add<f64>` is not implemented for `u64`

@joelgallant
Copy link
Contributor Author

Ah, oops. I wasn't cross-compiling it, so I figured the CI would catch any problems. I'll push what should be a fix (don't have a pi with me sorry).

@pedrosland
Copy link
Owner

Hi Joel. Thanks again. I have merged this manually and fixed CI to build the examples so we can catch errors 😄.

@pedrosland pedrosland closed this Nov 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants