-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Use multiple threads by default. Limits tests to one thread. Do some renaming. #57948
Conversation
This comment has been minimized.
This comment has been minimized.
@@ -150,9 +150,9 @@ newtype_index! { | |||
// The interner is pointed to by a thread local value which is only set on the main thread | |||
// with parallelization is disabled. So we don't allow `Symbol` to transfer between threads | |||
// to avoid panics and other errors, even though it would be memory safe to do so. | |||
#[cfg(not(parallel_queries))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CFGs like these seem very prone to invisible breakage (if we end up not running either configuration on CI). Perhaps we should maintain parallel-able code in all configurations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do check both on PRs on Travis at least.
@@ -1194,8 +1194,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, | |||
"prints the llvm optimization passes being run"), | |||
ast_json: bool = (false, parse_bool, [UNTRACKED], | |||
"print the AST as JSON and halt"), | |||
query_threads: Option<usize> = (None, parse_opt_uint, [UNTRACKED], | |||
"execute queries on a thread pool with N threads"), | |||
threads: Option<usize> = (None, parse_opt_uint, [UNTRACKED], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any support for jobserver or is this a static quantity? Perhaps the documentation should be adjusted either way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will use a jobserver once #56946 lands
This comment has been minimized.
This comment has been minimized.
(I'm assuming we will want a perf run here?) @bors try |
⌛ Trying commit fd9d9ee with merge a8571c0a49643e461ab10f2410709f5a9e9c1b00... |
☀️ Test successful - checks-travis |
📌 Commit fd9d9ee has been approved by |
Use multiple threads by default. Limits tests to one thread. Do some renaming. r? @michaelwoerister
☀️ Test successful - checks-travis, status-appveyor |
☀️ Test successful - checks-travis, status-appveyor |
r? @michaelwoerister