Skip to content

Conversation

@marc-hb
Copy link
Contributor

@marc-hb marc-hb commented May 5, 2023

This reverts commit 4516117.

A git bisect showed that the duration of an incremental build doubled
after this commit enabled numfig=True. Measurements shared and
discussed in #37572, #55708 and #56631 confirmed this. Here are yet more
measurements below in two different system configurations building docs
for very recent Zephyr commit b10817b + all :numref: removed by
the previous commit. In other words these numbers show the cost of
numfig=True without even using :numref:.

  • Ubuntu 22, 8 cores
    sphinx-build --version 4.3.2
time make -C doc/ html-fast

                           numfig=True   numfig=False

- from scratch             7 min 15 s       6 min 40 s

- one-line .rst change           48 s             24s
  • Current Arch Linux, 72 cores
    sphinx-build --version 6.2.1
                           numfig=True   numfig=False

- from scratch              5 min 0 s      4 min 50 s

- one-line .rst change           37 s            18 s

Signed-off-by: Marc Herbert [email protected]

Thalley
Thalley previously requested changes May 8, 2023
@marc-hb
Copy link
Contributor Author

marc-hb commented May 8, 2023

I just took @gmarull 's commit as is from #56631. Will pay more attention to it now, thanks for the reviews.

gmarull and others added 2 commits May 8, 2023 21:41
Usage of numfig=True option in conf.py significantly increases doc build
time. While it is a nice feature, it's not extensively used in Zephyr
documentation, so let's remove its usage in favor of faster doc builds.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
This reverts commit 4516117.

A git bisect showed that the duration of an incremental build doubled
after this commit enabled `numfig=True`. Measurements shared and
discussed in zephyrproject-rtos#37572, zephyrproject-rtos#55708 and zephyrproject-rtos#56631 confirmed this. Here are yet more
measurements below in two different system configurations building docs
for very recent Zephyr commit b10817b + all `:numref:` removed by
the previous commit. In other words these numbers show the cost of
`numfig=True` _without_ even using `:numref:`.

* Ubuntu 22, 8 cores
  sphinx-build --version 4.3.2

                           numfig=True   numfig=False

- from scratch             7 min 15 s       6 min 40 s

- one-line .rst change           48 s             24s

* Current Arch Linux, 72 cores
  sphinx-build --version 6.2.1

                           numfig=True   numfig=False

- from scratch              5 min 0 s      4 min 50 s

- one-line .rst change           37 s            18 s

Signed-off-by: Marc Herbert <[email protected]>
@marc-hb marc-hb dismissed stale reviews from Thalley and mbolivar-nordic May 8, 2023 22:09

fixed

@Thalley
Copy link
Contributor

Thalley commented May 15, 2023

@marc-hb Not opposing this change, but have you/we considered just omitting numfig for PRs, but keep it in for "release build documentation"?

@marc-hb
Copy link
Contributor Author

marc-hb commented May 15, 2023

@marc-hb Not opposing this change, but have you/we considered just omitting numfig for PRs, but keep it in for "release build documentation"?

This change does not help PRs that much actually because they build from scratch too. It's all about incremental build / iterative, local .rst compilation.

I think your idea is still valid and in fact this is more or less what I tried in #55708 but I believe the consensus there was "let's keep it simple and just get rid of numfig, it's not even that useful".

BTW I intend to return to #55708 once the numfig dust settles down because 55708 was not just about numfig.

@carlescufi carlescufi merged commit 9d11943 into zephyrproject-rtos:main May 22, 2023
@marc-hb marc-hb deleted the nonumfig branch May 22, 2023 21:59
marc-hb added a commit to marc-hb/zephyr that referenced this pull request May 23, 2023
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]>
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.

6 participants