[PATCH v3] meson: Pass -j option to sphinx

[PATCH v3] 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.

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.

before:
  $ time make man html
  ...
  [1/2] Generating docs/QEMU manual with a custom command
  [2/2] Generating docs/QEMU man pages with a custom command

  real    0m43.157s
  user    0m42.642s
  sys     0m0.576s

after:
  $ time make man html
  ...
  [1/2] Generating docs/QEMU manual with a custom command
  [2/2] Generating docs/QEMU man pages with a custom command

  real    0m25.014s
  user    0m51.288s
  sys     0m2.085s

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..138ec6ce6f 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('>=5.1.2')
+    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

Re: [PATCH v3] meson: Pass -j option to sphinx

[Daniel P. Berrangé]

On Fri, Apr 28, 2023 at 12:01:02PM -0300, Fabiano Rosas wrote:
> 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.
> 
> 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.
> 
> before:
>   $ time make man html
>   ...
>   [1/2] Generating docs/QEMU manual with a custom command
>   [2/2] Generating docs/QEMU man pages with a custom command
> 
>   real    0m43.157s
>   user    0m42.642s
>   sys     0m0.576s
> 
> after:
>   $ time make man html
>   ...
>   [1/2] Generating docs/QEMU manual with a custom command
>   [2/2] Generating docs/QEMU man pages with a custom command
> 
>   real    0m25.014s
>   user    0m51.288s
>   sys     0m2.085s
> 
> 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(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3] meson: Pass -j option to sphinx

[Markus Armbruster]

Fabiano Rosas <farosas@suse.de> writes:

> 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.
>
> 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.
>
> before:
>   $ time make man html
>   ...
>   [1/2] Generating docs/QEMU manual with a custom command
>   [2/2] Generating docs/QEMU man pages with a custom command
>
>   real    0m43.157s
>   user    0m42.642s
>   sys     0m0.576s
>
> after:
>   $ time make man html
>   ...
>   [1/2] Generating docs/QEMU manual with a custom command
>   [2/2] Generating docs/QEMU man pages with a custom command
>
>   real    0m25.014s
>   user    0m51.288s
>   sys     0m2.085s

Thanks for tackling this!  sphinx-build is so slow I disable doc
building unless I'm working on docs.

> 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..138ec6ce6f 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('>=5.1.2')

Where do you get 5.1.2 from?  I have 5.0.2, and -j auto appears to work
fine.  The manual page says "Changed in version 1.7: Support auto
argument."

> +    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.

[...]

Re: [PATCH v3] meson: Pass -j option to sphinx

[Fabiano Rosas]

Markus Armbruster <armbru@redhat.com> writes:

> Fabiano Rosas <farosas@suse.de> writes:
>
>> 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.
>>
>> 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.
>>
>> before:
>>   $ time make man html
>>   ...
>>   [1/2] Generating docs/QEMU manual with a custom command
>>   [2/2] Generating docs/QEMU man pages with a custom command
>>
>>   real    0m43.157s
>>   user    0m42.642s
>>   sys     0m0.576s
>>
>> after:
>>   $ time make man html
>>   ...
>>   [1/2] Generating docs/QEMU manual with a custom command
>>   [2/2] Generating docs/QEMU man pages with a custom command
>>
>>   real    0m25.014s
>>   user    0m51.288s
>>   sys     0m2.085s
>
> Thanks for tackling this!  sphinx-build is so slow I disable doc
> building unless I'm working on docs.
>
>> 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..138ec6ce6f 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('>=5.1.2')
>
> Where do you get 5.1.2 from?  I have 5.0.2, and -j auto appears to work
> fine.  The manual page says "Changed in version 1.7: Support auto
> argument."
>

Ouch, I was looking at the readthedocs repository which has a similar
change.

So I think we could probably just hardcode the option. Most distros will
have a more recent sphinx version.
https://repology.org/project/python:sphinx/versions

Let me try to figure out what gitlab is using. I know it is less than 4
because our docs don't show some of the dbus parts:

https://www.qemu.org/docs/master/interop/dbus-display.html

>> +    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.
>
> [...]

Re: [PATCH v3] meson: Pass -j option to sphinx

[Paolo Bonzini]

On 4/28/23 17:01, Fabiano Rosas wrote:
> 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.
> 
> before:
>    $ time make man html
>    ...
>    [1/2] Generating docs/QEMU manual with a custom command
>    [2/2] Generating docs/QEMU man pages with a custom command
> 
>    real    0m43.157s
>    user    0m42.642s
>    sys     0m0.576s
> 
> after:
>    $ time make man html
>    ...
>    [1/2] Generating docs/QEMU manual with a custom command
>    [2/2] Generating docs/QEMU man pages with a custom command
> 
>    real    0m25.014s
>    user    0m51.288s
>    sys     0m2.085s

The 'nproc' fallback will potentially cause twice #CPUs processes to be 
active, since sphinx will run in parallel with everything else.

Is this result with "-j auto", and if so with which computer?  If the 
speedup is only 2x as it seems to be from the "time" above, I'd rather 
have "-j 2" only so that sphinx doesn't risk killing the machine...

