rt: reduce an unnecessary lock operation

[biluohc]

Motivation

I found an unnecessary lock operation.

tokio/tokio/src/runtime/blocking/pool.rs

Lines 222 to 271 in afd2189

	pub(crate) fn spawn(&self, task: Task, rt: &Handle) -> Result<(), ()> {
	let shutdown_tx = {
	let mut shared = self.inner.shared.lock();
	if shared.shutdown {
	// Shutdown the task: it's fine to shutdown this task (even if
	// mandatory) because it was scheduled after the shutdown of the
	// runtime began.
	task.task.shutdown();
	// no need to even push this task; it would never get picked up
	return Err(());
	}
	shared.queue.push_back(task);
	if shared.num_idle == 0 {
	// No threads are able to process the task.
	if shared.num_th == self.inner.thread_cap {
	// At max number of threads
	None
	} else {
	shared.num_th += 1;
	assert!(shared.shutdown_tx.is_some());
	shared.shutdown_tx.clone()
	}
	} else {
	// Notify an idle worker thread. The notification counter
	// is used to count the needed amount of notifications
	// exactly. Thread libraries may generate spurious
	// wakeups, this counter is used to keep us in a
	// consistent state.
	shared.num_idle -= 1;
	shared.num_notify += 1;
	self.inner.condvar.notify_one();
	None
	}
	};
	if let Some(shutdown_tx) = shutdown_tx {
	let mut shared = self.inner.shared.lock();
	let id = shared.worker_thread_index;
	shared.worker_thread_index += 1;
	let handle = self.spawn_thread(shutdown_tx, rt, id);
	shared.worker_threads.insert(id, handle);
	}

        let shutdown_tx = {
            let mut shared = self.inner.shared.lock(); 
           ...
       }

        if let Some(shutdown_tx) = shutdown_tx {
            let mut shared = self.inner.shared.lock();
       }

Solution

delete it

[Darksonn]

You only call this once. Just put the code where you use it.

[biluohc]

The original code use let shutdown_tx = {} may be to consider that the block needs to be called multiple times in the future?

And the static dispatch closure should have no additional overhead?

[Darksonn]

I'm not worried about overhead. I just think it would be easier to read the code without the closure.

[biluohc]

I don't think the closure makes code more difficult to read, it's more clear.

[biluohc]

But I'll change it if you think it's better without the closure..

[biluohc]

Please continue to review @Darksonn

[Darksonn]

It seems reasonable to me.

[biluohc]

ok, thanks you very much

[carllerche]

Thanks. I can't find a reason for the unlock/lock in the history or comments. Tests pass, so lets merge it.