Networking: fold websocket and http tokio runtime into one.

[gterzian]

We use two separate Tokio runtimes:

One for HTTP requests:

servo/components/net/http_loader.rs

Line 74 in 525fc58

pub static ref HANDLE: Mutex<Option<Runtime>> = Mutex::new(Some(Runtime::new().unwrap()));

And another one for Websockets:

servo/components/net/websocket_loader.rs

Line 52 in 525fc58

pub static ref HANDLE: Mutex<Option<Runtime>> = Mutex::new(Some(Runtime::new().unwrap()));

The rationale is:

servo/components/net/websocket_loader.rs

Lines 48 to 50 in 525fc58

	// Websockets get their own tokio runtime that's independent of the one used for
	// HTTP connections, otherwise a large number of websockets could occupy all workers
	// and starve other network traffic.

I think it's doubtful that a large amount of websockets could really starve HTTP request, since the whole point of tokio is to run a large amount a tasks onto a single runtime.

We should investigate using one runtime for all networking tasks.

[wusyong]

I saw your comment in zulip mentioned there's another PR dealing with thread spawning too. But it seems it created even more threads. I'm wondering if we could make whole net crate to share the same tokio runtime without manually spawning threads and thread pool.

[gterzian]

But it seems it #31517 (comment) even more threads.

Yes that's correct, it uses a threadpool which will see threads created upfront, instead of spawning one thread for each image that is decoded.

I'm wondering if we could make whole net crate to share the same tokio runtime without manually spawning threads and thread pool.

We can use a single runtime for all async task, and a single core resource threadpool for the other non-async bacground work.

[wusyong]

@gterzian Could you add some good-first-issue or E-candidate-for-mentoring label?
This issue seems like a good chance for anyone who want to contribute.

[tannal]

I would like to take this up.

[MunishMummadi]

Hey @tannal what is your status with the issue. Can I join

[tannal]

@MunishMummadi Yes you can.
The quick fix for this issue is done.

We can use a single runtime for all async task, and a single core resource threadpool for the other non-async bacground work.

You can open another pr to do this part if you are interested in this issue.

[gterzian]

a single core resource threadpool for the other non-async bacground work.

Yes indeed, this part would be good for a new PR @MunishMummadi

The idea is to use a single CoreResourceThreadPool in components/net, see for guidance this issue, which added a second one. You could make changes to have a single pool used where now two are used.

[MunishMummadi]

Yes I would love to work on it @gterzian @tannal . Can I ask the bot to "assign me"!!!

[gterzian]

Actually, because the image cache runs in the script thread, not in the core resource thread, this is quite complicated and perhaps not a good idea. I withdraw my earlier conclusion.