ci: doc-build: add new check time-incremental-build #58165
Conversation
marc-hb
force-pushed
the
time-doc-build
branch
5 times, most recently
from
e62669d to
80567e3
Compare
The ability to make a one-line .rst change and quickly regenerate the docs has been broken at least three times in the past: zephyrproject-rtos#13159, zephyrproject-rtos#36033 and zephyrproject-rtos#57624. Add a small, final check to make sure this can't happen again. Signed-off-by: Marc Herbert <[email protected]>
marc-hb
requested review from
Thalley,
carlescufi,
gmarull,
kartben,
keith-zephyr and
mbolivar-nordic
| # Keep this last not to break upload and other "functional" tests above. | ||
| - name: time-incremental-build | ||
| if: github.event_name == 'pull_request' | ||
| run: | | ||
| # Catch major incremental build time regressions like the ones | ||
| # fixed by commits 52842317505f, a140a249b33b and 9d119431945f. | ||
| sed -i -e 's/Zephyr /Xephyr /' doc/index.rst # make a small .rst change | ||
| export DOC_TAG=development SPHINXOPTS='-j auto -t publish' # as above | ||
| time timeout 35 make -C doc/ html-fast # ${DOC_TARGET} as above | ||
| git checkout HEAD -- doc/index.rst # restore to be safe |
There was a problem hiding this comment.
I don't see the need to add such hacks in CI. If something breaks, one can open an issue. GH actions also support timeouts.
There was a problem hiding this comment.
No strong opinion on what to do here; though, given that there were multiple changes in the past that resulted in increased incremental build time and that seems to be a recurring problem, I can see this being useful in catching problems like that before they get merged; moreover, at most, it only adds 35 seconds to the workflow run time.
There was a problem hiding this comment.
I don't see the need to add such hacks in CI. If something breaks, one can open an issue.
The purpose of CI is to catch regressions when they happen. It took me a day to isolate the numfig regression, long after it was merged. I don't make doc changes every day, very far from it.
Few people contribute to documentation at all and from past experience it seems many of them don't notice how slow the incremental build becomes; they just assume that "building documentation is very slow" - which I think makes even fewer people contribute to documentation.
GH actions also support timeouts.
I will take a look, thanks. See comments below.
There was a problem hiding this comment.
+1
Re: GH Actions, it's not clear if this would allow to set a timeout of less than a minute https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepstimeout-minutes
There was a problem hiding this comment.
We would not want to use the timeout-minutes here since that is supposed to be the last resort when something gets stuck, which is not the case here. We want the job result to be "failed" (i.e. not "timeout") when incremental build takes too long.
| # fixed by commits 52842317505f, a140a249b33b and 9d119431945f. | ||
| sed -i -e 's/Zephyr /Xephyr /' doc/index.rst # make a small .rst change | ||
| export DOC_TAG=development SPHINXOPTS='-j auto -t publish' # as above | ||
| time timeout 35 make -C doc/ html-fast # ${DOC_TARGET} as above |
There was a problem hiding this comment.
Probably better to be explicit about the timeout units.
| time timeout 35 make -C doc/ html-fast # ${DOC_TARGET} as above | |
| time timeout 35s make -C doc/ html-fast # ${DOC_TARGET} as above |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
|
I started making some doc changes again and the incremental, zero-change, hot cache build is now over 30 seconds on my server system with 72 cores and 300G of RAM. I don't know why: time seems to be spent a little bit everywhere now. So this PR was really a lost cause. There's probably an equivalent to Zawinkski's "Law of Software Envelopment" but for documentation builds. |
|
Interesting, thanks! I once tried to debug an incremental build issue in Breathe and I had to stop for mental health reasons so for sure I won't miss breathe: I see your PR 73671 brags about a 3 minutes build time from scratch which seems very impressive! However I don't see yet any claim about incremental build times which is what I believe people use most of the time (or at least should) and also what this PR was about. |
6 participants
The ability to make a one-line .rst change and quickly regenerate the docs has been broken at least three times in the past: #13159, #36033 and #57624. Add a small, final check to make sure this can't happen again.