search

compiler-research / CppInterOp

A Clang-based C++ Interoperability Library
Other
38 stars 20 forks source link

Speed up running of Ubuntu jobs ci #271

Closed mcbarton closed 1 month ago

mcbarton commented 1 month ago

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.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 54.31%. Comparing base (f93e7b6) to head (a1f4ff1). Report is 27 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/271/graphs/tree.svg?width=650&height=150&src=pr&token=7UWTYSVVT5&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research)](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/271?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research) ```diff @@ 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 ``` [see 8 files with indirect coverage changes](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/271/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research) [see 8 files with indirect coverage changes](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/271/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research)
mcbarton commented 1 month ago

@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?

mcbarton commented 1 month ago

@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.

maximusron commented 1 month ago

@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?

mcbarton commented 1 month ago

@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.

maximusron commented 1 month ago

@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.

vgvassilev commented 1 month ago

Why the code coverage dropped so drastically?

mcbarton commented 1 month ago

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.

vgvassilev commented 1 month ago

Hm, strange. We should have it in one place indeed.

mcbarton commented 1 month ago

@vgvassilev @maximusron Is this PR able to be merged?