rust-lang / jobserver-rs

Apache License 2.0
69 stars 40 forks source link

Optimize `unix::Client::new`: Avoid multi `write` #39

Closed NobodyXu closed 2 years ago

NobodyXu commented 2 years ago

Signed-off-by: Jiahao XU Jiahao_XU@outlook.com

alexcrichton commented 2 years ago

Thanks! Instead of performing a heap allocation though could this perhaps reserve a 128-byte buffer as as static or const and write that instead? I suspect most systems have <128 cores so it'll likely have the same effective effect.

NobodyXu commented 2 years ago

Thanks! Instead of performing a heap allocation though could this perhaps reserve a 128-byte buffer as as static or const and write that instead? I suspect most systems have <128 cores so it'll likely have the same effective effect.

Done!

NobodyXu commented 2 years ago

Oops, I accidentally closed it

Edit:

I just did it again.

alexcrichton commented 2 years ago

Oh sorry but what I meant was that instead of having anything heap allocated this could instead still have a loop, but the loop is "chunked" into 128-bytes-at-a-time. So no need for vec! , still a need for the loop, and most of the time only one trip through the loop will be taken.

NobodyXu commented 2 years ago

Oh sorry but what I meant was that instead of having anything heap allocated this could instead still have a loop, but the loop is "chunked" into 128-bytes-at-a-time. So no need for vec! , still a need for the loop, and most of the time only one trip through the loop will be taken.

Got it, I will do that.

NobodyXu commented 2 years ago

Oh sorry but what I meant was that instead of having anything heap allocated this could instead still have a loop, but the loop is "chunked" into 128-bytes-at-a-time. So no need for vec! , still a need for the loop, and most of the time only one trip through the loop will be taken.

Done!

And out of some mysterious reasons, my PR got closed again after I pushed my commits.

NobodyXu commented 2 years ago

Can the set_nonblocking all happen within Client::new with comments as to what it's doing? Otherwise having it spread out across a few methods makes it harder to figure out what's going on.

Sure!

I was thinking about adding O_NONBLOCK to pipe2, that's why I added it to `mk.

NobodyXu commented 2 years ago

Can the set_nonblocking all happen within Client::new with comments as to what it's doing? Otherwise having it spread out across a few methods makes it harder to figure out what's going on.

Done!