Skip to content

Commit

Permalink
Trac #27680: MR11: Lazy initialization of libgap
Browse files Browse the repository at this point in the history
E. Madison Bray ([https://gitlab.com/embray @embray]) opened a merge
request at https://gitlab.com/sagemath/sage/merge_requests/11:
----
{{{
#!markdown
Previously the `Gap` class that implements the (effectively singleton)
`libgap` instance variable in `sage.libs.gap.libgap` called
`initialize()` (the function which initializes libgap for the process)
in its `__init__` method.

Because of this, it was impossible to import it without causing libgap
to be initialized, creating slowdown during Sage initialization.  The
tradeoff is that most code in Sage that uses the libgap interface has to
awkwardly use inline imports of `from sage.libs.gap.libgap import
libgap` all over the place.

I would like to do away with that, especially for work on
[#18267](https://trac.sagemath.org/ticket/18267).

This removes `initialize()` from `Gap.__init__()` and instead carefully
places calls to `initialize()` just in the few places where it's
absolutely crucial to ensure libgap is initialized first (specifically
in code paths that users and developers are actually intended to use
directly; it is not added directly to every single function that uses
GAP objects).

This sacrifices some simplicity in implementation of the libgap
interface for simplicity in using it, which I think is crucial for
updating more code in Sage to use it over the pexpect interface.
}}}

URL: https://trac.sagemath.org/27680
Reported by: galois
Ticket author(s): Erik Bray
Reviewer(s): Dima Pasechnik
  • Loading branch information
Release Manager authored and vbraun committed May 8, 2019
2 parents 28b02c4 + e687ee6 commit 51b11e4
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 22 deletions.
5 changes: 1 addition & 4 deletions src/sage/libs/gap/element.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ from cpython.object cimport Py_EQ, Py_NE, Py_LE, Py_GE, Py_LT, Py_GT
from cysignals.signals cimport sig_on, sig_off

from .gap_includes cimport *
from .libgap import libgap
from .util cimport *
from .util import GAPError
from sage.cpython.string cimport str_to_bytes, char_to_str
Expand Down Expand Up @@ -73,7 +74,6 @@ cdef Obj make_gap_list(sage_list) except NULL:
The list of the elements in ``a`` as a Gap ``Obj``.
"""
from sage.libs.gap.libgap import libgap
cdef GapElement l = libgap.eval('[]')
cdef GapElement elem
for x in sage_list:
Expand Down Expand Up @@ -104,7 +104,6 @@ cdef Obj make_gap_matrix(sage_list, gap_ring) except NULL:
The list of the elements in ``sage_list`` as a Gap ``Obj``.
"""
from sage.libs.gap.libgap import libgap
cdef GapElement l = libgap.eval('[]')
cdef GapElement elem
cdef GapElement one
Expand Down Expand Up @@ -170,7 +169,6 @@ cdef Obj make_gap_record(sage_dict) except NULL:
sage: libgap({'a': 1, 'b':123}) # indirect doctest
rec( a := 1, b := 123 )
"""
from sage.libs.gap.libgap import libgap
data = [ (str(key), libgap(value)) for key, value in sage_dict.iteritems() ]

cdef Obj rec
Expand Down Expand Up @@ -578,7 +576,6 @@ cdef class GapElement(RingElement):
...
GAPError: Error, no method found! Error, no 1st choice method found for `in' on 2 arguments
"""
from sage.libs.gap.libgap import libgap
GAP_IN = libgap.eval(r'\in')
return GAP_IN(other, self).sage()

Expand Down
20 changes: 7 additions & 13 deletions src/sage/libs/gap/libgap.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -228,17 +228,6 @@ from sage.misc.randstate cimport current_randstate
from sage.misc.superseded import deprecated_function_alias, deprecation


############################################################################
### Debugging ##############################################################
############################################################################

from sage.misc.lazy_import import is_during_startup
if is_during_startup():
import sys, traceback
print('Importing libgap during startup!')
traceback.print_stack(None, None, sys.stdout)


############################################################################
### Gap ###################################################################
############################################################################
Expand Down Expand Up @@ -312,6 +301,7 @@ class Gap(Parent):
[ 0.333333, 0.8, 3. ]
"""
initialize()
if isinstance(x, GapElement):
return x
elif isinstance(x, (list, tuple, Vector)):
Expand Down Expand Up @@ -412,6 +402,7 @@ class Gap(Parent):
if not isinstance(gap_command, basestring):
gap_command = str(gap_command._gap_init_())

initialize()
elem = make_any_gap_element(self, gap_eval(gap_command))

# If the element is NULL just return None instead
Expand Down Expand Up @@ -445,6 +436,7 @@ class Gap(Parent):
sage: libgap.function_factory('Print')
<Gap function "Print">
"""
initialize()
return make_GapElement_Function(self, gap_eval(function_name))

def set_global(self, variable, value):
Expand Down Expand Up @@ -552,6 +544,7 @@ class Gap(Parent):
1
"""
from sage.libs.gap.context_managers import GlobalVariableContext
initialize()
return GlobalVariableContext(variable, value)

def set_seed(self, seed=None):
Expand Down Expand Up @@ -630,8 +623,6 @@ class Gap(Parent):
sage: type(libgap._get_object())
<class 'sage.libs.gap.libgap.Gap'>
"""
initialize()
from sage.rings.integer_ring import ZZ
Parent.__init__(self, base=ZZ)

def __repr__(self):
Expand Down Expand Up @@ -690,9 +681,11 @@ class Gap(Parent):
from sage.libs.gap.gap_functions import common_gap_functions
from sage.libs.gap.gap_globals import common_gap_globals
if name in common_gap_functions:
initialize()
g = make_GapElement_Function(self, gap_eval(name))
assert g.is_function()
elif name in common_gap_globals:
initialize()
g = make_any_gap_element(self, gap_eval(name))
else:
raise AttributeError(f'No such attribute: {name}.')
Expand Down Expand Up @@ -785,6 +778,7 @@ class Gap(Parent):
sage: del a
sage: libgap.collect()
"""
initialize()
rc = CollectBags(0, 1)
if rc != 1:
raise RuntimeError('Garbage collection failed.')
Expand Down
5 changes: 0 additions & 5 deletions src/sage/tests/startup.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
r"""
Ensure that certain modules are not loaded on startup.
EXAMPLES::
sage: 'sage.libs.gap.libgap' in sys.modules
False
Check that IPython is not imported at startup (:trac:`18726`). It is
imported by the doctest framework, so the simple test like above would
not work. Instead, we test this by starting a new Python process::
Expand Down

0 comments on commit 51b11e4

Please sign in to comment.