libGAP! -- create a Cython library interface to gap

[vbraun]

Currently, libGAP doesn't give useful errors if something goes wrong during initialization.

Also, the GAP SIGINT handler got installed, this is fixed in the new spkg. Now Ctrl-C works as expected.

• Start with sage-5.6.beta3 or later.
• Install http://boxen.math.washington.edu/home/jdemeyer/spkg/libgap-4.5.7.spkg
• Apply attachment: 13588_libGAP_root.patch to the root repository
• Apply attachment: trac_6391_libGAP.patch to the Sage library
• Apply attachment: trac_13588_improve_libGAP.patch to the Sage library
• Apply attachment: trac_13588_exec_fix.patch to the Sage library
• Apply attachment: trac_13588_manifest.patch to the Sage library
• Apply attachment: trac_13588_docbuild.patch to the Sage library

The trac_6391_* patch has already been reviewed at #6391.

Depends on #13211 Depends on #13880

CC: @miguelmarco @sagetrac-tfeulner @gvol @simon-king-jena

Component: group theory

Author: Volker Braun

Reviewer: Dmitrii Pasechnik

Merged: sage-5.7.beta1

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

[vbraun]

comment:45

Replying to @jdemeyer:

Not really, as building the manual runs Sage code (as witnessed for example that Segmentation Faults run the Sage signal handler and display "Unhandled SIGSEGV...")

Theoretically true, but a normal Sage session won't start GAP until it has to and libGAP is lazily imported as well. Even then, none of that has anything to do with the elliptic curve stuff.

Right now the only thing in this ticket that seems to actually influence the docbuild is that the documentation becomes larger.

[jdemeyer]

comment:46

Replying to @vbraun:

Theoretically true, but a normal Sage session won't start GAP until it has to and libGAP is lazily imported as well.

The fact that I'm seeing "halving pool size" messages during the docbuilding proves that GAP is started.

[vbraun]

comment:47

Replying to @jdemeyer:

The fact that I'm seeing "halving pool size" messages during the docbuilding proves that GAP is started.

That indeed means that something is wonky. We are trying hard to pick a pool size that should fit into memory yet it doesn't. As I said before, this means that either there is very little memory to start with or the available address space is so fragmented that there is no contiguous piece left (32-bit only).

[vbraun]

comment:48

I guess sphinx has to resolve the lazy imports in order to get at the docstrings? That would pull in libGAP, which then subtracts ~250MB from the available address space. On a 32-bit machine that might push you over the cliff since sphinx is the most memory-demanding piece of the whole sage build process. Can you tell us which architecture we are talking about?

[jdemeyer]

comment:49

Sorry, I wasn't immediately aware of this, but this was with ulimit -v 2500000 (2.5GB) set on a 64-bit system.

[jdemeyer]

comment:50

This needs a patch to MANIFEST.in to add the Makefile (but again: I doubt that this really belongs in the Sage library).

[vbraun]

Attachment: trac_13588_manifest.patch.gz

Initial patch

[vbraun]

Description changed:

