compiler-research / CppInterOp

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

Reduce the cppyy builds to help with bot lags #226

Closed vgvassilev closed 2 months ago

vgvassilev commented 2 months ago

@mcbarton, @maximusron can you take a look. The idea is to have one latest release of llvm built with cppyy. If that does not help we will need to cut more...

mcbarton commented 2 months ago

@vgvassilev You will reduce the bot lag just as much as this PR, but without removing the other cppyy builds by building the caches as a single job before the CppInterOp, cppyy, xeus builds

mcbarton commented 2 months ago

@vgvassilev You will reduce the bot lag just as much as this PR, but without removing the other cppyy builds by building the caches as a single job before the CppInterOp, cppyy, xeus builds

After doing this I would combine the xeus-clang-repl and cppyy jobs. It should be possible to run the cppyy tests within the mamba envirnoment combining the jobs. Just last time I tried to do this the tests ran incredibly slowly on the Github runner, and I couldn't determine why at the time. That's the reason they are separate jobs in the first place.

vgvassilev commented 2 months ago

@vgvassilev You will reduce the bot lag just as much as this PR, but without removing the other cppyy builds by building the caches as a single job before the CppInterOp, cppyy, xeus builds

I do not know how to do that…

mcbarton commented 2 months ago

@vgvassilev You will reduce the bot lag just as much as this PR, but without removing the other cppyy builds by building the caches as a single job before the CppInterOp, cppyy, xeus builds

I do not know how to do that…

@vgvassilev I have just done this and put it in a new PR.

maximusron commented 2 months ago

It should be possible to run the cppyy tests within the mamba envirnoment combining the jobs.

I don't think its a good idea to combine them since logs can get cluttered and we usually would prefer a cleaner test environment. This can also entangle dependencies in the long run and have something fail because their requirements clash.

They can happen at the same stage in the CI but should be seperate jobs

maximusron commented 2 months ago

@vgvassilev I do think we should retain the cppyy builds for clang 17 and 16. There could be differences in the API behaviours in earlier clang versions that we may not be able to catch down the line if developed only with the latest clang

mcbarton commented 2 months ago

@maximusron I don't think we should be dropping the cppyy jobs that is suggested in this PR, but rather what is done in https://github.com/compiler-research/CppInterOp/pull/227 to stop multiple jobs making the same llvm cache wasting minutes on the Github runners . Any comments on this PR?

maximusron commented 2 months ago

@maximusron I don't think we should be dropping the cppyy jobs that is suggested in this PR, but rather what is done in #227 to stop multiple jobs making the same llvm cache wasting minutes on the Github runners . Any comments on this PR?

Yeah I think these jobs should not be dropped..

vgvassilev commented 2 months ago

@mcbarton, shall I close this one?

mcbarton commented 2 months ago

@vgvassilev Yes this PR should be closed.