-
Notifications
You must be signed in to change notification settings - Fork 104
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
Conversation
@qwang98 they're too slow and have too little RAM |
Number of computation minutes, so it shouldn't matter how many runners we're using if we're faster/same. |
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). |
It's also not a bottleneck yet, because even with this PR, our bottleneck is |
Got it |
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. |
This PR increases the number of
test_slow
PR test bins from 8 to 17, thereby strictly reducing the time spent ontest_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 parallelizerun_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

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.