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

Add some thread pool docs #421

Merged
merged 3 commits into from
Jun 15, 2018
Merged

Add some thread pool docs #421

merged 3 commits into from
Jun 15, 2018

Conversation

carllerche
Copy link
Member

This patch adds some additional comments to the thread pool implementation.

@@ -3,6 +3,107 @@
#![doc(html_root_url = "https://docs.rs/tokio-threadpool/0.1.4")]
#![deny(warnings, missing_docs, missing_debug_implementations)]

// The Tokio thread pool is a thread pool designed to scheduled futures in Tokio
Copy link
Member

Choose a reason for hiding this comment

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

yeah, i know the tokio thread pool is a thread pool.

better: The tokio thread pool is designed to schedule futures in Tokio-based applications

(present-tense "schedule", hyphenated "Tokio-based")

// * Worker threads.
// * Backup threads.
//
// Worker threads are primary threads and are used to schedule futures. These
Copy link
Member

Choose a reason for hiding this comment

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

is a "primary thread" something i should know about? Otherwise, this might be clearer as:

"The primary thread pool consists of worker threads and is used to schedule futures using a work-stealing strategy. Back up threads, on the other hand, are intended only to support the blocking API. Threads may transition between these pools.

// threads operate using a work-stealing strategy. Backup threads are to support
// the `blocking` API. Threads will transition between the two sets.
//
// The advantage of the work-stealing strategy is minimal cross thread
Copy link
Member

Choose a reason for hiding this comment

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

prefer "cross-thread"

// # Crate layout
//
// The primary type is `Pool`. This struct contains the majority of the pool
// state. Because worker threads will shutdown and be reestarted, the pool also
Copy link
Member

Choose a reason for hiding this comment

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

The primary type, Pool, holds the majority of a thread pool's state, including the state for each worker. Each worker's state is maintained in an instance of worker::Entry.

// "steal" from that deque. The mpsc channel is used to submit futures while
// external to the pool.
//
// As long as the thread pool has not been shutdown, a worker will run in a
Copy link
Member

Choose a reason for hiding this comment

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

this sentence makes me wonder: what happens when the thread pool shuts down?

// # Thread pool initialization
//
// By default, no threads are spawned on creation. Instead, when new futures are
// spawned, the pool first checks if there are enough active workeer threads. If
Copy link
Member

Choose a reason for hiding this comment

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

"active worker threads"

// # Spawning futures
//
// The process for spawning a future depends on whether the action is taken from
// a workere thread or external to the thread pool.
Copy link
Member

Choose a reason for hiding this comment

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

"a worker thread"

better: The spawning behavior depends on whether a future was spawned from within a worker or thread or if it was spawned from an external handle.

//
// # Sleeping workers
//
// Sleeping workers are tracked using a treiber stack. This results in the
Copy link
Member

Choose a reason for hiding this comment

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

i'm not sure if Treiber is a typo or a real word. If it's a real word, perhaps include a reference.

// In the first case, the worker will always be notified. However, it could be
// possible to avoid the notification if the mpsc channel has two or greater
// number of tasks *after* the task is submitted. In this case, we are able to
// assume that the worker has preeviously been notified.
Copy link
Member

Choose a reason for hiding this comment

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

"previously"

@carllerche carllerche merged commit 3fac7ce into master Jun 15, 2018
dekellum added a commit to dekellum/tokio that referenced this pull request Jun 25, 2018
dekellum added a commit to dekellum/tokio that referenced this pull request Jul 8, 2018
dekellum added a commit to dekellum/tokio that referenced this pull request Jul 11, 2018
Per review suggestion by @carllerche on tokio-rs#450, this essentially reverts
the promotion of these low-level detail sections, as done originally
in:

    014229f Promote tokio-threadpool crate level comments to rustdoc

github: cc tokio-rs#421
carllerche pushed a commit that referenced this pull request Jul 22, 2018
* Normalize links to docs.rs/CRATE/M.N/...

docs.rs is smart enough to show docs for the latest M.N.P release when
M.N is used in the link. For example:

  https://docs.rs/mio/0.6/mio/struct.Poll.html

..will show mio 0.6.14 and later docs. While using the `M.N.*`
(ASTERISK) syntax also works, `M.N` is the more common usage, so
standarize a few existing links to that format.

* Fix missing or malformed rustdoc links

* executor lib rustdoc minor format change

* Promote tokio-threadpool crate level comments to rustdoc

* Replace hidden tokio::executor::thread_pool docs with deprecation note

* Fix typo/simplify util module rustdoc

* Reuse some tokio::executor::thread_pool rustdoc for the crate

Relates to #421
@carllerche carllerche deleted the document-threadpool branch August 9, 2018 19:43
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