Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use container on Linux to run llvm-project-tests workflow #81349

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

tstellar
Copy link
Collaborator

No description provided.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 10, 2024

@llvm/pr-subscribers-github-workflow

Author: Tom Stellard (tstellar)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/81349.diff

1 Files Affected:

  • (modified) .github/workflows/llvm-project-tests.yml (+8-2)
diff --git a/.github/workflows/llvm-project-tests.yml b/.github/workflows/llvm-project-tests.yml
index 68b4a68d1af984..228ac0276861bf 100644
--- a/.github/workflows/llvm-project-tests.yml
+++ b/.github/workflows/llvm-project-tests.yml
@@ -58,6 +58,10 @@ jobs:
   lit-tests:
     name: Lit Tests
     runs-on: ${{ matrix.os }}
+    container:
+      image: ${{(startsWith(matrix.os, 'ubuntu') && format('ghcr.io/{0}/ci-ubuntu-22.04:latest', github.repository_owner)) || null}}
+      volumes:
+        - /mnt/:/mnt/
     strategy:
       fail-fast: false
       matrix:
@@ -77,6 +81,7 @@ jobs:
         with:
           python-version: ${{ inputs.python_version }}
       - name: Install Ninja
+        if: runner.os != 'Linux'
         uses: llvm/actions/install-ninja@main
       # actions/checkout deletes any existing files in the new git directory,
       # so this needs to either run before ccache-action or it has to use
@@ -108,8 +113,8 @@ jobs:
         run: |
           if [ "${{ runner.os }}" == "Linux" ]; then
             builddir="/mnt/build/"
-            sudo mkdir -p $builddir
-            sudo chown `whoami`:`whoami` $builddir
+            mkdir -p $builddir
+            extra_cmake_args="-DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang"
           else
             builddir="$(pwd)"/build
           fi
@@ -123,6 +128,7 @@ jobs:
                 -DLLDB_INCLUDE_TESTS=OFF \
                 -DCMAKE_C_COMPILER_LAUNCHER=sccache \
                 -DCMAKE_CXX_COMPILER_LAUNCHER=sccache \
+                $extra_cmake_args \
                 ${{ inputs.extra_cmake_args }}
           ninja -C "$builddir" '${{ inputs.build_target }}'
 

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One inline question, otherwise LGTM.

Do you have figures on how much this sped up the pipeline? Also, I'm assuming the plan is to enable this later for the other projects?

.github/workflows/llvm-project-tests.yml Outdated Show resolved Hide resolved
@tstellar
Copy link
Collaborator Author

Do you have figures on how much this sped up the pipeline? Also, I'm assuming the plan is to enable this later for the other projects?

Build Time of llvm, clang, lld, and lldb (not including running lit tests):

Without Container: 146m 45s
With Container: 109m 9s

So ~34% faster with the container

@boomanaiden154
Copy link
Contributor

Looks like things scale reasonably well when running in CI. I haven't done exact A/B testing with the container vs without it yet, so it's nice to see the gains are pretty significant.

@tstellar tstellar merged commit fe20a75 into llvm:main Feb 15, 2024
8 of 9 checks passed
tstellar added a commit to tstellar/llvm-project that referenced this pull request Feb 15, 2024
tstellar added a commit to tstellar/llvm-project that referenced this pull request Feb 16, 2024
tstellar added a commit that referenced this pull request Feb 18, 2024
@pointhex pointhex mentioned this pull request May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants