compiler-research / CppInterOp

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

[ci] Reduce bot lag by building cache first #227

Closed mcbarton closed 2 months ago

mcbarton commented 2 months ago

@vgvassilev This should reduce the bot lag as much as https://github.com/compiler-research/CppInterOp/pull/226 while keeping all the jobs.

mcbarton commented 2 months ago

image

The CI is now in 3 stages. The first builds the caches if it cannot find them. It then builds the non wasm builds, and if they pass all tests, it then goes onto build the wasm builds. Eventually I hope to be able to make use of this https://github.com/github/roadmap/issues/826 so that we are not dependent on a cache at all, and can work with vms which have the patched llvms included.

vgvassilev commented 2 months ago

Do we really need the arm builds? Maybe arm for the latest clang maybe not even that…

mcbarton commented 2 months ago

@vgvassilev My suggestion is to keep the osx arm ci job coverage, but drop the osx x86 jobs. If it works on the arm then it is likely to work on the x86. We also have the Ubuntu builds to test x86z The issue with the osx builds is that you can only have 5 running simultaneously over the whole organisation. If we drop osx x86 and merge the xeus-clang-repl and cppyy jobs, then the number of osx jobs drop in CppInterOp reduces to a quarter of its current value.

mcbarton commented 2 months ago

osx arm is more important that osx x86 going forward now that Apple has moved to an Arm based architechture. In a few years news osx operating systems won't even support x86.

mcbarton commented 2 months ago

@vgvassilev Will changing the build type for the llvm from Release to MinSizeRel make much of a difference to the size it takes up in the cache? I don't have much an idea of how much the different build types take up.

vgvassilev commented 2 months ago

@vgvassilev Will changing the build type for the llvm from Release to MinSizeRel make much difference to the size it takes up in the cache? I don't have much an idea of how much the different build types take up.

It will make a difference if we build only the clang and clang-repl targets as done in my PR.

mcbarton commented 2 months ago

@vgvassilev Will changing the build type for the llvm from Release to MinSizeRel make much difference to the size it takes up in the cache? I don't have much an idea of how much the different build types take up.

It will make a difference if we build only the clang and clang-repl targets as done in my PR.

I already made these changes in this PR (with the also needed lld which I mention in the other PR)

mcbarton commented 2 months ago

After @maximusron comment in the other PR about suggesting against merging the xeus-clang-repl and cppyy jobs I won't be doing this now, but I still think its better to drop the osx x86 jobs instead of the osx arm ones.

maximusron commented 2 months ago

Looks good to me, one comment would be that 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

@mcbarton yeah the OS X builds seem to be excessive, reducing the number of osx x86 builds seems like a good idea to me (maybe keep just the CppInterOp build for a sanity check that the API works)

mcbarton commented 2 months ago

@mcbarton yeah the OS X builds seem to be excessive, reducing the number of osx x86 builds seems like a good idea to me (maybe a few as a sanity check)

Tomorrow morning i'll add another commit to this PR removing all but one of the osx x86 builds as the sanity check (probably the latest llvm, with the cpppyy option as the xeus-clang-repl only currently checks the config file isn't wrong, which is done on the arm side).

vgvassilev commented 2 months ago

There is a problem with the wasm builds. Shall we merge to start rebuilding the caches in the master and reuse them?

mcbarton commented 2 months ago

@vgvassilev clear the cache, merge this PR and then I will look at the output of the ci to determine the problem with the wasm builds.

vgvassilev commented 2 months ago

Done