--- 
+++ 
@@ -8,5 +8,6 @@
 * Apply [attachment: trac_6391_libGAP.patch](https://github.com/sagemath/sage-prod/files/10656443/trac_6391_libGAP.patch.gz) to the Sage library
 * Apply [attachment: trac_13588_improve_libGAP.patch](https://github.com/sagemath/sage-prod/files/10656442/trac_13588_improve_libGAP.patch.gz) to the Sage library
 * Apply [attachment: trac_13588_exec_fix.patch](https://github.com/sagemath/sage-prod/files/10656445/trac_13588_exec_fix.patch.gz) to the Sage library
+* Apply [attachment: trac_13588_manifest.patch](https://github.com/sagemath/sage-prod/files/10656446/trac_13588_manifest.patch.gz) to the Sage library

 The `trac_6391_*` patches have already been reviewed at #6391, I'm just attaching them to this ticket for convenience.

[vbraun]

comment:51

I've added it to the manifest. I do think it is useful, when I started working on libgap that was one of the first things that I had to figure out.

[jdemeyer]

Attachment: 13588_libGAP_root.patch.gz

[jdemeyer]

Description changed:

--- 
+++ 
@@ -4,10 +4,10 @@

 * Start with sage-5.6.beta3 or later.
 * Install [http://www.stp.dias.ie/~vbraun/Sage/spkg/libgap-4.5.7.spkg](http://www.stp.dias.ie/~vbraun/Sage/spkg/libgap-4.5.7.spkg)
-* Apply [attachment: trac_6391_libGAP_root.patch](https://github.com/sagemath/sage-prod/files/10656444/trac_6391_libGAP_root.patch.gz) to the root repository
+* Apply [attachment: 13588_libGAP_root.patch](https://github.com/sagemath/sage-prod/files/10656447/13588_libGAP_root.patch.gz) to the root repository
 * Apply [attachment: trac_6391_libGAP.patch](https://github.com/sagemath/sage-prod/files/10656443/trac_6391_libGAP.patch.gz) to the Sage library
 * Apply [attachment: trac_13588_improve_libGAP.patch](https://github.com/sagemath/sage-prod/files/10656442/trac_13588_improve_libGAP.patch.gz) to the Sage library
 * Apply [attachment: trac_13588_exec_fix.patch](https://github.com/sagemath/sage-prod/files/10656445/trac_13588_exec_fix.patch.gz) to the Sage library
 * Apply [attachment: trac_13588_manifest.patch](https://github.com/sagemath/sage-prod/files/10656446/trac_13588_manifest.patch.gz) to the Sage library

-The `trac_6391_*` patches have already been reviewed at #6391, I'm just attaching them to this ticket for convenience.
+The `trac_6391_*` patch has already been reviewed at #6391.

[jdemeyer]

comment:53

New SAGE_ROOT patch which adds libgap as dependency of the Sage library.

[vbraun]

comment:54

Looks good to me!

[jdemeyer]

Patch needed for SPARC

[jdemeyer]

comment:55

Attachment: libgap_sparc.patch.gz

attachment: libgap_sparc.patch must be added to the spkg to work on SPARC.

[vbraun]

comment:56

Thanks. I've added it to the symbol rewriting code in the make_spkg.sh script.

#ifdef SPARC
#if SPARC
        asm("           .globl  SparcStackFuncBags              ");
        asm("   SparcStackFuncBags:                             ");
        asm("           ta      0x3     ! ST_FLUSH_WINDOWS      ");
        asm("           mov     %sp,%o0                         ");
        asm("           retl                                    ");
        asm("           nop                                     ");
#endif
#endif

/* sunos */
#ifdef SPARC
#if SPARC
        asm("           .globl  _SparcStackFuncBags             ");
        asm("   _SparcStackFuncBags:                            ");
        asm("           ta      0x3     ! ST_FLUSH_WINDOWS      ");
        asm("           mov     %sp,%o0                         ");
        asm("           retl                                    ");
        asm("           nop                                     ");
#endif
#endif
#else
#if defined(SPARC) && SPARC
void SparcStackFuncBags( void )
{
  asm (" ta 0x3 ");
  asm (" mov %sp,%o0" );
  return;
}
#endif

I presume the offending code has grown organically over the decades...

[jdemeyer]

comment:58

The similar operation needs to be done for

ItaniumRegisterStackTop

[jdemeyer]

comment:59

Also: please remove src/src/gasman.c.orig

[vbraun]

comment:60

Yay, more conditional compilation ;-) I've added the conditional itanium.s source. I also changed the make_spkg.sh script to start from the tarball, so no superfluous files should be included. I tested the new spkg (same location) successfully on sage.math and iras.

[jdemeyer]

comment:62

Replying to @vbraun:

I tested the new spkg

I assume that also includes running libgap doctests, because that's where the error shows up.

[vbraun]

comment:63

Well I checked that itanium.s is compiled and linked into libgap on iras.

[vbraun]

comment:64

I compiled sage on iras overnight and doctests pass, too ;-)

[jdemeyer]

comment:65

Something went wrong with your search/replace script, on mark (Solaris):

ld.so.1: python2.7: fatal: relocation error: file /home/buildbot/build/sage/mark-1/mark_full/build/sage-5.7.beta0/local/lib/libgap.so.0: symbol libGAP_libGAP_SparcStackFuncBags: referenced symbol not found

[vbraun]

comment:67

Fixed - the SparcStackFuncBags occurs in inline assembly and in C, so I now made sure to only rename it once.

[jdemeyer]

comment:68

I'm still not entirely happy with the memory allocation during docbuilding. For reasons I cannot understand, the documentation used to build fine with

ulimit -v 2500000

but now it needs (I tested 100MB increments)

ulimit -v 3500000

which is an increase of 1GB. Of course, the documentation is also getting bigger, but that explains only a small part of the increase. Also, on the buildbot I'm seeing sometimes errors of the form

sphinx-build -b html -d /home/buildbot/build/sage/iras-1/iras_full/build/sage-5.7.beta0/devel/sage/doc/output/doctrees/de/tutorial    /home/buildbot/build/sage/iras-1/iras_full/build/sage-5.7.beta0/devel/sage/doc/de/tutorial /home/buildbot/build/sage/iras-1/iras_full/build/sage-5.7.beta0/devel/sage/doc/output/html/de/tutorial
Traceback (most recent call last):
  File "/home/buildbot/build/sage/iras-1/iras_full/build/sage-5.7.beta0/devel/sage/doc/common/builder.py", line 1093, in <module>
    getattr(get_builder(name), type)()
  File "/home/buildbot/build/sage/iras-1/iras_full/build/sage-5.7.beta0/devel/sage/doc/common/builder.py", line 232, in _wrapper
    getattr(get_builder(document), name)(*args, **kwds)
  File "/home/buildbot/build/sage/iras-1/iras_full/build/sage-5.7.beta0/devel/sage/doc/common/builder.py", line 88, in f
    subprocess.call(build_command, shell=True)
  File "/home/buildbot/build/sage/iras-1/iras_full/build/sage-5.7.beta0/local/lib/python/subprocess.py", line 493, in call
    return Popen(*popenargs, **kwargs).wait()
  File "/home/buildbot/build/sage/iras-1/iras_full/build/sage-5.7.beta0/local/lib/python/subprocess.py", line 679, in __init__
    errread, errwrite)
  File "/home/buildbot/build/sage/iras-1/iras_full/build/sage-5.7.beta0/local/lib/python/subprocess.py", line 1143, in _execute_child
    self.pid = os.fork()
OSError: [Errno 12] Cannot allocate memory
make: *** [doc-html-mathjax] Error 1

I suspect that GAP has something to do with this.

Could you somehow fix the memory allocation to allocate an absolutely minimal amount during docbuilding? Maybe it would make sense to introduce an environment variable SAGE_GAP_MEGABYTES which could be user-set to the desired memory size and which would be set by the docbuilder to a small number (say, 1 MB).

[vbraun]

comment:69

I've added a setting to our Sphinx config (doc/common/conf.py) and builder config (build_options.py) to use only the minimum amount for GAP memory pool. The builder and sphinx itself both import everything to get at docstrings, so two libgap instances are being created when building the documentation. I verified that I can now build all documentation with ulimit -v 2500000).

[vbraun]

Description changed:

--- 
+++ 
@@ -9,5 +9,6 @@
 * Apply [attachment: trac_13588_improve_libGAP.patch](https://github.com/sagemath/sage-prod/files/10656442/trac_13588_improve_libGAP.patch.gz) to the Sage library
 * Apply [attachment: trac_13588_exec_fix.patch](https://github.com/sagemath/sage-prod/files/10656445/trac_13588_exec_fix.patch.gz) to the Sage library
 * Apply [attachment: trac_13588_manifest.patch](https://github.com/sagemath/sage-prod/files/10656446/trac_13588_manifest.patch.gz) to the Sage library
+* Apply [attachment: trac_13588_docbuild.patch](https://github.com/sagemath/sage-prod/files/10656449/trac_13588_docbuild.patch.gz) to the Sage library

 The `trac_6391_*` patch has already been reviewed at #6391.

[jdemeyer]

comment:70

The following is fairly reproducible (with #13717 applied):

sage -t  -force_lib devel/sagenb-main/sagenb/misc/support.py
**********************************************************************
File "/release/merger/sage-5.7.beta1/devel/sagenb-main/sagenb/misc/support.py", line 479:
    sage: sage.server.support.syseval(gap, "2+3")
Expected:
    '5'
Got:
    ** Gap crashed or quit executing '2+3;' **
    Restarting Gap and trying again
    '5'
**********************************************************************

Note that GAP is started with only 1MB of memory, as the top of this strace log shows:

execve("/release/merger/sage-5.7.beta1/local/gap/latest/bin/x86_64-unknown-linux-gnu-gcc-default64/gap", ["/release/merger/sage-5.7.beta1/local/gap/latest/bin/x86_64-unknown-linux-gnu-gcc-default64/gap", "-m", "24m", "-l", "/release/merger/sage-5.7.beta1/local/gap/latest", "-r", "-b", "-p", "-T", "-o", "1m", "-s", "1m", "/release/merger/sage-5.7.beta1/local/share/sage/ext/gap/sage.g"], [/* 84 vars */]) = 0

Full strace log: http://boxen.math.washington.edu/home/jdemeyer/gap.28216

[vbraun]

Attachment: trac_13588_docbuild.patch.gz

Updated patch

[vbraun]

comment:71

I see, introspection runs Sphinx as well so there is a chance to get conf.py imported in Sage. Though curiously, it doesn't cause any doctest failure on my machine. I moved the part that limits GAP to a branch in conf.py that is only executed when we actually build the documentation, this should fix it.

[jdemeyer]

Merged: sage-5.7.beta1

[jdemeyer]

Description changed:

--- 
+++ 
@@ -3,7 +3,7 @@
 Also, the GAP SIGINT handler got installed, this is fixed in the new spkg. Now Ctrl-C works as expected.

 * Start with sage-5.6.beta3 or later.
-* Install [http://www.stp.dias.ie/~vbraun/Sage/spkg/libgap-4.5.7.spkg](http://www.stp.dias.ie/~vbraun/Sage/spkg/libgap-4.5.7.spkg)
+* Install [http://boxen.math.washington.edu/home/jdemeyer/spkg/libgap-4.5.7.spkg](http://boxen.math.washington.edu/home/jdemeyer/spkg/libgap-4.5.7.spkg)
 * Apply [attachment: 13588_libGAP_root.patch](https://github.com/sagemath/sage-prod/files/10656447/13588_libGAP_root.patch.gz) to the root repository
 * Apply [attachment: trac_6391_libGAP.patch](https://github.com/sagemath/sage-prod/files/10656443/trac_6391_libGAP.patch.gz) to the Sage library
 * Apply [attachment: trac_13588_improve_libGAP.patch](https://github.com/sagemath/sage-prod/files/10656442/trac_13588_improve_libGAP.patch.gz) to the Sage library