[PATCH v4 0/2] docs: Speedup docs build

[PATCH v4 0/2] docs: Speedup docs build

[Fabiano Rosas]

We currently have two documentation targets to build:
- 'man' for the man pages;
- 'html' for the web page.

There are two bottlenecks in the process:

1) sphinx runs with a single process;
2) the two targets are serialized.

For (1), we can just add the "-j auto" to sphinx_build and that should
bring some speed gains.

For (2) it's a little trickier because the reason the builds are
serialized is that Sphinx keeps track of changed files, but we still
need a way to tell meson whether a target needs to re-execute, so we
added dependency tracking and timestamp checking for (only) the 'html'
build via the depfile.py extension. Since the sources for both builds
are the same, we made the 'man' build dependent on 'html' so that it
would rebuild when needed.

So patch 1 adds the -j option to Sphinx and patch 2 adds depfile
support to the 'man' build. We can now run the two in parallel (ninja
will take care of that).

On my 16 core machine,
the -j change saves about 20s
the depfile change saves about 10s

===
On master:
 $ ../configure --enable-docs ...
 $ time make -j$(nproc) html man
   GIT     ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 tests/fp/berkeley-softfloat-3 dtc
 /usr/bin/ninja  build.ninja && touch build.ninja.stamp
 ninja: no work to do.
 /usr/bin/python3 -B .../qemu/meson/meson.py introspect --targets --tests --benchmarks | /usr/bin/python3 -B scripts/mtest2make.py > Makefile.mtest
   GIT     ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 tests/fp/berkeley-softfloat-3 dtc
 [1/2] Generating docs/QEMU manual with a custom command
 [2/2] Generating docs/QEMU man pages with a custom command
 make: Nothing to be done for 'man'.

 real    0m50.155s
 user    0m49.759s
 sys     0m0.761s

 $ mv docs ../saved-docs

This series:
 $ ../configure --enable-docs ...
 $ time make -j$(nproc) html man
   GIT     ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 tests/fp/berkeley-softfloat-3 dtc
 /usr/bin/ninja  build.ninja && touch build.ninja.stamp
 ninja: no work to do.
 /usr/bin/python3 -B .../qemu/meson/meson.py introspect --targets --tests --benchmarks | /usr/bin/python3 -B scripts/mtest2make.py > Makefile.mtest
   GIT     ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 tests/fp/berkeley-softfloat-3 dtc
 [1/2] Generating docs/QEMU man pages with a custom command
 [2/2] Generating docs/QEMU manual with a custom command
 make: Nothing to be done for 'man'.

 real    0m21.708s
 user    1m12.317s
 sys     0m2.056s

Diff sanity check:
 $ diff -rq docs/ ../saved-docs/
 Only in ../saved-docs/: docs.d      # now manual.dep
 Only in ../saved-docs/: docs.stamp  # now manual.stamp
 Only in docs/: man.dep
 Only in docs/: man.p
 Only in docs/: man.stamp
 Only in docs/: manual.dep
 Only in docs/: manual.stamp
 Files docs/manual.p/about/build-platforms.doctree and ../saved-docs/manual.p/about/build-platforms.doctree differ
 ...  # sphinx cache files, a bunch of these^

Rebuilding (here I show that a man file and an html file are
unchanged, change their source .rst and rebuild each target):

 $ ninja -d explain html
 ninja: no work to do.
 $ ninja -d explain man
 ninja: no work to do.
 $ man -l docs/qemu.1 | grep foobar
 $ grep foobar docs/manual/system/i386/pc.html
 $ vi ../docs/system/target-i386-desc.rst.inc    #add the 'foobar' string

 $ ninja -d explain man
 ninja explain: restat of output docs/man.stamp older than most recent input docs/system/target-i386-desc.rst.inc (1683122999140339620 vs 1683123032492362281)
 ninja explain: docs/man.stamp is dirty
 ninja explain: docs/qemu-block-drivers.7 is dirty
 ninja explain: docs/qemu-cpu-models.7 is dirty
 ninja explain: docs/qemu-ga-ref.7 is dirty
 ninja explain: docs/qemu-ga.8 is dirty
 ninja explain: docs/qemu-img.1 is dirty
 ninja explain: docs/qemu-nbd.8 is dirty
 ninja explain: docs/qemu-pr-helper.8 is dirty
 ninja explain: docs/qemu-qmp-ref.7 is dirty
 ninja explain: docs/qemu-storage-daemon-qmp-ref.7 is dirty
 ninja explain: docs/qemu-storage-daemon.1 is dirty
 ninja explain: docs/qemu-trace-stap.1 is dirty
 ninja explain: docs/qemu.1 is dirty
 ninja explain: docs/virtfs-proxy-helper.1 is dirty
 [1/1] Generating docs/QEMU man pages with a custom command

 $ man -l docs/qemu.1 | grep foobar
        The QEMU PC System emulator simulates the following foobar peripherals:
 $ grep foobar docs/manual/system/i386/pc.html  #html files unchanged

 $ ninja -d explain html
 ninja explain: restat of output docs/manual.stamp older than most recent input docs/system/target-i386-desc.rst.inc (1683122995876337403 vs 1683123032492362281)
 ninja explain: docs/manual.stamp is dirty
 [1/1] Generating docs/QEMU manual with a custom command

 $ man -l docs/qemu.1 | grep foobar
        The QEMU PC System emulator simulates the following foobar peripherals:
 $ grep foobar docs/manual/system/i386/pc.html
 <p>The QEMU PC System emulator simulates the following foobar peripherals:</p>

===

Fabiano Rosas (2):
  meson: Pass -j option to sphinx
  meson: Deserialize the man pages and html builds

 docs/meson.build           | 48 +++++++++++++++++++++++++-------------
 docs/sphinx/dbusdomain.py  |  4 ++++
 docs/sphinx/fakedbusdoc.py |  5 ++++
 docs/sphinx/qmp_lexer.py   |  5 ++++
 4 files changed, 46 insertions(+), 16 deletions(-)

-- 
2.35.3

[PATCH v4 1/2] meson: Pass -j option to sphinx

[Fabiano Rosas]

Save a bit of build time by passing the number of jobs option to
sphinx.

We cannot use  the -j option from make because  meson does not support
setting build time parameters for custom targets. Use nproc instead or
the equivalent sphinx option "-j  auto", if that is available (version
>=1.7.0).

Also make sure our plugins support parallelism and report it properly
to sphinx. Particularly, implement the merge_domaindata method in
DBusDomain that is used to merge in data from other subprocesses.

Tested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 docs/meson.build           | 12 ++++++++++++
 docs/sphinx/dbusdomain.py  |  4 ++++
 docs/sphinx/fakedbusdoc.py |  5 +++++
 docs/sphinx/qmp_lexer.py   |  5 +++++
 4 files changed, 26 insertions(+)

diff --git a/docs/meson.build b/docs/meson.build
index f220800e3e..6d0986579e 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -10,6 +10,18 @@ if sphinx_build.found()
     SPHINX_ARGS += [ '-W', '-Dkerneldoc_werror=1' ]
   endif
 
+  sphinx_version = run_command(SPHINX_ARGS + ['--version'],
+                               check: true).stdout().split()[1]
+  if sphinx_version.version_compare('>=1.7.0')
+    SPHINX_ARGS += ['-j', 'auto']
+  else
+    nproc = find_program('nproc')
+    if nproc.found()
+      jobs = run_command(nproc, check: true).stdout()
+      SPHINX_ARGS += ['-j', jobs]
+    endif
+  endif
+
   # This is a bit awkward but works: create a trivial document and
   # try to run it with our configuration file (which enforces a
   # version requirement). This will fail if sphinx-build is too old.
diff --git a/docs/sphinx/dbusdomain.py b/docs/sphinx/dbusdomain.py
index 2ea95af623..9872fd5bf6 100644
--- a/docs/sphinx/dbusdomain.py
+++ b/docs/sphinx/dbusdomain.py
@@ -400,6 +400,10 @@ def get_objects(self) -> Iterator[Tuple[str, str, str, str, str, int]]:
         for refname, obj in self.objects.items():
             yield (refname, refname, obj.objtype, obj.docname, obj.node_id, 1)
 
+    def merge_domaindata(self, docnames, otherdata):
+        for name, obj in otherdata['objects'].items():
+            if obj.docname in docnames:
+                self.data['objects'][name] = obj
 
 def setup(app):
     app.add_domain(DBusDomain)
diff --git a/docs/sphinx/fakedbusdoc.py b/docs/sphinx/fakedbusdoc.py
index d2c5079046..2d2e6ef640 100644
--- a/docs/sphinx/fakedbusdoc.py
+++ b/docs/sphinx/fakedbusdoc.py
@@ -23,3 +23,8 @@ def run(self):
 def setup(app: Sphinx) -> Dict[str, Any]:
     """Register a fake dbus-doc directive with Sphinx"""
     app.add_directive("dbus-doc", FakeDBusDocDirective)
