Skip to content

Optimize PR test workflow (Simple starter version) #2581

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

Merged
merged 4 commits into from
Mar 26, 2025

Conversation

qwang98
Copy link
Collaborator

@qwang98 qwang98 commented Mar 24, 2025

This PR increases the number of test_slow PR test bins from 8 to 17, thereby strictly reducing the time spent on test_slow.

We have three types of runners:

  • warp-ubuntu-2404-x64-8x for: build, run_examples, bench. I think this is a paid server: https://www.warpbuild.com/, which @leonardoalt last changed in update CI ubuntu #2187, so I have a question: why don't we use the GitHub Workflow free ones? Besides, I think we can parallelize run_examples, which is a bottleneck of 38 minutes currently run single threaded, so there can be some immediate time improvement if we run it on multiple runner instances. Is this server charged by the number of runners or the number of minutes?
  • ubuntu-24.04 for: test_quick, test_estark_polygon, test_slow.
  • ubuntu-22.04 for: udeps.

BEFORE Optimization
Screen Shot 2025-03-24 at 17 25 11

AFTER Optimization
See run time of this PR.

Future Optimization
In #2580, I'm working on creating 14 bins with 4 threads each because we have 55 tests in total and each can run on a separate thread. This would reduce the test_slow run time to that of the longest test, which is ~10 minutes.

This requires computing the bin-thread assignment because nextest only supports the hash method for partitions, which isn't ideal in this case.

@qwang98 qwang98 changed the title Optimize Workflow (Simple version) Optimize PR Workflow (Simple starter version) Mar 24, 2025
@qwang98 qwang98 changed the title Optimize PR Workflow (Simple starter version) Optimize PR test workflow (Simple starter version) Mar 24, 2025
@qwang98 qwang98 marked this pull request as ready for review March 24, 2025 09:45
@qwang98 qwang98 requested a review from leonardoalt March 24, 2025 09:46
@leonardoalt
Copy link
Member

so I have a question: why don't we use the GitHub Workflow free ones?

@qwang98 they're too slow and have too little RAM

@leonardoalt
Copy link
Member

Is this server charged by the number of runners or the number of minutes?

Number of computation minutes, so it shouldn't matter how many runners we're using if we're faster/same.

@leonardoalt
Copy link
Member

I think we can parallelize run_examples, which is a bottleneck of 38 minutes currently run single threaded

Yea those are bad in general, because they don't re-use the build binaries. I think I/we need to re-think those tests anyway.

@qwang98
Copy link
Collaborator Author

qwang98 commented Mar 24, 2025

I think we can parallelize run_examples, which is a bottleneck of 38 minutes currently run single threaded

Yea those are bad in general, because they don't re-use the build binaries. I think I/we need to re-think those tests anyway.

I mean we can just create multiple bins for them to run, because we currently run 5 tests single threaded. (Unless our subscription doesn't support multiple concurrent runners).

@qwang98
Copy link
Collaborator Author

qwang98 commented Mar 24, 2025

I think we can parallelize run_examples, which is a bottleneck of 38 minutes currently run single threaded

Yea those are bad in general, because they don't re-use the build binaries. I think I/we need to re-think those tests anyway.

It's also not a bottleneck yet, because even with this PR, our bottleneck is build + test_slow (the latter depends on the former)...

@qwang98
Copy link
Collaborator Author

qwang98 commented Mar 24, 2025

so I have a question: why don't we use the GitHub Workflow free ones?

@qwang98 they're too slow and have too little RAM

Got it

@qwang98
Copy link
Collaborator Author

qwang98 commented Mar 26, 2025

I tested a few parameters and think 17 bins for test_slow is the best so far.

I think this can be merged for some immediate speedups.

@Schaeff Schaeff added this pull request to the merge queue Mar 26, 2025
Merged via the queue into main with commit 8ed4987 Mar 26, 2025
25 checks passed
@Schaeff Schaeff deleted the optimize-pr-tests-simple branch March 26, 2025 12:47
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.

3 participants