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

Add navigation_depth to html_theme_options #674

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Eric-Arellano
Copy link
Contributor

When switching the docs at https://qiskit.org/documentation to Furo, we found that including the full left sidebar is prohibitively expensive:

  • Build time goes from 0.5 hours using our legacy Pytorch fork theme to 3.5 hours.
  • HTML page size goes from ~100KB to ~800KB.

We are planning to reorganize our site to work around this. We'll stop having a dedicated HTML page for each function and method. That dramatically ameliorates the problem, going from about 3.5 hours to 0.5 hours (vs. 10 minutes for Pytorch with this change.)

However, I realized it's trivial for Furo to allow customizing the navigation_depth, similar to themes like RTD. That provides an escape hatch for large projects to use Furo still.

A lower navigation_depth usually results in a worse user experience, so the new docs emphasize the tradeoffs of this option.

Currently not possible for theme inheritance to override navigation_depth

My first approach was to try to work around this in qiskit-sphinx-theme via Qiskit/qiskit_sphinx_theme#451. I fixed the HTML page size issue by overriding furo_navigation_toc.

But, I could not fix the build performance because I could not figure out how to turn off Furo's __init__.py from calling _compute_navigation_tree. In Sphinx, when you set inherit = furo in theme.conf, that calls all of Furo's extension code from __init__.py. I could not get monkeypatching to work either so that _compute_navigation_tree no-oped.

@jonathan343
Copy link

jonathan343 commented Jul 6, 2023

I also ran into this issue when updating my teams docs site which is very large. I agree it's something the Furo devs should consider. A work around is to use something like the sphinx-remove-toctrees extension which allows you to specify paths for files to remove from the toctree before the sidebar is build.

@Eric-Arellano
Copy link
Contributor Author

Thanks, @jonathan343 for pointing me to sphinx-remove-toctrees! I tried out today a similar approach to automatically remove all method pages from the left ToC: Qiskit/qiskit_sphinx_theme#474. That took the build down from 3.5 hours to 1.5 hours.

Unfortunately, 1.5 hours is still slow for us. So we're going to continue with two approaches:

  1. Reorganize the site to have fewer HTML pages, specifically no method pages
  2. Add navigation_depth to our theme that inherits Furo. We probably will stick with -1 for prod builds, but we want to set 1 or 2 in local development builds so that devs are more likely to iterate on the docs.

Due to Sphinx inheritance not allowing us to turn off Furo's __init__.py, it means we need to fully vendor Furo (with attribution per MIT license), then override its Python to add navigation_depth: Qiskit/qiskit_sphinx_theme#463.

So, it'd still be really helpful to our project if upstream Furo were to have navigation_depth directly - but I understand if the maintainers don't want to merge.

@jonathan343
Copy link

jonathan343 commented Jul 7, 2023

Last thing I'll comment on (incase you haven't considered) is the -j N option for sphinx-build which will:

Distribute the build over N processes in parallel, to make building on multiprocessor machines more effective. Note that not all parts and not all builders of Sphinx can be parallelized. If auto argument is given, Sphinx uses the number of CPUs as N.

Reference: https://www.sphinx-doc.org/en/master/man/sphinx-build.html#cmdoption-sphinx-build-j

This also might help you situation if the hardware you're building on supports it.

Good luck!

@pradyunsg
Copy link
Owner

pradyunsg commented Jul 7, 2023

What's the behaviour of this if you're on a page that has been trimmed from the sidebar? Is there any indication that you're on such a page?

My understanding is that there isn't, which isn't something I'm keen on. 😅

@pradyunsg
Copy link
Owner

A work around is to use something like the sphinx-remove-toctrees

FWIW, I wouldn't call it a workaround and would indeed suggest using that as an explicit mechanism that provides much more control than a "chop-off-beyond-depth" approach.

@pradyunsg
Copy link
Owner

Adding a x-ref to #289 (reply in thread) as well.

@Eric-Arellano
Copy link
Contributor Author

Eric-Arellano commented Jul 7, 2023

What's the behaviour of this if you're on a page that has been trimmed from the sidebar? Is there any indication that you're on such a page?

It will highlight the closet parent page possible. E.g. this is on a subpage of Electron:

Screenshot 2023-07-07 at 1 57 17 PM

I agree that's not a great user experience. Hence, we're planning on still using -1 in production builds. We only want to set 1 or 2 in local docs builds so that devs can build our docs in ~5-10 minutes rather than ~30 when iterating. I'm happy to reword the docs here to emphasize production builds should almost certainly use -1.

We could even name the variable something like dev_only__navigation_depth?

Also, this downside could be mitigated by breadcrumbs, which is how we deal with the UX problem in Pytorch, where we currently have the depth at 1. I intend to add breadcrumbs to qiskit-sphinx-theme and I'll be glad to upstream this to Furo: #139 (reply in thread).

@Eric-Arellano
Copy link
Contributor Author

For posterity, Pradyun and I have been investigating the performance issues from the left sidebar more in Qiskit/qiskit_sphinx_theme#328. tl;dr: the latest experiment suggests Furo's slowdown is entirely from the navigation_depth.

I'm proposing reworking this PR to be solely intended to speed up dev builds; it should not be used for prod builds. E.g. we rename the variable to dev_only_navigation_depth.

@Eric-Arellano
Copy link
Contributor Author

FYI here is how Pydata-sphinx-theme sped up this issue just recently: pydata/pydata-sphinx-theme#1609. (Also happy new year! 🥳)

@pradyunsg pradyunsg added the configuration Affects one or more configurable behaviours label Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Affects one or more configurable behaviours
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants