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

Reimplement parallel docbuild using make #31948

Closed
mkoeppe opened this issue Jun 10, 2021 · 105 comments
Closed

Reimplement parallel docbuild using make #31948

mkoeppe opened this issue Jun 10, 2021 · 105 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 10, 2021

Based on #31353 (sage --docbuild: Add options to list all documents), we can remove the issues with parallel (#31344) and serial (#31936) docbuild by restructuring the docbuild using make.

Blocker because it solves blocker issue #31936.

CC: @dimpase

Component: build

Author: John Palmieri, Matthias Koeppe

Branch: 53f260c

Reviewer: Matthias Koeppe, John Palmieri, Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/31948

@mkoeppe mkoeppe added this to the sage-9.4 milestone Jun 10, 2021
@jhpalmieri
Copy link
Member

comment:1

I'm not quite sure how to go about this. I tried this:

diff --git a/build/make/Makefile.in b/build/make/Makefile.in
index fb3a6ed5bc..4581dc3226 100644
--- a/build/make/Makefile.in
+++ b/build/make/Makefile.in
@@ -335,7 +335,12 @@ DOC_DEPENDENCIES = sagelib sage_docbuild $(inst_sphinx) \
        $(inst_ipykernel) $(inst_jupyter_client) $(inst_conway_polynomials) \
        $(inst_tachyon) $(inst_jmol) $(inst_thebe) $(inst_ipywidgets)
 
-doc: doc-html
+doc: doc-new
+
+doc-new: $(DOC_DEPENDENCIES)
+       for doc in $(shell cd $(SAGE_ROOT) && ./sage --docbuild --all-documents all); do \
+           $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $$doc html $(SAGE_DOCBUILD_OPTS)" logs/dochtml.log; \
+       done
 
 doc-html: $(DOC_DEPENDENCIES)
        $(AM_V_at)cd ../.. && sage-logger -p './sage --docbuild --no-pdf-links all html $(SAGE_DOCBUILD_OPTS)' logs/dochtml.log

and it seems to build the documentation, but it doesn't do it in parallel, I think because we've turned off parallel processing for recursive calls to make earlier in the file.

If it did work, this approach would need a little tinkering: we should build the reference manual first, then everything else.

A separate question: why do we use cd ../.. in the recipe for doc-html (and other doc targets), rather than cd $(SAGE_ROOT)?

@jhpalmieri
Copy link
Member

comment:2

And if we really want to pull all of the parallel building out of the current system, then we should build the pieces of the reference manual separately in the Makefile. I think this is what you intended.

@jhpalmieri
Copy link
Member

comment:3

Here is my current attempt, which doesn't quite work:

diff --git a/build/make/Makefile.in b/build/make/Makefile.in
index fb3a6ed5bc..c199de440d 100644
--- a/build/make/Makefile.in
+++ b/build/make/Makefile.in
@@ -335,7 +335,27 @@ DOC_DEPENDENCIES = sagelib sage_docbuild $(inst_sphinx) \
        $(inst_ipykernel) $(inst_jupyter_client) $(inst_conway_polynomials) \
        $(inst_tachyon) $(inst_jmol) $(inst_thebe) $(inst_ipywidgets)
 
-doc: doc-html
+doc: doc-references doc-other
+
+doc-references: $(DOC_DEPENDENCIES)
+       $(eval DOCS = $(shell cd $(SAGE_ROOT) && ./sage --docbuild --all-documents reference))
+       $(eval biblio = $(firstword $(DOCS)))
+       $(eval other_docs = $(wordlist 2, 100, $(DOCS)))
+       $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $(biblio) inventory $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
+       for doc in $(other_docs); do \
+           $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $$doc inventory $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
+       done
+       $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links reference inventory $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
+       $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $(biblio) html $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
+       for doc in $(other_docs); do \
+           $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $$doc html $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
+       $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links reference html $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
+       done
+
+doc-other: $(DOC_DEPENDENCIES) doc-references
+       for doc in $(wordlist 2, 100, $(shell cd $(SAGE_ROOT) && ./sage --docbuild --all-documents all)); do \
+           $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $$doc html $(SAGE_DOCBUILD_OPTS)" logs/docs-other-html.log; \
+       done
 
 doc-html: $(DOC_DEPENDENCIES)
        $(AM_V_at)cd ../.. && sage-logger -p './sage --docbuild --no-pdf-links all html $(SAGE_DOCBUILD_OPTS)' logs/dochtml.log
diff --git a/src/sage_docbuild/__init__.py b/src/sage_docbuild/__init__.py
index 79005b903a..a3caa8a92d 100644
--- a/src/sage_docbuild/__init__.py
+++ b/src/sage_docbuild/__init__.py
@@ -116,7 +116,7 @@ def builder_helper(type):
             # WEBSITESPHINXOPTS is either empty or " -A hide_pdf_links=1 "
             options += WEBSITESPHINXOPTS
 
-        if kwds.get('use_multidoc_inventory', True):
+        if kwds.get('use_multidoc_inventory', True) and type != 'inventory':
             options += ' -D multidoc_first_pass=0'
         else:
             options += ' -D multidoc_first_pass=1'

Several problems:

  • lack of parallel building
  • line 348 should be replaced by something that just builds the top-level reference manual stuff, assembling the parts into one set of indices, etc. Probably the same for line 352.
  • I'm getting some warning messages that I don't know what to make of: "[schemes ] Handler <function on_build_finished at 0x33872ef70> for event 'build-finished' threw an exception", for example.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 11, 2021

comment:4

To get make to parallelize it, the individual documents need to become make targets. A combination of $(eval...) and a macro call can do this -- similar to https://www.gnu.org/software/make/manual/html_node/Eval-Function.html

@jhpalmieri
Copy link
Member

comment:5

I don't see how to do anything like that unless we hardcode the list of documents: we can't run sage --docbuild ... without building some other targets, so it seems to me that we have to construct the list of documents inside a recipe. But I'm very far from a Makefile expert, so I could be missing something.

@jhpalmieri
Copy link
Member

comment:6

Or write a standalone script to construct the document list.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 11, 2021

comment:7

Could you push your current version to the ticket please?

@jhpalmieri
Copy link
Member

@jhpalmieri
Copy link
Member

Commit: c2c9b3c

@jhpalmieri
Copy link
Member

comment:9

Here it is.


New commits:

c2c9b3ctrac 31948: very rough draft

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 11, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 11, 2021

comment:11

Here's a solution using recursive make invocation


New commits:

393ab65build/make/Makefile.in (doc-reference): Invoke make recursively to build documents in parallel

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 11, 2021

Changed commit from c2c9b3c to 393ab65

@jhpalmieri
Copy link
Member

comment:12

This doesn't work for me, unfortunately. If I do make doc-clean && make doc, I get this error, which I don't understand.

[reference] building [inventory]: targets for 1 source files that are out of date
[reference] updating environment: [new config] 1 added, 0 changed, 0 removed
[reference] The inventory files are in local/share/doc/sage/inventory/en/reference/references.
Build finished. The built documents can be found in /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/share/doc/sage/inventory/en/reference/references
Deleting empty directory /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage/manifolds
Deleting empty directory /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage
Error building the documentation.
Traceback (most recent call last):
  File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__main__.py", line 2, in <module>
    main()
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 1762, in main
    builder()
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 769, in _wrapper
    self.write_auto_rest_file(module_name)
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 1070, in write_auto_rest_file
    with open(filename, 'w') as outfile:
FileNotFoundError: [Errno 2] No such file or directory: '/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage/manifolds/utilities.rst'

With the following change, the documentation builds:

diff --git a/build/make/Makefile.in b/build/make/Makefile.in
index 339497fd17..12b4e56b20 100644
--- a/build/make/Makefile.in
+++ b/build/make/Makefile.in
@@ -349,11 +349,7 @@ doc-references: $(DOC_DEPENDENCIES)
        $(eval DOCS = $(shell cd $(SAGE_ROOT) && ./sage --docbuild --all-documents reference))
        $(eval biblio = $(firstword $(DOCS)))
        $(eval other_docs = $(wordlist 2, 100, $(DOCS)))
-       +$(MAKE_REC) doc-inventory-$(subst /,-,$(biblio))
-       +$(MAKE_REC) $(foreach doc, $(other_docs), doc-inventory-$(subst /,-,$(doc)))
        +$(MAKE_REC) doc-inventory-reference
-       +$(MAKE_REC) doc-html-$(subst /,-,$(biblio))
-       +$(MAKE_REC) $(foreach doc, $(other_docs), doc-html-$(subst /,-,$(doc)))
        +$(MAKE_REC) doc-html-reference
 
 doc-other: $(DOC_DEPENDENCIES) doc-references

This is a hybrid model, using the old parallel building methods for the reference manual. The inventory build is fine, but I get some strange warnings during the html build:

[repl     ] building [html]: targets for 35 source files that are out of date
[polynomia] building [html]: targets for 62 source files that are out of date
[spkg     ] building [html]: targets for 316 source files that are out of date
[tensor_fr] building [html]: targets for 19 source files that are out of date
[algebras ] building [html]: targets for 97 source files that are out of date
[manifolds] building [html]: targets for 81 source files that are out of date
[repl     ] writing additional pages...  searchdone
[repl     ] dumping search index in English (code: en)... done
[repl     ] The HTML pages are in local/share/doc/sage/html/en/reference/repl.
[repl     ] Extension error:
[repl     ] Handler <function on_build_finished at 0x33c247af0> for event 'build-finished' threw an exception
Build finished. The built documents can be found in /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/share/doc/sage/html/en/reference/repl
[tensor_fr] writing additional pages...  searchdone
[tensor_fr] dumping search index in English (code: en)... done
[tensor_fr] The HTML pages are in local/share/doc/sage/html/en/reference/tensor_free_modules.
[tensor_fr] Extension error:
[tensor_fr] Handler <function on_build_finished at 0x33d1c3040> for event 'build-finished' threw an exception

I don't know what Handler <function on_build_finished at 0x33d1c3040> for event 'build-finished' threw an exception means or where it comes from.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 12, 2021

comment:13

Replying to @jhpalmieri:

This doesn't work for me, unfortunately. If I do make doc-clean && make doc, I get this error, which I don't understand.

[reference] building [inventory]: targets for 1 source files that are out of date
[reference] updating environment: [new config] 1 added, 0 changed, 0 removed
[reference] The inventory files are in local/share/doc/sage/inventory/en/reference/references.
Build finished. The built documents can be found in /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/share/doc/sage/inventory/en/reference/references
Deleting empty directory /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage/manifolds
Deleting empty directory /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage
Error building the documentation.
Traceback (most recent call last):
  File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__main__.py", line 2, in <module>
    main()
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 1762, in main
    builder()
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 769, in _wrapper
    self.write_auto_rest_file(module_name)
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 1070, in write_auto_rest_file
    with open(filename, 'w') as outfile:
FileNotFoundError: [Errno 2] No such file or directory: '/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage/manifolds/utilities.rst'

Looks like something forgot to create the directory in which this .rst file is to be created

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 12, 2021

comment:14

I was able to reproduce an error of this form as well.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 12, 2021

comment:15

If you do make doc-clean && make SAGE_DOCBUILD_OPTS="--verbose=3" doc, you can see that this is a race between creating directories and "deleting empty directories"

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 12, 2021

comment:16

I'll add an option to turn it off

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 12, 2021

Changed commit from 393ab65 to d08633f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 12, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

d08633fbuild/make/Makefile.in, src/sage_docbuild/__init__.py: New option --no-prune-empty-dirs, use it to remove races

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 12, 2021

Author: John Palmieri, Matthias Koeppe

@jhpalmieri
Copy link
Member

comment:19

I still see

[valuation] Extension error:
[valuation] Handler <function on_build_finished at 0x3375808b0> for event 'build-finished' threw an exception

for (I think) every piece of the reference manual. Any ideas about this?

Edit: the manual seems to build correctly, regardless.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 12, 2021

comment:20

No idea about this error. I don't see it here. I suppose you could try to run the docbuild for a document in pdb to debug this.

@jhpalmieri
Copy link
Member

comment:22

I haven't tried pdb, but I get it when doing make doc-clean && make — running make again at that point doesn't produce the error. I also get it on two different machines, one Big Sur, one Catalina.

One more task: the reference manual is built twice: the line

+$(MAKE_REC) SAGE_DOCBUILD_OPTS="$(SAGE_DOCBUILD_OPTS) --no-prune-empty-dirs" doc-inventory--reference

needs to be replaced with something that just builds the top-level ref manual indices, and similarly for the html build. I may be able to work on this later, but please go ahead if you want to take a stab at it.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 12, 2021

comment:23

If it shows up again, perhaps #31005 (Add traceback information to exceptions during docbuild) can help to diagnose it

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 12, 2021

comment:24

OK, I see Handler <function on_build_finished at 0x3b9411310> for event 'build-finished' threw an exception now too

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 16, 2021

Changed commit from 2c3a9ae to 54e68e0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 16, 2021

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

54e68e0trac 31948: move some "$(AM_V_at)"s around

@jhpalmieri
Copy link
Member

comment:73

Okay, I think I'm done tinkering now.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 16, 2021

comment:75

Looks good and seems to work well. Positive review from my side

@jhpalmieri
Copy link
Member

comment:76

Dima, any comments? I'm happy with Matthias' contributions.

@jhpalmieri
Copy link
Member

Changed reviewer from Matthias Koeppe, ... to Matthias Koeppe, John Palmieri, ...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 16, 2021

Changed commit from 54e68e0 to 53f260c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 16, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

53f260ctrac 31948: fix doctests

@dimpase
Copy link
Member

dimpase commented Jun 17, 2021

comment:79

looks good, thanks!

@dimpase
Copy link
Member

dimpase commented Jun 17, 2021

Changed reviewer from Matthias Koeppe, John Palmieri, ... to Matthias Koeppe, John Palmieri, Dima Pasechnik

@jhpalmieri
Copy link
Member

comment:80

Great, thank you!

@mkoeppe

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Jun 20, 2021

Changed branch from u/jhpalmieri/parallel-docbuild-with-make to 53f260c

@kwankyu
Copy link
Collaborator

kwankyu commented Jun 23, 2021

Changed commit from 53f260c to none

@kwankyu
Copy link
Collaborator

kwankyu commented Jun 23, 2021

comment:83

Previously sage --docbuild all pdf built a main website listing all documentations with icons linking to pdf files. Now it seems that make doc-pdf does not build such an website. Is there a way to build the website?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 23, 2021

comment:84

I've opened #32043 for this

@dimpase
Copy link
Member

dimpase commented Jul 21, 2021

comment:85

I am seeing a race condition, sphinx may race FileExistsError, cf. e.g.

[dochtml] [reference] preparing documents... skipping loading of indexes... done
[dochtml] [reference] Merging js index files...
[dochtml] [reference]     algebras: 4424 js index entries
[dochtml] [reference]     arithgroup: 1204 js index entries
...

[dochtml] [reference]     topology: 1937 js index entries
[dochtml] [reference]     valuations: 973 js index entries
[dochtml] [reference] ... done (64387 js index entries)
[dochtml] [reference] copying static files... failed
[dochtml] [reference] WARNING: cannot copy static file
FileExistsError(17, 'File exists')
[dochtml] [reference] dumping search index in English (code: en)... done
[dochtml] [reference] The HTML pages are in
local/share/doc/sage/html/en/reference.
[dochtml] Error building the documentation.

make -j1
allows the docs to build.

See e.g. https://github.com/sagemath/sagetrac-mirror/runs/3104375540

It's of course not totally surprising that several processes may try to, say, create the same directory. Perhaps the relevant part of the builder should be launched with this specific error being ignored?

@jhpalmieri
Copy link
Member

comment:86

It's strange because that doesn't look like part of the docbuild process that's done in parallel with anything else: it looks like the main reference page, built after the other parts of the reference manual. How reliably can you reproduce this?

I think this part of the process (copying static files) is controlled by Sphinx, not by Sage or its modifications to Sphinx, for what that's worth.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 21, 2021

comment:87

Replying to @dimpase:

I am seeing a race condition, sphinx may race FileExistsError

new ticket please?

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 22, 2021

comment:88

Replying to @mkoeppe:

Replying to @dimpase:

I am seeing a race condition, sphinx may race FileExistsError

new ticket please?

This is now #32262

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants