Skip to content

blocking on spawn not work as intended #740

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

Open
win-t opened this issue Apr 7, 2020 · 6 comments
Open

blocking on spawn not work as intended #740

win-t opened this issue Apr 7, 2020 · 6 comments

Comments

@win-t
Copy link

win-t commented Apr 7, 2020

in the blog it is said that:

The runtime measures the time it takes to perform the blocking operation and if it takes a while, a new thread is automatically spawned and replaces the old executor thread. That way, only the current task is blocked on the operation rather than the whole executor thread or the whole runtime. If the blocking operation is quick, we don’t spawn a new thread and therefore no additional cost is inflicted.

so I tried to block all available thread (my machine has 4 core), but after a while, I don't see any new thread is spawned

use async_std::task; // version 1.5.0
use std::time::Duration;

fn main() {
    task::block_on(async {
        let num = num_cpus::get();
        let mut handlers = Vec::with_capacity(num * 2);

        for i in 1..=num {
            handlers.push(task::spawn(blocking_work(i)));
            handlers.push(task::spawn(normal_work(i)));
        }

        for h in handlers.into_iter() {
            h.await
        }
    })
}

async fn blocking_work(i: usize) {
    loop {
        std::thread::sleep(Duration::from_secs(5));
        println!("blocking_work {}", i)
    }
}

async fn normal_work(i: usize) {
    loop {
        task::sleep(Duration::from_secs(1)).await;
        println!("normal_work {}", i)
    }
}

shouldn't runtime spawn more OS thread for this?

@win-t
Copy link
Author

win-t commented Apr 7, 2020

more infomation

no normal_work is printed
image

no extra thread is spawned (image from htop)
image

@win-t
Copy link
Author

win-t commented Apr 7, 2020

for comparison, same logic but, with golang (because the blog said the runtime is inspired by Go)

package main

// #include <unistd.h>
import "C"

import (
	"fmt"
	"runtime"
	"sync"
	"time"
)

func main() {
	var wg sync.WaitGroup
	num := runtime.GOMAXPROCS(-1)
	for i := 1; i <= num; i++ {
		wg.Add(1)
		go blockingWork(wg.Done, i)

		wg.Add(1)
		go normalWork(wg.Done, i)
	}
	wg.Wait()
}

func blockingWork(done func(), i int) {
	defer done()
	for {
		C.usleep((C.uint)((5 * time.Second).Microseconds())) // POSIX's usleep
		fmt.Println("blocking_work", i)
	}

}

func normalWork(done func(), i int) {
	defer done()
	for {
		time.Sleep(1 * time.Second)
		fmt.Println("normal_work", i)
	}
}

golang spawn more extra OS thread
image

and normal_work is printed
image

@yoshuawuyts
Copy link
Contributor

Hi @win-t, thanks for opening this issue. The new runtime wasn't merged due to some performance concerns. That might be why you're not seeing the numbers you were expecting.

@Ujang360
Copy link

Ujang360 commented Apr 7, 2020

Hi @win-t, thanks for opening this issue. The new runtime wasn't merged due to some performance concerns. That might be why you're not seeing the numbers you were expecting.

I see, I'm curious though, do you have the issue link for the performance concern discussion @yoshuawuyts 🙏

@win-t
Copy link
Author

win-t commented Apr 8, 2020

after read comments on #631
I understand that we should use spawn_blocking, but I still think auto spawn new thread is still needed as a fallback when user forgot to use spawn_blocking, and we should give a warning (via stderr) to the user when that new thread is spawned because it means that user is misused the API

@win-t
Copy link
Author

win-t commented Apr 8, 2020

Hi @win-t, thanks for opening this issue. The new runtime wasn't merged due to some performance concerns. That might be why you're not seeing the numbers you were expecting.

I see, I'm curious though, do you have the issue link for the performance concern discussion @yoshuawuyts

I don't read the code yet, but I guess because we need a mutex to steal work from other threads (to know which thread to steal), given that the number of thread is dynamic, and dealing with dynamic (shared mutable) requires a mutex

This was referenced Apr 9, 2020
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

No branches or pull requests

3 participants