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

Override RTD build commands to speed up docs build #933

Merged
merged 1 commit into from
Oct 7, 2022

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Aug 8, 2022

Similar to python/peps#2728.

ReadTheDocs now has a beta feature to override the build commands:

Right now, a whole bunch of dependencies are installed that we don't use.

This can be slow especially as some old dependencies are built from source because they don't support modern Python and don't have wheels available (e.g. readthedocs/readthedocs.org#9118).

Instead, we can just make html to only install the stuff we need.

This speeds up the command-running time from ~61 to ~22 seconds (the upload is a constant ~30s in both cases).

Before

image

Each build step took:

  1. Command time: 0s
  2. Command time: 0s
  3. Command time: 0s
  4. Command time: 0s
  5. Command time: 0s
  6. Command time: 1s
  7. Command time: 11s
  8. Command time: 38s
  9. Command time: 3s
  10. Command time: 0s
  11. Command time: 8s

Total command time: 61s

Build took 94 seconds (includes command time and upload time)

https://readthedocs.org/projects/cpython-devguide/builds/17656911/

After

image

  1. Command time: 0s
  2. Command time: 0s
  3. Command time: 0s
  4. Command time: 0s
  5. Command time: 22s

Total command time: 22s

Build took 49 seconds (includes command time and upload time)

https://readthedocs.org/projects/hugovk-devguide/builds/17657155/

Copy link
Member

@ezio-melotti ezio-melotti left a comment

Choose a reason for hiding this comment

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

The change looks good to me.

It seems that the only downside is that the feature is in beta, and currently doesn't support the flyout menu. Do you know if there is any ETA for that?

@hugovk
Copy link
Member Author

hugovk commented Aug 9, 2022

It seems that the only downside is that the feature is in beta, and currently doesn't support the flyout menu. Do you know if there is any ETA for that?

I didn't find it on the tracker, let's ask @humitos from RTD.

@humitos
Copy link

humitos commented Aug 9, 2022

Hi! Thanks for the ping here.

@ezio-melotti

It seems that the only downside is that the feature is in beta, and currently doesn't support the flyout menu. Do you know if there is any ETA for that?

Currently, we don't have an ETA. However, we are working on all the context around this feature that is required to be decoupled from the build process itself to be customizable and "injectable" into already built HTML files --since there are some Sphinx extensions (where we run Read the Docs' specific code) that won't be installed anymore when people overrides all the commands.

We published it as "beta" because of the current features limitations that you already found and also because we weren't sure how people will use this feature exactly. The current state of this feature has been helping us to collect feedback to continue building the feature in the direction that it's useful for most of our users.

Originally, the build.commands feature was mainly thought as an alternative for projects that weren't able to build their docs in Read the Docs due they require a completely different set of arguments / workflow than the one offered by Read the Docs. As your project uses the common Sphinx workflow, I, personally, would keep using Read the Docs without build.commands --in particular, after giving a try to the fix for pillow package (keep reading) and see if it reduces the build time.

Note the emphasis in "I, personally" 😄

@hugovk

This can be slow especially as some old dependencies are built from source because they don't support modern Python and don't have wheels available (e.g. readthedocs/readthedocs.org#9118).

The fix for this problem (readthedocs/readthedocs.org#9473) is going out in today's deploy. The build is unpinning pillow to install the best option for the Python version used. With this change, your build should install pillow from a wheel without compiling it.


I hope this clarifies a little the situation. I'm happy to help here in case you have any other doubts. I'll keep an eye on this PR to read following conversations. Again, thanks for the ping on it. It's really useful for us to know what people is doing and why they decided to use build.commands at this point 👍🏼

@hugovk
Copy link
Member Author

hugovk commented Aug 9, 2022

Thanks! Will retest after today's deploy.

The fix for this problem (readthedocs/readthedocs.org#9473) is going out in today's deploy. The build is unpinning pillow to install the best option for the Python version used. With this change, your build should install pillow from a wheel without compiling it.

Arguably this is a partial fix: it will download/install much quicker but we don't need Pillow (and many of the external dependencies like old Sphinx and sphinx-rtd-theme) so it will still be downloading several MB of unnecessary packages, and it would be nice to try and reduce the (100% discounted but) $1.8M/month bill for PyPI.

@humitos
Copy link

humitos commented Aug 9, 2022

Arguably this is a partial fix: it will download/install much quicker but we don't need Pillow (and many of the external dependencies like old Sphinx and sphinx-rtd-theme) so it will still be downloading several MB of unnecessary packages, and it would be nice to try and reduce the (100% discounted but) $1.8M/month bill for PyPI.

Yes. I agree. There are other set of work we have started to discuss some time ago that will eventually allow different types of customizable builders. This is a long term plan, tho. We do no have the resources to start working on this immediately, unfortunately.

@hugovk
Copy link
Member Author

hugovk commented Aug 10, 2022

After deploy

  1. Command time: 0s
  2. Command time: 0s
  3. Command time: 0s
  4. Command time: 0s
  5. Command time: 1s
  6. Command time: 11s
  7. Command time: 16s
  8. Command time: 1s
  9. Command time: 0s
  10. Command time: 8s

Total command time: 37s

Build took 72 seconds (includes command time and upload time)

https://readthedocs.org/projects/hugovk-devguide/builds/17667806/

Comparison

Before deploy After deploy This PR
Total command time 61s 37s 22s
Build took 94s 72s 49s

@hugovk
Copy link
Member Author

hugovk commented Aug 15, 2022

If we don't go for this PR (and we could do it even if we do): would we like @humitos to enable the feature flag USE_SPHINX_LATEST for the devguide project on RTD?

See readthedocs/readthedocs.org#9118 (comment) for details.

I think it's a good idea (whether we merge this PR or not).

@hugovk
Copy link
Member Author

hugovk commented Oct 5, 2022

I see two thumbs up from core devs to my previous question 👍

@humitos Hi again! Please could you enable USE_SPHINX_LATEST for https://readthedocs.org/projects/cpython-devguide/ ?

@humitos
Copy link

humitos commented Oct 6, 2022

@hugovk I just enabled USE_SPHINX_LATEST on that project. Please, provide any feedback you may have. It will be useful for us 😄

@hugovk
Copy link
Member Author

hugovk commented Oct 6, 2022

Will do, thank you! (I can see now we just get the single Sphinx download, as expected.)

@ezio-melotti ezio-melotti merged commit 2a09dac into python:main Oct 7, 2022
@hugovk hugovk deleted the rtd-commands branch October 7, 2022 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants