Skip to content
This repository has been archived by the owner on Aug 18, 2022. It is now read-only.

Remove double threadpool #1

Closed
spacekookie opened this issue May 12, 2018 · 3 comments
Closed

Remove double threadpool #1

spacekookie opened this issue May 12, 2018 · 3 comments

Comments

@spacekookie
Copy link

Right now you're spawning a thread for each cpu core (with thread::spawn(...) and executing a worker on it. Then for each worker you create a tokio cpu_pool with 8 (hardcoded) threads. This means that you will, on an 8 "core" system, spawn 64 threads, meaning a lot of context switching.

The tokio cpu_pool is actually designed to scale Async I/O futures across available CPU cores. Removing the second layer of thread pooling decreases memory consumption by about half and increases throughput (on my laptop, binding to 127.0.0.1 by about 30%

@timvisee
Copy link
Owner

You're totally right! I've known about this for quite a while, but have never really had the time to change it to a single-pooled design (although that should be fairly simple).
I didn't know that it would improve performance by that much (on your machine of course) however, so that's awesome.

Thanks for the issue report though, issues, ideas and feedback are always welcome! It seems like you've already transferred the logic to use a single thread pool, feel free to submit a pull request for this change.

You've currently posted this issue on the pixelpwnr-render repository, but I think this issue should be part of pixelpwnr-server. It would be awesome if you could re-post this issue there. The pixelpwnr-render module is just a part of the full server, providing the rendering logic. The actual server (and thread pool) implementation is part of pixelpwnr-server.

I would like to note that this isn't a finished product, by far. So far I've only really been able to work on it in a prototyping phase. I really want to release a proper polished version some day, although I can't tell when that will be. The current state of the project is quite messy, to be honest.

@spacekookie
Copy link
Author

Oh woops! I completely missed that this was the renderer 😅 I'll repost this on the proper repo.

But I understand that this is prototype software. I'm a huge Rust fan and maybe for GPN next year we can have a server that can handle several 10G NICs in a proper server 😉

@timvisee
Copy link
Owner

@spacekookie that sounds awesome!
By the way, I'd definitely be willing to look into this project to make it as performant as possible for such high loads before the end of the year (possibly to deploy at 35C3). I feel that there are quit a few other areas to improve on. I don't have any 10G hardware at hand though, thus sadly I won't be able to test such load levels over the network.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants