Opened 6 years ago

Closed 22 months ago

#21612 closed defect (worksforme)

Incremental docbuild broken on 7.3

Reported by: infinity0 Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords:
Cc: jhpalmieri, dimpase, embray Merged in:
Authors: Reviewers: Erik Bray, John Palmieri
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by infinity0)

Whilst trying to package sagemath for Debian: https://anonscm.debian.org/cgit/debian-science/packages/sagemath.git/

During the docbuild, if I run:

$ find ./src/doc/en/reference/ -type f -exec ls -l --time-style=long-iso '{}' \; | sort -k6,7

I can see that the .rst files get touched during the docbuild. This breaks incremental build and makes it very time-consuming to debug segfaults.

I have already tried applying build/pkgs/sphinx/patches/environment.patch to my system sphinx. The patch applies, but the above behaviour remains.

Attachments (1)

fixsage-incremental-docbuild.patch (3.8 KB) - added by infinity0 6 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 6 years ago by infinity0

  • Description modified (diff)

comment:2 Changed 6 years ago by infinity0

  • Description modified (diff)

comment:3 Changed 6 years ago by mantepse

Is this the same as #21611?

comment:4 Changed 6 years ago by infinity0

No, it's definitely different. This is about incremental build, errors are not relevant.

More information: the doctrees and inventory is currently getting built in ~/.local/share/sagemath/doc, but the rst files are being built in sage.git/src/doc/en/reference and they're being deleted and re-written every time.

I'm testing out a patch for it. It's a massive hack, but it seems to be working and it'll take just a bit more time for me to make sure. Basically, fixing up the logic in write_auto_rest_file and commenting out some cleaning logic, both in src/sage_setup/docbuild/__init__.py.

Last edited 6 years ago by infinity0 (previous) (diff)

Changed 6 years ago by infinity0

comment:5 Changed 6 years ago by infinity0

Ok, here is the patch I'm using. Incremental build works again, but as you can see, it's quite a hack and I don't know how this affects the rest of sage. https://xkcd.com/1739/ is relevant, sigh.

comment:6 Changed 6 years ago by infinity0

Another bonus is that the docbuild (with the patch applied) seems to take less memory AND disk space.

For example, now my build actually finishes instead of sometimes getting killed due to OOM. And my doc/share/doctrees/en/reference/* build dir is now only 400MB instead of 14GB. I am fairly sure the 14GB included lots of duplicate entries - each subdir in there used to be 200MB, suspiciously all similar in size - whereas now each subdir is more proportional in size to the corresponding directory in the source repo.

comment:7 Changed 6 years ago by infinity0

OK, it looks like memory usage is not really improved directly. If I try the docbuild from scratch, my system will still fail it with an OOM.

However, this patch allows me to re-run the build - now it will skip the already-built stuff, proceed directly to the memory-intensive "Merging environment/index files" step, and this time it will succeed. This suggests that there is a rather bad memory leak in the docbuilder - the skipped stuff was unnecessary to perform the merging, yet it still used up enough memory to crash the docbuilder the first time around.

comment:8 Changed 2 years ago by mkoeppe

  • Cc jhpalmieri dimpase added
  • Milestone changed from sage-7.4 to sage-9.2

Wondering if this proposed change may still be relevant?

comment:9 Changed 2 years ago by dimpase

  • Cc embray added

comment:10 Changed 2 years ago by jhpalmieri

I think this was fixed elsewhere, but I'm not sure. Incremental docbuilding seems to work okay for me.

comment:11 Changed 2 years ago by embray

I haven't had any problems with incremental docbuilds. Unless there is a more recent report anyone can point to I propose closing as "worksforme".

comment:12 Changed 2 years ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

comment:13 Changed 2 years ago by jhpalmieri

  • Status changed from needs_review to positive_review

comment:14 Changed 22 months ago by slelievre

  • Resolution set to worksforme
  • Reviewers set to Erik Bray, John Palmieri
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.