Skip to content
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

Consider (re?)enabling parallelism for debuginfo tests. #72719

Closed
eddyb opened this issue May 29, 2020 · 8 comments
Closed

Consider (re?)enabling parallelism for debuginfo tests. #72719

eddyb opened this issue May 29, 2020 · 8 comments
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented May 29, 2020

AFAICT debuginfo tests are way slower than other kinds of tests due to being executed sequentially (I haven't properly checked the source but I couldn't see many threads in htop and it looked sequential).

This is especially noticeable on machines with 16 or more hardware threads, where other test suites make visible progress at significantly faster rates.

@eddyb
Copy link
Member Author

eddyb commented May 29, 2020

The other thing I've noticed before but I wasn't sure about, is that there seems to be no skipping of previously-passing tests.

@LeSeulArtichaut LeSeulArtichaut added A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 29, 2020
@jonas-schievink jonas-schievink added the A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) label May 29, 2020
@tromey
Copy link
Contributor

tromey commented Mar 18, 2022

src/tools/compiletest/src/main.rs says:

    // Some older versions of LLDB seem to have problems with multiple
    // instances running in parallel, so only run one test thread at a
    // time.
    env::set_var("RUST_TEST_THREADS", "1");

This dates back to 2014 (and maybe before but I didn't dig any deeper):

commit ca0446d10795691e3fc206794ac7fbd441442b34
Author: Michael Woerister <michaelwoerister@posteo>
Date:   Sun Oct 26 10:57:29 2014 +0100

    debuginfo: Set RUST_TEST_TASKS to 1 again for LLDB tests

Maybe such old versions of lldb aren't relevant any more.

@tromey
Copy link
Contributor

tromey commented Mar 18, 2022

Removing that line sure seems to help. The question then is whether it can be removed. @michaelwoerister - you added that, do you recall?

I don't know anything about skipping already-passed tests.

@michaelwoerister
Copy link
Member

As far as I remember, back in 2014 LLDB tests failed reliably when run in parallel. That was the only reason why we made them run one at a time. If that's not a problem anymore, we should run them in parallel, yes.

For GDB and CDB tests are already running in parallel, right?

@tromey
Copy link
Contributor

tromey commented Mar 18, 2022

For GDB and CDB tests are already running in parallel, right?

I believe so.

tromey added a commit to tromey/rust that referenced this issue Mar 18, 2022
Debuginfo tests are serialized due to some older version of LLDB.
However, that comment was last touched in 2014, so presumably these
older versions are long since obsolete.

Partially fixes bug rust-lang#72719.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Mar 18, 2022
Re-enable parallel debuginfo tests

Debuginfo tests are serialized due to some older version of LLDB.
However, that comment was last touched in 2014, so presumably these
older versions are long since obsolete.

Partially fixes bug rust-lang#72719.
@tromey
Copy link
Contributor

tromey commented Mar 18, 2022

I don't know anything about skipping already-passed tests.

I think this already happens, since I see this i output:

running 268 tests
i.iFFiiiiiiiiiiiiiiiiiiiiiii.iiiiii.iiiiiiiiiiiiiiiiiiiiiiiiiiiiiii..Fi.iiiiiiiiiiiiiiFiiiiiiiiiiiii 100/268
iiiiii.ii..ii.iiiiiiiiiii.iiiiiii.i.iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii 200/268
i.i.iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii.ii..iiiiiiiiiiiiii.iiiiiii.

@tmiasko
Copy link
Contributor

tmiasko commented Mar 18, 2022

I don't know anything about skipping already-passed tests.

I think this already happens, since I see this i output:

I think I fixed this back in #68391 (the spurious hash mismatch mentioned in the pull request description).

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Mar 19, 2022
Re-enable parallel debuginfo tests

Debuginfo tests are serialized due to some older version of LLDB.
However, that comment was last touched in 2014, so presumably these
older versions are long since obsolete.

Partially fixes bug rust-lang#72719.
@tromey
Copy link
Contributor

tromey commented Mar 19, 2022

I think this is fixed now.

@tromey tromey closed this as completed Mar 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants