Message ID | 20230428150102.13114-1-farosas@suse.de |
---|---|
State | New |
Headers | show |
Series | [v3] meson: Pass -j option to sphinx | expand |
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
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. [...]
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. > > [...]
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
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
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
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.
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 + )