[Github] Fetch number of commits in PR for docs action

[boomanaiden154]

This patches changes the docs action to run a fetch with a depth of the number of commits in the PR (1 if we're just running against a push event) which significantly increases the speed of the changed files event. The changed files event goes from taking ~30m to ~3s without any noticeable increase in fetch time.

[llvmbot]

@llvm/pr-subscribers-github-workflow

Author: Aiden Grossman (boomanaiden154)

Changes

This patch forces the files changed step in the test documentation Github action to use the Github API to look for changed files rather than using the local git history. Using the local git history takes a significant amount of time(~30m with no depth limit, ~10m with a depth limit of 100) while using the GH API is virtually instantaneous. This uses an extra GH API request, but given the frequency that this action runs with, that shouldn't be an issue.

---

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

1 Files Affected:

• (modified) .github/workflows/docs.yml (+9-9)

diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml
index d58c7d51e0e44ab..0603dfa3df19b21 100644
--- a/.github/workflows/docs.yml
+++ b/.github/workflows/docs.yml
@@ -25,6 +25,15 @@ jobs:
     name: "Test documentation build"
     runs-on: ubuntu-latest
     steps:
+      - name: Get subprojects that have doc changes
+        id: docs-changed-subprojects
+        uses: tj-actions/changed-files@v39
+        with:
+          files_yaml: |
+            llvm:
+              - 'llvm/docs/**'
+            clang:
+              - 'clang/docs/**'
       - name: Fetch LLVM sources
         uses: actions/checkout@v4
         with:
@@ -41,15 +50,6 @@ jobs:
         run: |
           sudo apt-get update
           sudo apt-get install -y cmake ninja-build
-      - name: Get subprojects that have doc changes
-        id: docs-changed-subprojects
-        uses: tj-actions/changed-files@v39
-        with:
-          files_yaml: |
-            llvm:
-              - 'llvm/docs/**'
-            clang:
-              - 'clang/docs/**'
       - name: Build LLVM docs
         if: steps.docs-changed-subprojects.outputs.llvm_any_changed == 'true'
         run: |

[tstellar]

Moving it before the checkout is what causes it to use the API? You might want to add a comment explaining this.

[boomanaiden154]

Yes. If there isn't a checkout, it uses the Github API. A comment is a good point. I'll add one in.

I'm planning on doing this for the code formatting action too but wanted to do it in a separate PR as I don't understand all the details of that action and want Tobias to make sure my changes won't disturb anything and I assume he's out for the weekend at this point.

[tru]

One thing that changed files also solved for us here is that it made sure that the git history was there for git-clang-format. I want you to test this when there is a big commit list between the PR head and the base commit. I would expect this to fail since git-clang-format won't be able to compute the diff.

[tru]

(I am mainly out for the weekend yeah)

[boomanaiden154]

I'll make sure to do some experimentation with the code formatting action. It seems like it tries to only fetch the commits between main and the tip of the PR, but it doesn't make sense why that takes 10 minutes on average. I'll play around with it and see what I can get working. It doesn't matter for the documentation since we only need the state after the top commit to test the build.

Enjoy your weekend!

[boomanaiden154]

Looks like this needs some work as this only works on PRs and not against main (which still requires history). Will need to figure out some proper mechanism to get this to work.

[boomanaiden154]

Updated. Apparently just setting fetch_depth to the number of commits in the PR makes everything go way more smoothly. Not sure why the changed files action takes so long in the first place, but at least this should be way faster.

[tstellar]

LGTM. If the speed is still an issue, the other solution would be to have separate workflow files for the clang and llvm docs.

[boomanaiden154]

LGTM. If the speed is still an issue, the other solution would be to have separate workflow files for the clang and llvm docs.

Thanks for the review! I don't think the speed was a huge issue, but it's definitely better to not waste tens of minutes doing essentially nothing. After this I think the speed on PRs should be more than fine (sub ten minutes). If we still need more speed, I'll work on splitting jobs/throwing all the dependencies into a container image.