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.

Sign up for GitHub

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

improve documentation on std::thread::sleep #54646

Merged
merged 4 commits into from
Oct 18, 2018
Merged

improve documentation on std::thread::sleep #54646

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a605a6
fix std::thread::sleep typo
Sep 28, 2018
049ccbb
update wording for std::thread::sleep
Sep 28, 2018
b63517a
update wording for thread::sleep
Oct 1, 2018
7a0fa95
improve docs on thread::sleep
Oct 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/libstd/thread/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,15 +650,15 @@ pub fn panicking() -> bool {
panicking::panicking()
}

/// Puts the current thread to sleep for the specified amount of time.
/// Puts the current thread to sleep for at least the specified amount of time.
///
/// The thread may sleep longer than the duration specified due to scheduling
/// specifics or platform-dependent functionality.
/// specifics or platform-dependent functionality. It will never sleep less.
///
/// # Platform-specific behavior
///
/// On Unix platforms this function will not return early due to a
/// signal being received or a spurious wakeup.
/// On Unix platforms this function may invoke multiple syscalls
/// in case of a signal being received or a spurious wakeup.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Unix platforms this function may invoke multiple syscalls in case of a signal being received or a spurious wakeup.

I don't quite understand why we're talking about system calls here. Can someone explain how it's relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frewsxcv The way how "sleep" is usually implemented is telling the OS to put a thread to sleep. This is done via a "syscall". "sleep" is so often implemented via syscall that some people may even assume it's the same. And in many languages this is basically the same.

Also, making more than one syscall could probably have some performance impact in very specific cases -- though I'm not entirely sure about that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation! Why should the reader care how many system calls are invoked per platform? Just trying to figure out what the value-add is for this section.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frewsxcv I guess the value-add is clarity.

Some people are just too used to the "old"/"usual" behavior. Even JVM, which poses itself as cross-platform, still makes only one underlying sleeping syscall, which results in the specific behavior.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you feel about this text?

> On Unix platforms, the [underlying `sleep` system call][syscall] can sporadically
> wake early. To ensure we sleep for at least the specified duration, this function
> may invoke that system call multiple times.
>
> [syscall]: http://man7.org/linux/man-pages/man3/sleep.3.html

I think this provides some more context to the reader why we're talking about system calls and why this is platform-specific. Thoughts?

Copy link
Contributor Author

@vn971 vn971 Oct 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds interesting. Some remarks though:

  • the underlying syscall is nanosleep: http://man7.org/linux/man-pages/man2/nanosleep.2.html (note the number -- it's "2" for systemcalls. Number "3" is for "library calls" -- basically the C interface).

  • the thread can as well wake up due to a signal received (as documented in the manual). That needs to be mentioned as well.

I'll return to this a bit later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another attempt:

> On Unix platforms, the [underlying `nanosleep` system call][syscall] can
> spuriously wake early or wake early from a signal interruption. To
> ensure the sleep occurs for at least the specified duration, this function
> may invoke that system call multiple times.
>
> [syscall]: http://man7.org/linux/man-pages/man2/nanosleep.2.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frewsxcv but then again if I think more about the problem, it seems that the exact system call might be Linux-specific, whereas the start of paragraph says "Unix".
I've pushed an update taking the idea and wording of your proposal, please take a look.

///
/// # Examples
///
Expand Down