Skip to content

use a single test task when valgrinding #1921 #10250

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
wants to merge 3 commits into from

Conversation

reedlepee123
Copy link
Contributor

No description provided.

@reedlepee123
Copy link
Contributor Author

r? @brson

_ => fail!("RUST_TEST_TASKS is `{}`, should be a positive integer.", s)
if running_on_valgrind() {
1
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The convention is for this to be formattted like

    1
} else {
    match ...

with the else and the braces all on one line.

_ => fail!("RUST_TEST_TASKS is `{}`, should be a positive integer.", s)
if running_on_valgrind() {
1
} else{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else {, with the space.

@reedlepee123
Copy link
Contributor Author

rebased!

@reedlepee123
Copy link
Contributor Author

r? @brson

@brson
Copy link
Contributor

brson commented Nov 5, 2013

Oh, no. I've realized this may not be the right way to make this fix. This performance problem is only an issue for those tests that are not run by our compiletest driver - if we make the fix this way then it will probably speed up execution of our normal integrated test drivers, but vastly slow down the execution of the compiler tests (src/run-pass, compile-fail, etc.).

I wonder how worthwhile it is to do this. @reedlepee123 can you measure the difference in time it takes to run make check-stage1-std CFG_ENABLE_VALGRIND=1 both with and without this fix (you'll need valgrind installed), and also let me know how many cores you have? I'd like to know what impact this change has on testing performance; it may not make sense to do it at all. Be sure to run the command twice each time and only measure the second run, otherwise you could be measuring the time it takes to perform the rest of the build.

@catamorphism
Copy link
Contributor

Closing for lack of activity. @reedlepee123 , feel free to re-open it if you work on it more!

flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 10, 2023
wildcard_enum_match_arm lint takes the enum origin into account

fixes rust-lang#7419

---

changelog: Enhancement: [`wildcard_enum_match_arm`]: Now lints missing private variants, for local enums
[rust-lang#10250](rust-lang/rust-clippy#10250)
<!-- changelog_checked -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants