sphinx-doc / sphinx-doc/sphinx

Oprimize writing step of parallel builds

Open
#10,779 5 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

internals:parallel type:enhancement type:performance
Dominant language
Python
Stars
8k
Forks
2.6k
PR merge metrics
No merged PRs in 30d

Description

Is your feature request related to a problem? Please describe.
During the writing step of the build process, even while using -j auto, there are some time-consuming operations that are performed serially. It might be possible to parallelize them and further improve performances.

Describe the solution you'd like
Last night I went down a rabbit hole and tried to optimize _write_parallel:
https://github.com/sphinx-doc/sphinx/blob/8c4865c30d5fa847d727fea16519d7afce627932/sphinx/builders/__init__.py#L571-L607

I tried to naively apply the following changes:

diff --git a/sphinx/builders/__init__.py b/sphinx/builders/__init__.py
index 2aede5c24..d27df5fdd 100644
--- a/sphinx/builders/__init__.py
+++ b/sphinx/builders/__init__.py
@@ -569,9 +569,11 @@ class Builder:
                 self.write_doc(docname, doctree)
 
     def _write_parallel(self, docnames: Sequence[str], nproc: int) -> None:
-        def write_process(docs: List[Tuple[str, nodes.document]]) -> None:
-            self.app.phase = BuildPhase.WRITING
-            for docname, doctree in docs:
+        def write_process(docs: List[str]) -> None:
+            for docname in docs:
+                doctree = self.env.get_and_resolve_doctree(docname, self)
+                self.app.phase = BuildPhase.WRITING
+                self.write_doc_serialized(docname, doctree)
                 self.write_doc(docname, doctree)
 
         # warm up caches/compile templates using the first document
@@ -595,12 +597,7 @@ class Builder:
 
         self.app.phase = BuildPhase.RESOLVING
         for chunk in chunks:
-            arg = []
-            for docname in chunk:
-                doctree = self.env.get_and_resolve_doctree(docname, self)
-                self.write_doc_serialized(docname, doctree)
-                arg.append((docname, doctree))
-            tasks.add_task(write_process, arg, on_chunk_done)
+            tasks.add_task(write_process, chunk, on_chunk_done)
 
         # make sure all threads have finished
         tasks.join()

This resulted in a build time reduction of ~25%.

Without patch, using Sphinx v4.5.0 to build the python/cpython docs:

real    2m13,386s
user    4m35,046s
sys     0m7,205s

With patch:

real    1m36,804s
user    4m42,314s
sys     0m5,909s

Despite the performance improvements, this solution is wrong for (at least) a few reasons:

  • while it mostly works, some things (e.g. images) break;
  • write_doc_serialized was explicitly created for executing code that can't be parallelized, so it shouldn't be called in a process executed in parallel;

However, write_doc_serialized was added ~10 years ago in 5cd0841e5f041f3ef03840fafac425654a48b40d to fix "fix parallel build globals problems". At the time the parallelization also relied on threading, whereas now it seems entirely based on multiprocessing. In builders/__init__.py it's defined as an empty method to be overridden, and the html builder overrides it with:
https://github.com/sphinx-doc/sphinx/blob/8c4865c30d5fa847d727fea16519d7afce627932/sphinx/builders/html/__init__.py#L673-L678

In addition, I noticed that _read_parallel seems to do some post-processing/merging in
https://github.com/sphinx-doc/sphinx/blob/8c4865c30d5fa847d727fea16519d7afce627932/sphinx/builders/__init__.py#L467-L475

At this point, my lack of knowledge of Sphinx internals prevented me to dig deeper, but I was left wondering:

  1. if the fix added in 5cd0841e5f041f3ef03840fafac425654a48b40d is still relevant after moving away from threading, or if it can be revisited/reverted (at least partially);
  2. if the issue fixed by the above commit can be addressed with some post-processing similar to the one used in _read_parallel;
  3. if any of the operations currently executed serially by write_doc_serialized can be parallelized;

Perhaps someone more familiar with the Sphinx internal can take a look and confirm whether there is room for improvement or not?

Addressing this (especially 2. above) might also help fix the following issue (cc @tk0miya, since you worked on this code):

  • #4459

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in sphinx/builders/init.py with _write_parallel, _read_parallel, and write_doc_serialized, then compare the HTML builder override in sphinx/builders/html/init.py. Review commit 5cd0841e5f041f3ef03840fafac425654a48b40d and the linked #4459 context before running the Python documentation build benchmark. Done means identifying a safe parallelization or post-processing approach without breaking features such as images.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
build-system, performance
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.