+
+    return dict(
+        parallel_read_safe = True,
+        parallel_write_safe = True
+    )
diff --git a/docs/sphinx/qmp_lexer.py b/docs/sphinx/qmp_lexer.py
index f7e4c0e198..a59de8a079 100644
--- a/docs/sphinx/qmp_lexer.py
+++ b/docs/sphinx/qmp_lexer.py
@@ -41,3 +41,8 @@ def setup(sphinx):
         sphinx.add_lexer('QMP', QMPExampleLexer)
     except errors.VersionRequirementError:
         sphinx.add_lexer('QMP', QMPExampleLexer())
+
+    return dict(
+        parallel_read_safe = True,
+        parallel_write_safe = True
+    )
-- 
2.35.3

[PATCH v4 2/2] meson: Deserialize the man pages and html builds

[Fabiano Rosas]

For the documentation builds (man pages & manual), we let Sphinx
decide when to rebuild and use a depfile to know when to trigger the
make target.

We currently use a trick of having the man pages custom_target take as
input the html pages custom_target object, which causes both targets
to be executed if one of the dependencies has changed. However, having
this at the custom_target level means that the two builds are
effectively serialized.

We can eliminate the dependency between the targets by adding a second
depfile for the man pages build, allowing them to be parallelized by
ninja while keeping sphinx in charge of deciding when to rebuild.

Since they can now run in parallel, separate the Sphinx cache
directory of the two builds. We need this not only for data
consistency but also because Sphinx writes builder-dependent
environment information to the cache directory (see notes under
smartquotes_excludes in sphinx docs [1]).

Note that after this patch the commands `make man` and `make html`
only build the specified target. To keep the old behavior of building
both targets, use `make man html` or `make sphinxdocs`.

1- https://www.sphinx-doc.org/en/master/usage/configuration.html

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 docs/meson.build | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/docs/meson.build b/docs/meson.build
index 6d0986579e..858e737431 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -42,7 +42,9 @@ if sphinx_build.found()
 endif
 
 if build_docs
-  SPHINX_ARGS += ['-Dversion=' + meson.project_version(), '-Drelease=' + get_option('pkgversion')]
+  SPHINX_ARGS += ['-Dversion=' + meson.project_version(),
+                  '-Drelease=' + get_option('pkgversion'),
+                  '-Ddepfile=@DEPFILE@', '-Ddepfile_stamp=@OUTPUT0@']
 
   man_pages = {
         'qemu-ga.8': (have_ga ? 'man8' : ''),
@@ -61,41 +63,43 @@ if build_docs
   }
 
   sphinxdocs = []
-  sphinxmans = []
 
   private_dir = meson.current_build_dir() / 'manual.p'
   output_dir = meson.current_build_dir() / 'manual'
   input_dir = meson.current_source_dir()
 
