Replies: 5 comments 10 replies
-
The cache limit is 10GB (source). A full debug build creates a directory of about 3GB, however, half of it are the linked executables. So it would help to run
This would mean to cache the executables. I don't think it is worth the cache space. I am currently working on improving the cache usage regarding the vcpkg builds to reduce the number of required package rebuilds. |
Beta Was this translation helpful? Give feedback.
-
|
I'm now experimenting with ccache locally. I wonder if we could merge the |
Beta Was this translation helpful? Give feedback.
-
OK, then how about using the last daily build for these? … But we only have appimages. I'm not familiar with them. |
Beta Was this translation helpful? Give feedback.
-
Can you give estimations on the ccache size?
This is especially true for the static Windows builds. Does ccache can also be used here?
Currently we only have clang-10 Release and Debug builds as AppImages. Also they are designed to be standalone, so you can just download and execute it. All data files are packed into the image, but |
Beta Was this translation helpful? Give feedback.
-
A release build on my debian oldstable system created a 212MB uncompressed cache (total of
No, it only caches the object files. It doesn't handle linking at all.
Well, I guess one release and one debug build would be enough for this case. It's rather unlikely that such a change would hit a compiler incompatibility issue, and in the worst case, it would be found by the next full run.
I'm afraid that complicates things for |
Beta Was this translation helpful? Give feedback.
-
|
I use ccache for all builds locally now, and it saves some time when switching branches in the same worktree, but in the end it does much less than I expected. When I first read about its existence, I thought it was a feature of the compilers themselves, and worked at lower levels. But it turns out all it does is checks whether anything changed in each compile unit (ie. a source file and all its includes), and caches the resulting object files (mostly the same as what I did for clang-tidy in #5922, only they can usually skip running the preprocessor to detect hits). This means that if there's even just a typo change in some low level header file, then a large part of the codebase needs recompiling. I also checked the testsuite runs again, and build times are actually only 15-20 minutes. It's the regression test suite that takes more than an hour. So with the large cache size it's probably not worth it to cache every build. But maybe it could still be used for 1 or 2 "main" configs that are always built first, and the rest of the matrix would only start if the compilation phase of these succeeds. (that'll need some advanced workflow trickery though). Also these would be used for the regression tests when there are no changes in |
Beta Was this translation helpful? Give feedback.
-
|
And for clang-tidy, we can try https://github.com/freedick/cltcache |
Beta Was this translation helpful? Give feedback.
-
|
Well, it didn't work for us (it expects compiler arguments on the clang-tidy command line, not in |
Beta Was this translation helpful? Give feedback.
-
|
Most time is spent in running tests. I did an analysation of duration of tests run for one run, for https://github.com/widelands/widelands/actions/runs/5556166018/jobs/10148384149?pr=6017 shortly my summary:
Here is the duration for each test (total duration is 1:24:20) (table is from calc, I can attach the calc file if desired)
|
Beta Was this translation helpful? Give feedback.
-
|
From #6020 Current behaviour Expected behaviour either some (include) paths for ... notes |
Beta Was this translation helpful? Give feedback.
-
|
I can not push a change which modifies Well, then I include the patch here: From dee85abfae98c6195b93440fbe72095e1cc69065 Mon Sep 17 00:00:00 2001
From: Simon Heimberg <simon.heimberg@heimberg-ea.ch>
Date: Mon, 17 Jul 2023 18:08:35 +0200
Subject: [PATCH 1/2] only run tests and clang_tidy if a relevant file has
changed
fixes #6020
enhances #5880
---
.github/workflows/build.yaml | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index 290748fcbb..4ec7f0be87 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -81,6 +81,9 @@ jobs:
clang_tidy:
runs-on: ubuntu-latest
+ # Set job outputs to values from changed_files step for other jobs
+ outputs:
+ run_test_suite: ${{ steps.changed_files.outputs.test_suite }}
env:
CC: /usr/bin/clang
CXX: /usr/bin/clang++
@@ -89,9 +92,30 @@ jobs:
uses: actions/checkout@v3
with:
fetch-depth: 1
+ - name: check for changed files
+ uses: dorny/paths-filter@v2
+ id: changed_files
+ with:
+ # groups of files to check for changed files
+ filters: |
+ clang_tidy:
+ - 'src/**'
+ - 'utils/run-clang-tidy.py'
+ compile: &compile
+ - 'CMakeLists.txt'
+ - 'cmake/**'
+ - 'src/**'
+ test_suite:
+ - *compile
+ - 'data/**'
+ - 'regression_test.py'
+ - 'test/**'
- name: Installing dependencies
+ if: &run_clang_tidy steps.filter_files.outputs.clang_tidy == 'true'
run: sh ./.github/scripts/install_deps.sh
- name: CMake
+ # *run_clang_tidy is reference to above &run_clang_tidy
+ if: *run_clang_tidy
run: |
# Prepare build dir
echo "Will use $(nproc) cores."
@@ -101,6 +125,7 @@ jobs:
popd
- name: Restore cache
uses: actions/cache/restore@v3
+ if: *run_clang_tidy
with:
path: build/clang-tidy-cache
key: clang-tidy-cache-${{ github.sha }}
@@ -109,6 +134,7 @@ jobs:
- name: Check
# To run a single check, call:
# python3 ../utils/run-clang-tidy.py -checks=-*,my-check-prefix* | tee ../clang-tidy.log
+ if: *run_clang_tidy
run: |
# run-clang-tidy.py
pushd build
@@ -116,11 +142,13 @@ jobs:
popd
- name: Save cache
# We want to save the cache even if the report fails because of failed files
+ if: *run_clang_tidy
uses: actions/cache/save@v3
with:
path: build/clang-tidy-cache
key: clang-tidy-cache-${{ github.sha }}
- name: Report
+ if: *run_clang_tidy
run: utils/check_clang_tidy_results.py clang-tidy.log
lua_style:
@@ -140,6 +168,7 @@ jobs:
testsuite:
needs: [clang_tidy, documentation, codecheck, misc_sanity_checks, lua_style]
+ if: needs.clang_tidy.outputs.run_test_suite == 'true'
strategy:
fail-fast: false
matrix:
--
2.39.2 |
Beta Was this translation helpful? Give feedback.
-
|
I pushed the patch here: #6028 I also sent you an invitation to the dev team, then you should be able to push workflow changes |
Beta Was this translation helpful? Give feedback.
-
|
This is in work and could be done soon. Some tasks are in a separate thread/comment/something below. |
Beta Was this translation helpful? Give feedback.
-
|
Some tasks from discussion in a pr (see links below):
|
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
@widelands/developers
It looks like we can run fewer workflow jobs in parallel than before, so I wonder if we could make better use of the available resources and speed up PR testing by optimising our workflows.
My ideas for optimisation so far:
src/then don't even rebuild, and only re-run regression tests ifdata/ortest/changedclang-tidyonly on changed files? Though headers are difficult, because we need to track down all places where they are included, even transitively… (again, master would still get full check)Does this make sense? Do you have more ideas?
Beta Was this translation helpful? Give feedback.
All reactions