tcp-stress-test: Pull out thread count as a constant #35140
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
the-kenny
changed the title
|
Sorry, got slightly confused with the commit message. Will force-push a fixed one. |
the-kenny
force-pushed
the
tcp-stress-test-const-thread-count
branch
from
d9af199
to
aaf8a6e
Compare
|
Do we perhaps want to go ahead and just reduce the number of threads so nixos doesn't need to have a patch? |
|
Can we just drop this test entirely? Now that the big runtime's gone, this seems like a test of the OS, not Rust. |
|
I don't personally see much reason to actively drop this, it's a test that we're exercising and it's good to at least keep it running in some capacity. I know in Windows it least it may stress some initialization logic or something like that. |
|
If we want to lower the count, what would a sensible number? |
|
Yeah something on the order of 200 seems fine to me |
|
@bors r=alexcrichton rollup |
|
📌 Commit 3ea293d has been approved by |
…ead-count, r=alexcrichton tcp-stress-test: Pull out thread count as a constant This PR factors out the number of concurrent threads used in `tcp-stress-test.rs` to a constant at the top of the file. We at @NixOS had to lower our thread count as the chrooted-builds don't allow that many threads. This change will make it easier to lower/increase the count in the future (I actually forgot to change the second `1000` when I was working on this). Another benefit is the removal of magic numbers in the test suite. This is related to rust-lang#35107
This PR factors out the number of concurrent threads used in
tcp-stress-test.rsto a constant at the top of the file.We at @NixOS had to lower our thread count as the chrooted-builds don't allow that many threads.
This change will make it easier to lower/increase the count in the future (I actually forgot to change the second
1000when I was working on this). Another benefit is the removal of magic numbers in the test suite.This is related to #35107