-  this_manual = custom_target('QEMU manual',
+  manual = custom_target('QEMU manual',
                 build_by_default: build_docs,
-                output: 'docs.stamp',
+                output: 'manual.stamp',
                 input: files('conf.py'),
-                depfile: 'docs.d',
-                command: [SPHINX_ARGS, '-Ddepfile=@DEPFILE@',
-                          '-Ddepfile_stamp=@OUTPUT0@',
-                          '-b', 'html', '-d', private_dir,
+                depfile: 'manual.dep',
+                command: [SPHINX_ARGS, '-b', 'html', '-d', private_dir,
                           input_dir, output_dir])
-  sphinxdocs += this_manual
+  sphinxdocs += manual
   install_subdir(output_dir, install_dir: qemu_docdir, strip_directory: true)
 
-  these_man_pages = []
-  install_dirs = []
+  man_private_dir = meson.current_build_dir() / 'man.p'
+  # man.stamp is not installed
+  these_man_pages = ['man.stamp']
+  install_dirs = [false]
+
   foreach page, section : man_pages
     these_man_pages += page
     install_dirs += section == '' ? false : get_option('mandir') / section
   endforeach
 
-  sphinxmans += custom_target('QEMU man pages',
+
+  man_pages = custom_target('QEMU man pages',
                               build_by_default: build_docs,
                               output: these_man_pages,
-                              input: this_manual,
+                              depfile: 'man.dep',
                               install: build_docs,
                               install_dir: install_dirs,
-                              command: [SPHINX_ARGS, '-b', 'man', '-d', private_dir,
+                              command: [SPHINX_ARGS, '-b', 'man', '-d', man_private_dir,
                                         input_dir, meson.current_build_dir()])
+  sphinxdocs += man_pages
 
   alias_target('sphinxdocs', sphinxdocs)
-  alias_target('html', sphinxdocs)
-  alias_target('man', sphinxmans)
+  alias_target('html', manual)
+  alias_target('man', man_pages)
 endif
-- 
2.35.3

Re: [PATCH v4 2/2] meson: Deserialize the man pages and html builds

[Peter Maydell]

On Wed, 3 May 2023 at 21:39, Fabiano Rosas <farosas@suse.de> wrote:
>
> For the documentation builds (man pages & manual), we let Sphinx
> decide when to rebuild and use a depfile to know when to trigger the
> make target.
>
> We currently use a trick of having the man pages custom_target take as
> input the html pages custom_target object, which causes both targets
> to be executed if one of the dependencies has changed. However, having
> this at the custom_target level means that the two builds are
> effectively serialized.
>
> We can eliminate the dependency between the targets by adding a second
> depfile for the man pages build, allowing them to be parallelized by
> ninja while keeping sphinx in charge of deciding when to rebuild.
>
> Since they can now run in parallel, separate the Sphinx cache
> directory of the two builds. We need this not only for data
> consistency but also because Sphinx writes builder-dependent
> environment information to the cache directory (see notes under
> smartquotes_excludes in sphinx docs [1]).

The sphinx-build manpage disagrees about that last part.
https://www.sphinx-doc.org/en/master/man/sphinx-build.html
says about -d:
"with this option you can select a different cache directory
 (the doctrees can be shared between all builders)"

If we don't share the cache directory, presumably Sphinx
now ends up parsing all the input files twice, once per
builder, rather than being able to share them?

thanks
-- PMM

Re: [PATCH v4 2/2] meson: Deserialize the man pages and html builds

[Fabiano Rosas]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Wed, 3 May 2023 at 21:39, Fabiano Rosas <farosas@suse.de> wrote:
>>
>> For the documentation builds (man pages & manual), we let Sphinx
>> decide when to rebuild and use a depfile to know when to trigger the
>> make target.
>>
>> We currently use a trick of having the man pages custom_target take as
>> input the html pages custom_target object, which causes both targets
>> to be executed if one of the dependencies has changed. However, having
>> this at the custom_target level means that the two builds are
>> effectively serialized.
>>
>> We can eliminate the dependency between the targets by adding a second
>> depfile for the man pages build, allowing them to be parallelized by
>> ninja while keeping sphinx in charge of deciding when to rebuild.
>>
>> Since they can now run in parallel, separate the Sphinx cache
>> directory of the two builds. We need this not only for data
>> consistency but also because Sphinx writes builder-dependent
>> environment information to the cache directory (see notes under
>> smartquotes_excludes in sphinx docs [1]).
>
> The sphinx-build manpage disagrees about that last part.
> https://www.sphinx-doc.org/en/master/man/sphinx-build.html
> says about -d:
> "with this option you can select a different cache directory
>  (the doctrees can be shared between all builders)"
>

The issue I had is that sphinx by default uses smart quotes for html
builders, but not for man builders. But whichever builder runs first
gets to set the smartquotes option and that sticks for the next
builder. That causes our man pages to come up with fancy curly quotes
instead of ' which is probably not an issue, but I didn't want to
produce different output from what we already have today.

I ended up conflating the cache directory (-d) with the environment
(-E), so it is possible that we can reuse the cache but not the
environment (where I assume the smartquotes option is stored). Well, I
better go read the sphinx code and figure that out.

> If we don't share the cache directory, presumably Sphinx
> now ends up parsing all the input files twice, once per
> builder, rather than being able to share them?
>

Yes, but having it run in parallel from the ninja level is still
faster. Of course, if we could reuse the cache, this could potentially
be even faster. I'll try to determine if it is really safe to do so.

Re: [PATCH v4 2/2] meson: Deserialize the man pages and html builds

[Peter Maydell]

On Thu, 4 May 2023 at 13:06, Fabiano Rosas <farosas@suse.de> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Wed, 3 May 2023 at 21:39, Fabiano Rosas <farosas@suse.de> wrote:
> >> Since they can now run in parallel, separate the Sphinx cache
> >> directory of the two builds. We need this not only for data
> >> consistency but also because Sphinx writes builder-dependent
> >> environment information to the cache directory (see notes under
> >> smartquotes_excludes in sphinx docs [1]).
> >
> > The sphinx-build manpage disagrees about that last part.
> > https://www.sphinx-doc.org/en/master/man/sphinx-build.html
> > says about -d:
> > "with this option you can select a different cache directory
> >  (the doctrees can be shared between all builders)"
> >
>
> The issue I had is that sphinx by default uses smart quotes for html
> builders, but not for man builders. But whichever builder runs first
> gets to set the smartquotes option and that sticks for the next
> builder. That causes our man pages to come up with fancy curly quotes
> instead of ' which is probably not an issue, but I didn't want to
> produce different output from what we already have today.
>
> I ended up conflating the cache directory (-d) with the environment
> (-E), so it is possible that we can reuse the cache but not the
> environment (where I assume the smartquotes option is stored). Well, I
> better go read the sphinx code and figure that out.
>
> > If we don't share the cache directory, presumably Sphinx
> > now ends up parsing all the input files twice, once per
> > builder, rather than being able to share them?
> >
>
> Yes, but having it run in parallel from the ninja level is still
> faster. Of course, if we could reuse the cache, this could potentially
> be even faster. I'll try to determine if it is really safe to do so.

Yeah, I wouldn't be surprised if we need the caches separate
for concurrency reasons, so this may just be a "commit message
might need tweaking" nit.

-- PMM

Re: [PATCH v4 0/2] docs: Speedup docs build

[Paolo Bonzini]

Queued, thanks.

Paolo

Re: [PATCH v4 2/2] meson: Deserialize the man pages and html builds

[Paolo Bonzini]

On 5/3/23 22:39, Fabiano Rosas wrote:
> For the documentation builds (man pages & manual), we let Sphinx
> decide when to rebuild and use a depfile to know when to trigger the
> make target.
> 
> We currently use a trick of having the man pages custom_target take as
> input the html pages custom_target object, which causes both targets
> to be executed if one of the dependencies has changed. However, having
> this at the custom_target level means that the two builds are
> effectively serialized.
> 
> We can eliminate the dependency between the targets by adding a second
> depfile for the man pages build, allowing them to be parallelized by
> ninja while keeping sphinx in charge of deciding when to rebuild.
> 
> Since they can now run in parallel, separate the Sphinx cache
> directory of the two builds. We need this not only for data
> consistency but also because Sphinx writes builder-dependent
> environment information to the cache directory (see notes under
> smartquotes_excludes in sphinx docs [1]).
> 
> Note that after this patch the commands `make man` and `make html`
> only build the specified target. To keep the old behavior of building
> both targets, use `make man html` or `make sphinxdocs`.
> 
> 1- https://www.sphinx-doc.org/en/master/usage/configuration.html

Unfortunately this breaks CentOS 8, which has an older version of ninja:

ninja: error: build.ninja:16369: multiple outputs aren't (yet?) 
supported by depslog; bring this up on the mailing list if it affects you

This was fixed in ninja 1.10.0.

Paolo

Re: [PATCH v4 2/2] meson: Deserialize the man pages and html builds

[Fabiano Rosas]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 5/3/23 22:39, Fabiano Rosas wrote:
>> For the documentation builds (man pages & manual), we let Sphinx
>> decide when to rebuild and use a depfile to know when to trigger the
>> make target.
>> 
>> We currently use a trick of having the man pages custom_target take as
>> input the html pages custom_target object, which causes both targets
>> to be executed if one of the dependencies has changed. However, having
>> this at the custom_target level means that the two builds are
>> effectively serialized.
>> 
>> We can eliminate the dependency between the targets by adding a second
>> depfile for the man pages build, allowing them to be parallelized by
>> ninja while keeping sphinx in charge of deciding when to rebuild.
>> 
>> Since they can now run in parallel, separate the Sphinx cache
>> directory of the two builds. We need this not only for data
>> consistency but also because Sphinx writes builder-dependent
>> environment information to the cache directory (see notes under
>> smartquotes_excludes in sphinx docs [1]).
>> 
>> Note that after this patch the commands `make man` and `make html`
>> only build the specified target. To keep the old behavior of building
>> both targets, use `make man html` or `make sphinxdocs`.
>> 
>> 1- https://www.sphinx-doc.org/en/master/usage/configuration.html

Sorry it took me a while to get back to this, I've been caught in
downstream work.

>
> Unfortunately this breaks CentOS 8, which has an older version of ninja:
>
> ninja: error: build.ninja:16369: multiple outputs aren't (yet?) 
> supported by depslog; bring this up on the mailing list if it affects you
>
> This was fixed in ninja 1.10.0.
>

It looks like it would be easier to just wait until all our supported
build platforms reach this version.

Is this CentOS 8 or CentOS Stream 8? I believe CentOS Stream 8 would
drop from our support matrix at the end of this year. And CentOS 8
should have already dropped no? Due to Stream 9 being released in
2021. Unless we do not count Stream as a new version over plain CentOS.

For the dates and versions, I'm looking at:
https://en.wikipedia.org/wiki/CentOS
https://repology.org/project/ninja/versions