reuse runtime instead of spawning new ones in new Thread

[Witcher01]

main spawns a new thread to fetch the correct mirrors

rate-mirrors/src/main.rs

Lines 103 to 109 in dad9c23

	let thread_handle = thread::spawn(move || {
	let mirrors = match &config.target {
	Target::Arch(target) => fetch_arch_mirrors(
	Arc::clone(&config),
	target.clone(),
	mpsc::Sender::clone(&tx_progress),
	),

This thread then spawns a Runtime to block on an async function

rate-mirrors/src/targets/archlinux.rs

Line 39 in dad9c23

let runtime = tokio::runtime::Runtime::new().unwrap();

rate-mirrors/src/targets/archlinux.rs

Lines 42 to 43 in dad9c23

	let response = runtime
	.block_on(

Would it make sense to create a general Runtime instead to reuse in the functions that fetch mirrors instead? Or is this done by design?

Reusing the runtime would, for instance, make sense in fetch_arch_mirrors as the Runtime currently blocks twice there, negating the pros of using tokio there in the first place.

[westandskif]

Thank you for your interest! Sharing my thoughts below, please let me know yours:)

This assumes that the Runtime is needed in every single use case, but this is not guaranteed.
Also, since every independent implementation (arch/manjaro etc.) can reuse their own runtimes they create, this shouldn't be a problem too.

Regarding double blocking: since it works inside a regular (non-async) function, it has to block since it has nothing else to do while fetching the list of mirrors to test.

The threading approach allows for parallel progress tracking in the main thread.

[Witcher01]

This assumes that the Runtime is needed in every single use case, but this is not guaranteed.

I'm not sure I understand why this is. I was assuming that every target has to fetch their mirrorlist or test their servers with the asynchronous calls on reqwest.

Regarding double blocking: since it works inside a regular (non-async) function, it has to block since it has nothing else to do while fetching the list of mirrors to test.

In that case, how about using blocking calls to reqwest instead? I'm guessing this is way cheaper than spawning a Tokio Runtime.
https://docs.rs/reqwest/latest/reqwest/blocking/index.html

The threading approach allows for parallel progress tracking in the main thread.

I wasn't sure about that. Thanks for clearing that up!

[westandskif]

In that case, how about using blocking calls to reqwest instead? I'm guessing this is way cheaper than spawning a Tokio Runtime.

I've left the explicit use of Runtime & block_on for more granular control over what is happening. e.g. it's easy to log "connection established" and then "mirror list fetched" if you are able to log something between the moment when the connection is established and when the data starts to be fetched. The blocking calls don't allow this.

Regarding "way cheaper", I haven't profiled this, but I bet this cannot be noticed in a networking application like this.
However, thank you for your suggestions! I'll consider the simplification in next versions.

[Witcher01]

I see, thanks for clearing this up!

Regarding "way cheaper", I haven't profiled this, but I bet this cannot be noticed in a networking application like this.

Seems like this is what you call "premature optimization" on my end, oops. You are right, it's definitely not needed, there's barely anything to gain, if anything at all.