Paolo

Re: [PATCH v3] meson: Pass -j option to sphinx

[Thomas Huth]

On 28/04/2023 19.45, Fabiano Rosas wrote:
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Fabiano Rosas <farosas@suse.de> writes:
>>
>>> 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.
>>>
>>> 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.
...
>>> diff --git a/docs/meson.build b/docs/meson.build
>>> index f220800e3e..138ec6ce6f 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('>=5.1.2')
>>
>> Where do you get 5.1.2 from?  I have 5.0.2, and -j auto appears to work
>> fine.  The manual page says "Changed in version 1.7: Support auto
>> argument."
>>
> 
> Ouch, I was looking at the readthedocs repository which has a similar
> change.
> 
> So I think we could probably just hardcode the option. Most distros will
> have a more recent sphinx version.
> https://repology.org/project/python:sphinx/versions
> 
> Let me try to figure out what gitlab is using. I know it is less than 4
> because our docs don't show some of the dbus parts

That's the "pages" job in .gitlab-ci.d/buildtest.yml, i.e. the debian-amd64 
container, i.e. Debian 11.

If I get that right (https://packages.debian.org/source/sphinx), this means 
we're using Sphinx v3.4.3 here.

  Thomas

Re: [PATCH v3] meson: Pass -j option to sphinx

[Daniel P. Berrangé]

On Sat, Apr 29, 2023 at 02:33:17PM +0200, Paolo Bonzini wrote:
> On 4/28/23 17:01, Fabiano Rosas wrote:
> > 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.
> > 
> > before:
> >    $ time make man html
> >    ...
> >    [1/2] Generating docs/QEMU manual with a custom command
> >    [2/2] Generating docs/QEMU man pages with a custom command
> > 
> >    real    0m43.157s
> >    user    0m42.642s
> >    sys     0m0.576s
> > 
> > after:
> >    $ time make man html
> >    ...
> >    [1/2] Generating docs/QEMU manual with a custom command
> >    [2/2] Generating docs/QEMU man pages with a custom command
> > 
> >    real    0m25.014s
> >    user    0m51.288s
> >    sys     0m2.085s
> 
> The 'nproc' fallback will potentially cause twice #CPUs processes to be
> active, since sphinx will run in parallel with everything else.
> 
> Is this result with "-j auto", and if so with which computer?  If the
> speedup is only 2x as it seems to be from the "time" above, I'd rather have
> "-j 2" only so that sphinx doesn't risk killing the machine...

Why would it kill the machine ? If there are two sphinx processes
concurrent, thus overcomitting available CPUs, the scheduler will
just end up giving them shorter timeslice OS. Given that the
sphinx parallelism seems to be very bursty when I monitored it,
I think having the high CPU counts is justified. The times when
both sphinx processes need all 8/16/whatever CPUs is relatively
unlikely to clash for prolonged periods.

What could kill the machine is if the RAM usage was excessive,
but I would thing we would see that already if it were a problem.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3] meson: Pass -j option to sphinx

[Fabiano Rosas]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Sat, Apr 29, 2023 at 02:33:17PM +0200, Paolo Bonzini wrote:
>> On 4/28/23 17:01, Fabiano Rosas wrote:
>> > 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.
>> > 
>> > before:
>> >    $ time make man html
>> >    ...
>> >    [1/2] Generating docs/QEMU manual with a custom command
>> >    [2/2] Generating docs/QEMU man pages with a custom command
>> > 
>> >    real    0m43.157s
>> >    user    0m42.642s
>> >    sys     0m0.576s
>> > 
>> > after:
>> >    $ time make man html
>> >    ...
>> >    [1/2] Generating docs/QEMU manual with a custom command
>> >    [2/2] Generating docs/QEMU man pages with a custom command
>> > 
>> >    real    0m25.014s
>> >    user    0m51.288s
>> >    sys     0m2.085s
>> 
>> The 'nproc' fallback will potentially cause twice #CPUs processes to be
>> active, since sphinx will run in parallel with everything else.
>> 
>> Is this result with "-j auto", and if so with which computer?  If the
>> speedup is only 2x as it seems to be from the "time" above, I'd rather have
>> "-j 2" only so that sphinx doesn't risk killing the machine...

Tested with -j auto and -j16 on my 16 cpu i7-11850H.

>
> Why would it kill the machine ? If there are two sphinx processes
> concurrent, thus overcomitting available CPUs, the scheduler will
> just end up giving them shorter timeslice OS.

One other thing I noticed is that we're not actually running the two
documentation targets in parallel. The man pages have a dependency on
the html pages. I'm not sure if that is legitimate.

  sphinxmans += custom_target('QEMU man pages',
                              build_by_default: build_docs,
                              output: these_man_pages,
HERE -->                      input: this_manual,
                              install: build_docs,
                              install_dir: install_dirs,
                              command: [SPHINX_ARGS, '-b', 'man', '-d', private_dir,
                                        input_dir, meson.current_build_dir()])

Removing that line gains us about 10 more seconds. However there are
some annoying differences in the man pages produced such as quotation
marks using a different character. I'm still investigating.