-
Notifications
You must be signed in to change notification settings - Fork 31
Speed up running of Ubuntu jobs ci #271
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Speed up running of Ubuntu jobs ci #271
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #271 +/- ##
===========================================
- Coverage 87.65% 54.31% -33.35%
===========================================
Files 4 8 +4
Lines 1742 2957 +1215
===========================================
+ Hits 1527 1606 +79
- Misses 215 1351 +1136 |
@maximusron This PR has not succeded in its goals. As you've been doing the work with regards to the cppyy tests, any guesses as to why its running much slower in CppInterOPs workflow? |
287dc80
to
d48c87a
Compare
@vgvassilev @maximusron can one of you merge this PR please? It provides an almost 3x speed increase in the time it takes Ubuntu jobs to run. It also provides a reliable number for the code coverage. Previously it was turned on for all Ubuntu jobs instead of just one for some reason. I kept it on for the latest llvm. |
@mcbarton I took a quick look, it looks like a few things are being reverted(snap installing valgrind, valgrind commands). I think your branch head is at an older point. Can you rebase into a single commit? |
@maximusron I made this change to the Valgrind, etc to be consistent with what is in the cppyy repo (see here https://github.com/compiler-research/cppyy/blob/3f284954281610c0f02a255729e6bf8daed63f28/.github/workflows/ci.yml#L768C1-L781C1 for the Valgrind in cppyy) . If you need to me to change the Valgrind back I can, but I feel it needs to be update in cppyy to be consistent between the repos. |
No its the other way around. I have been fixing memory issues on the CI on the InterOp repo and will move these things down to cppyy and backend once its done. Snap installing valgrind was because of a specific bug in glibc for <3.20 that incorrectly reported memory issues with ubuntu libraries. This is a WIP and should be done with clang specific suppressions that live in cppyy/etc. |
601521b
to
518a1ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be merged after the jobs are clear
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! I think this can be merged after the jobs are done
Why the code coverage dropped so drastically? |
@vgvassilev Don't really know why. Previously we had coverage turned on for all llvms (don't know why), so maybe the coverage we get is llvm dependent. Its probably something that needs to be investigated. |
Hm, strange. We should have it in one place indeed. |
@vgvassilev @maximusron Is this PR able to be merged? |
518a1ad
to
a1f4ff1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
The running of the cppyy test jobs currently takes 30 minutes plus to run in the CppInterOp ci on Ubuntu, but around 10 minutes in the cppyy repo workflow. This PR should address this issue and allow PR workflows to run almost 3 time quicker. It also allows for greater consistency between the repos in what they are running in their respective workflows.