Skip to content

Decouple number of concurrent tests from RUST_THREADS env var #7335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
brson opened this issue Jun 24, 2013 · 4 comments
Closed

Decouple number of concurrent tests from RUST_THREADS env var #7335

brson opened this issue Jun 24, 2013 · 4 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@brson
Copy link
Contributor

brson commented Jun 24, 2013

Currently the test runner will adjust the number of tests run concurrently based on RUST_THREADS. I don't think this coupling really makes sense and we should instead use RUST_TEST_TASKS.

@vijaykramesh
Copy link

Where were you imagining the env would load RUST_TEST_TASKS instead of RUST_THREADS? It'd be simple enough to also check getenv(RUST_TEST_TASKS) and use that if it exists (at https://github.com/mozilla/rust/blob/master/src/rt/rust_env.cpp#L87) but it seems a bit messy to include test logic in the main env loader.

Unless I'm mistaken, the result of that environmental setting gets loaded for both tests and non-tests when creating the scheduler at https://github.com/mozilla/rust/blob/master/src/rt/rust_kernel.cpp#L49 so not sure if there is a cleaner place to add in the test-specific env stuff, or if it belongs in src/rt/rust_env.cpp?

@vijaykramesh
Copy link

I don't think that is quite right though, as if you specify RUST_TEST_TASKS=X RUST_THREADS=Y it will start the scheduler with X even if you aren't running tests. Hmm.

@brson
Copy link
Contributor Author

brson commented Jul 9, 2013

@vijaykramesh sorry for the delayed response. I would put a call to os::getenv("RUST_TEST_TASKS") directly in extra::test::get_concurrency, and if that's not set fallback to the current behavior of using the same number of tasks as RUST_THREADS * some constant, minus the special case when RUST_THREADS=1 (when RUST_THREADS=1 we don't multiply times the constant).

@huonw
Copy link
Member

huonw commented Aug 28, 2013

Triage: will be fixed when #8823 lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants