-
Notifications
You must be signed in to change notification settings - Fork 681
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
Improve the CI time #1463
Comments
how can we improve it? Maybe we can list the action items so anyone can work on it |
@seladb let's brainstorm some ideas to improve this. cc @Dimi1010 @egecetin @clementperon |
I didn't look at the pipeline but using a cache + ccache could avoid to rebuild some files. |
My suggestion is that if the cache is not enough, we can distribute the workflow to several providers to prevent reaching compute limits. For example,
It should make it harder to maintain a bit, but we can handle it if we have to |
@egecetin I think it could be the last step. The issue is the current CI script is not efficient enough. Just like that we need to improve the algorithm before trying to use multithread(?) |
@tigercosmos Yes of course. If possible, I agree we should improve the pipeline first. To be honest I did not check all workflows but most of the runs between 5-8 mins (approx 2-3 mins from build so it is like 30%-50% of run is build), right? So, I'm not sure we can dramatically improve CI runtime (even with caching) unless we limit the runner OS/version/config or improve parallelism. I think for "multiplatform" definition the code should be tested as much as possible. So, I don't know what we can do. |
Something that comes to my mind, testing consumes approx 1 minute. If I'm not mistaken current test suite does not support multithreading (@seladb correct me if I'm wrong pls). So maybe using a test library supporting multithreading might lead better testing times? (eg. integrating GoogleTest + ctest to run tests parallel to efficiently saturate CPU) or maybe we can just remove "Check Installation" step which can halve the workflow time but not sure about this. |
@egecetin true, I think the parallelism of test cases may be the key. Probably we should focus on it in the next step. |
@seladb what do you think? |
The "Check installation" step re-builds the entire library, although it was built just a few steps earlier. Maybe we can use cache to prevent that? Regarding the tests - they indeed take between 1 min (on Linux) and 4 mins (on Windows), so parallelism may help. But replacing the whole testing framework will take a very long time and I'm not sure it's worth the relatively small runtime gain... |
we have 39 checks now, if we can save one minute for each, then we save 40 mins. I think both cache and test parallelism are worth a try. We don't need to switch a new testing framework, I think it's not hard to introduce parallelization in the current one. |
sure, we can try both. Hopefully it can save some of the CI time |
I will tried to work on the test parallelization |
@tigercosmos Maybe we can close this issue what do you think? With ccache and disabling old commit runners for PRs, CI performance improved and moving to Gtest is mostly related to improving test framework itself for now not for CI performance. |
I agree we can close it 👍 |
I expect parallelism, but if we are surely moving to gtest, then this can be closed. |
We have many platforms; it's about 30s for now. If we can save 30 seconds for each, it turns out to be 15 minutes (of course, this number will be divided by the number of runners, which is 3 or 4). I think it's a huge improvement. |
Also, if we can improve the efficiency of the testing, it will run faster when we do the local testing, where the build time can be ignored in this case. |
Most of the runners are running in parallel, so even if we achieve a performance improvement of 20-30 seconds per run, it won't give more than 1-1.5 minutes in overall CI runtime. I'm not sure it's worth the effort... |
As previously mentioned, local testing is time-consuming, and we should improve its speed. We can either continue with this issue or open a new one specifically focused on reducing testing time. |
currently, we only bind to one interface, which restricts the possibility of parallelism, so we may bind to multiple interfaces (including virtual ones) if possible. |
I'm not sure it is easy to distribute tests to all interfaces and get consistent results, it will probably require more work 🤔. Just for curiosity what is current time approx for local environments. For me around 2.5 mins which is not bad, I think. Most of time I'm not testing whole library locally, just test changed module (Packet++ or Pcap++ only for example) |
Why would running each test case on different interfaces yield different results? We're parallelizing the test cases, but not each individual test case. Typically, I have to comment out any unnecessary tests to speed things up, which is inconvenient. Perhaps we could add a test-filtering flag, similar to what other testing frameworks provide. |
We do have test-filtering flags:
These tags can also be a single test, for example:
Or:
Or:
|
Thanks! It's good to know this. I guess we just need to support running tests in parallel. As there is still room for improvement in CI, I would suggest closing the issue later. |
@tigercosmos I thought the |
Currently, we have too many GitHub action items, which results in a long building and testing time.
We need to refactor the current CI files and make the CI faster.
The text was updated successfully, but these errors were encountered: