doc: improves spawn_blocking documentation #2426

palashahuja · 2020-04-22T14:12:04Z

The initial spawn_blocking documentation was unclear. This PR attempts to fix that.

Fixes #2143

Solution

Wrote a simpler documentation since the initial blocking documentation was unclear.

Darksonn

Hi. Thank you for the PR. Please see CONTRIBUTING.md for instructions for fixing the CI issue.

Darksonn · 2020-04-22T14:46:29Z

tokio/src/task/blocking.rs

+    /// Spawns a blocking thread on the currently provided executor.
+    /// More information about the blocking threads is available at 
+    /// [CPU-bound tasks and blocking code](../index.html#cpu-bound-tasks-and-blocking-code)


It does not necessarily spawn a thread. The threads are reused. Additionally, can you split the link and url with this syntax:

at [CPU-bound tasks and blocking code][blocking]. [blocking]: ../index.html#cpu-bound-tasks-and-blocking-code

You're also missing a period at the end of the sentence.

@Darksonn Sure, I can split the URL. Also, is the following correct then -
Runs the provided function on the executor reserved for blocking operations

Well, it's part of the same executor as the async code.. I quite like the phrasing in the sentence above: "It can be thought of as an efficient Iterator for collections of bytes." Perhaps you can merge the two lines together?

Okay. So, this is the final comment -

Runs the provided function on the executor reserved for blocking operations. It can be thought of as an efficient iterator for collections of bytes. More information about the blocking threads is available at [CPU-bound tasks and blocking code][blocking]. [blocking]: ../index.html#cpu-bound-tasks-and-blocking-code

Is this okay @Darksonn ?

Did you miss up your clipboard?

Yes, I did not run rustfmt before sending it. I will correct this and send it. Is the above comment okay though?

I was referring to the "It can be thought of as an efficient iterator for collections of bytes." part.

Oh okay!
I meant this

Runs the provided function on the executor reserved for blocking operations. More information about the blocking threads is available at [CPU-bound tasks and blocking code][blocking]. [blocking]: ../index.html#cpu-bound-tasks-and-blocking-code

Sure, looks good. Perhaps add an empty line between each pair of lines.

I will send the PR right away. Thanks !

Darksonn · 2020-04-22T18:08:44Z

tokio/src/task/blocking.rs

-    /// yielding is not okay, as it may prevent the executor from driving other futures forward.
-    /// A closure that is run through this method will instead be run on a dedicated thread pool for
-    /// such blocking tasks without holding up the main futures executor.
+    /// Runs the provided function on the executor reserved for blocking operations.


Can we fix the duplicated line?

I have fixed it.

It is not running on a separate executor, but a thread on the same executor. I would rather swap them the other way.

I've just realized that it was I who messed up my clipboard previously. When I said "It can be thought of as an efficient Iterator for collections of bytes.", I was trying to say "Runs the provided closure on a thread where blocking is acceptable."

Okay, I will commit the final change

Darksonn · 2020-04-22T18:39:38Z

tokio/src/task/blocking.rs

@@ -37,12 +37,11 @@ cfg_rt_threaded! {
 }

 cfg_blocking! {
-    /// Runs the provided closure on a thread where blocking is acceptable.
+    /// Runs the provided closue on a thread where blocking is acceptable 


If we fix the typo here, I think this is ready to merge :)

lol!
I don't know why I am making so many mistakes. Sorry for the troubles.

Darksonn · 2020-04-22T18:44:27Z

tokio/src/task/blocking.rs

@@ -1,7 +1,7 @@
 use crate::task::JoinHandle;

 cfg_rt_threaded! {
-    /// Runs the provided blocking function without blocking the executor.
+    /// Runs the provided closure on a thread where blocking is acceptable.


The block_in_place function works differently from spawn_blocking. It doesn't run it on a separate thread.

Sorry about that. I am making the change. I modified the wrong function by mistake.

palashahuja · 2020-04-22T19:27:03Z

@Darksonn I think the PR is ready for merge.

Darksonn · 2020-04-22T19:28:09Z

CI is still running. I will do it tomorrow.

Darksonn reviewed Apr 22, 2020

View reviewed changes

palashahuja force-pushed the doc_blocking branch from 458db38 to 908f131 Compare April 22, 2020 17:41

Darksonn reviewed Apr 22, 2020

View reviewed changes

palashahuja force-pushed the doc_blocking branch 3 times, most recently from f524362 to dc03b55 Compare April 22, 2020 18:37

Darksonn reviewed Apr 22, 2020

View reviewed changes

palashahuja force-pushed the doc_blocking branch 2 times, most recently from bc4f2b5 to ecef736 Compare April 22, 2020 18:42

Darksonn reviewed Apr 22, 2020

View reviewed changes

task: improves spawn_blocking documentation

b9b4e9b

palashahuja force-pushed the doc_blocking branch from ecef736 to b9b4e9b Compare April 22, 2020 18:46

Darksonn approved these changes Apr 23, 2020

View reviewed changes

Darksonn merged commit f83f638 into tokio-rs:master Apr 23, 2020

palashahuja deleted the doc_blocking branch April 23, 2020 14:56

Darksonn added A-tokio Area: The main tokio crate C-maintenance Category: PRs that clean code up or issues documenting cleanup. M-task Module: tokio/task T-docs Topic: documentation labels Apr 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: improves spawn_blocking documentation #2426

doc: improves spawn_blocking documentation #2426

palashahuja commented Apr 22, 2020

Darksonn left a comment •

edited

Loading

Darksonn Apr 22, 2020

palashahuja Apr 22, 2020 •

edited

Loading

Darksonn Apr 22, 2020

palashahuja Apr 22, 2020

Darksonn Apr 22, 2020

palashahuja Apr 22, 2020

Darksonn Apr 22, 2020

palashahuja Apr 22, 2020

Darksonn Apr 22, 2020

palashahuja Apr 22, 2020

Darksonn Apr 22, 2020

palashahuja Apr 22, 2020

Darksonn Apr 22, 2020

Darksonn Apr 22, 2020

palashahuja Apr 22, 2020

Darksonn Apr 22, 2020

palashahuja Apr 22, 2020

Darksonn Apr 22, 2020

palashahuja Apr 22, 2020

palashahuja commented Apr 22, 2020

Darksonn commented Apr 22, 2020

doc: improves spawn_blocking documentation #2426

doc: improves spawn_blocking documentation #2426

Conversation

palashahuja commented Apr 22, 2020

Solution

Darksonn left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

palashahuja Apr 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

palashahuja commented Apr 22, 2020

Darksonn commented Apr 22, 2020

Darksonn left a comment •

edited

Loading

palashahuja Apr 22, 2020 •

edited

Loading