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

Fix quadratic performance with nextest #8

Closed
sunshowers opened this issue Aug 29, 2023 · 8 comments · Fixed by #49
Closed

Fix quadratic performance with nextest #8

sunshowers opened this issue Aug 29, 2023 · 8 comments · Fixed by #49
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@sunshowers
Copy link
Member

Let's say you have 10k files. Datatest-stable iterates over all files currently to build the list of tests. Since nextest has a process-per-test model, each test will iterate over every file. That's not great.

Instead, if --exact is passed in, we should be able to pinpoint the exact test and just use that.

@sunshowers sunshowers added the help wanted Extra attention is needed label Aug 29, 2023
@sunshowers
Copy link
Member Author

sunshowers commented Aug 29, 2023

Here's how to address this:

  1. Detect situations in which this approach is valid. More specifically: --exact is passed in and --skip is not passed in, I believe.
  2. In those situations, figure out the path from the passed in test name.
  3. If NEXTEST=1 is set, fail tests if the path doesn't exist. This will be a check to ensure our code is correct -- it's better to break loudly than to not match any tests and pass silently.
  4. Add tests for all this, including weird test and path names. (For example, test or path names with :: embedded in them.)

I'd estimate around 4-6 hours of work for someone new to the project, with the bulk of that time being spent in writing tests.

You're encouraged to submit a PR even if your implementation isn't totally perfect. I'm happy to pick up the last 10-20%.

@sunshowers sunshowers added the good first issue Good for newcomers label Aug 29, 2023
@sunshowers sunshowers changed the title Quadratic performance with nextest Fix quadratic performance with nextest Aug 29, 2023
@poopsicles
Copy link

thinking of trying this out this weekend 🥳

@jalil-salame
Copy link
Contributor

@poopsicles if you aren't working on it, then I'd like to give it a try.

@poopsicles
Copy link

@jalil-salame yeah, that would be for the best

have fun 🥳

@jalil-salame
Copy link
Contributor

I thought I would have some free time these last few weeks, but I didn't, if anyone was holding back because of me, please don't and feel free to pick this up, just drop a comment if you start work on this.

@sunshowers
Copy link
Member Author

No worries, totally understandable. I'd still love it if someone picked it up!

@Kriskras99
Copy link

Kriskras99 commented Oct 31, 2023

Some performance numbers using cargo nextest run --release --status-level fail --final-status-level fail --no-fail-fast -E "test($name)":

"$name" testcases: time
""  65942 tests: 249.311s
"a"  7087 tests:  22.232s
"aa"  440 tests:   2.943s
"aaa"  19 tests:   1.963s
"aaaa"  4 tests:   1.842s
"aaaaa" 0 tests:   1.522s

All my test files are named by the hash of their content, so that's why adding more as reduces the amount of tests mostly evenly. This was run on a 16 core machine.

@zaneduffield
Copy link
Contributor

I'll pick this one up 😄.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants