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

Timer functions should maybe not take milliseconds as an argument #11189

Closed
alexcrichton opened this issue Dec 29, 2013 · 11 comments
Closed

Timer functions should maybe not take milliseconds as an argument #11189

alexcrichton opened this issue Dec 29, 2013 · 11 comments
Assignees
Milestone

Comments

@alexcrichton
Copy link
Member

System timers often expose a granularity much more fine than one millisecond. Right now this is an artifact of libuv's api, but in a 1:1 context you can get much finer-grained control over this.

Perhaps the arguments should be a (second/nsec) struct (like in C), with sugar to do the "easy thing"

@alexcrichton
Copy link
Member Author

Ideally these would take a "Time" or "Duration" structure which has nanosecond precision (or something like that).

Nominating because this API will determine the backwards compatibility of timers.

@sfackler
Copy link
Member

+1 for a Duration type or at the very least a unit and magnitude (e.g. 1, time::Seconds).

@alexcrichton
Copy link
Member Author

One thought I had this morning is that if we don't have a solution for a "time type" by 1.0, we could rename all these functions to foo_ms and then deprecate them once we have the true function, but that also seems... bad.

@pnkfelix
Copy link
Member

pnkfelix commented Jan 9, 2014

Accepted for 1.0 backcompat-libs.

@brson
Copy link
Contributor

brson commented Jul 11, 2014

If we create a duration type here we should consider it's forward compatibility with #14657. We may end up in a situation where Duration ends up defined somewhere like core and reexported in a bunch of places.

@alexcrichton
Copy link
Member Author

It may end up being the case that this is the correct API for the standard library, and the correponding time library will provide an extension trait to use an external Duration type (just one possibility)

@brson
Copy link
Contributor

brson commented Jul 15, 2014

The rust-chrono Duration type supports ns: https://github.com/lifthrasiir/rust-chrono/blob/master/src/chrono/duration.rs

@brson
Copy link
Contributor

brson commented Jul 15, 2014

cc @lifthrasiir discussing how timers want a duration type.

@brson
Copy link
Contributor

brson commented Jul 15, 2014

I have a branch that does the following:

  • Adds rust-chrono's Duration under std::duration
  • Renames the sleep, periodic, and oneshot functions to sleep_ms, periodic_ms, oneshot_ms
  • Adds back those functions, but taking Duration.

Looking for feedback.

@huonw
Copy link
Member

huonw commented Jul 15, 2014

The public API should probably use i32 etc. like the internals (rather than int and uint), and maybe it could be std::time rather than std::duration? (The latter is a complete bikeshed, I have no strong opinion at all.)

@lifthrasiir
Copy link
Contributor

@huonw FYI, rust-chrono now uses i32 and u32 for public APIs. I've thought a bit hard about that, and reached the conclusion that keeping int and uint is not worth enough to take a risk.

brson added a commit to brson/rust that referenced this issue Jul 21, 2014
Taken from rust-chrono[1]. Needed for timers per rust-lang#11189.
Experimental.

[1]: https://github.com/lifthrasiir/rust-chrono
@brson brson self-assigned this Aug 1, 2014
brson added a commit to brson/rust that referenced this issue Aug 13, 2014
Taken from rust-chrono[1]. Needed for timers per rust-lang#11189.
Experimental.

[1]: https://github.com/lifthrasiir/rust-chrono
@bors bors closed this as completed in 28b5e45 Aug 14, 2014
flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 31, 2023
flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 31, 2023
[significant_drop_tightening] Fix rust-lang#11189

Fix rust-lang#11189

```
changelog: FP: [`significant_drop_tightening`]: Consider tuples in drop calls
```
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 a pull request may close this issue.

6 participants