Skip to content

Synchronize cypari2 stack and Python lifecycle #180

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion cypari2/gen.pxd
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
cimport cython
from cpython.object cimport PyObject
from cpython.weakref cimport PyWeakref_NewRef, PyWeakref_GetObject
from .types cimport GEN, pari_sp


Expand Down Expand Up @@ -28,14 +29,37 @@ cdef class Gen(Gen_base):
cdef inline pari_sp sp(self) noexcept:
return <pari_sp>self.address

# Enable weak references in Gen.
cdef object __weakref__

# The Gen objects on the PARI stack form a linked list, from the
# bottom to the top of the stack. This makes sense since we can only
# deallocate a Gen which is on the bottom of the PARI stack. If this
# is the last object on the stack, then next = top_of_stack
# (a singleton object).
#
# The connection between the list elements are implemented using the
# _next attribute. In order to not increase reference counts, the
# _next attribute is implemented as a weak reference.
# In the clone and constant cases, this is None.
cdef Gen next
#
# Do not set or access _next directly. Please use the get_next() and
# set_next() methods.
cdef object _next

cdef inline object get_next(self):
if self._next is None:
return None
cdef PyObject* result_ptr = PyWeakref_GetObject(self._next)
if result_ptr == NULL or <object>result_ptr is None:
return None
return <object>result_ptr

cdef inline void set_next(self, Gen value):
if value is None:
self._next = None
else:
self._next = PyWeakref_NewRef(value, None)

# A cache for __getitem__. Initially, this is None but it will be
# turned into a dict when needed.
Expand Down
24 changes: 16 additions & 8 deletions cypari2/gen.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,21 @@ cdef class Gen(Gen_base):
def __init__(self):
raise RuntimeError("PARI objects cannot be instantiated directly; use pari(x) to convert x to PARI")

def __del__(self):
if self.get_next() is not None:
# Python can deallocate Gen at any time even when it is not
# at the stack bottom. When self is not at the stack
# bottom, we need to move everything below it to heap
# before deallocation.
move_gens_to_heap(self.sp())

def __dealloc__(self):
if self.next is not None:
if self.get_next() is not None:
# stack
remove_from_pari_stack(self)
elif self.address is not NULL:
# clone
gunclone_deep(self.address)
gunclone(self.address)

cdef Gen new_ref(self, GEN g):
"""
Expand Down Expand Up @@ -191,7 +199,7 @@ cdef class Gen(Gen_base):
>>> pari("[[1, 2], 3]")[0][1] # indirect doctest
2
"""
if self.next is not None:
if self.get_next() is not None:
raise TypeError("cannot create reference to PARI stack (call fixGEN() first)")
if is_on_stack(g):
raise ValueError("new_ref() called with GEN which does not belong to parent")
Expand All @@ -205,8 +213,8 @@ cdef class Gen(Gen_base):
Return the PARI ``GEN`` corresponding to ``self`` which is
guaranteed not to change.
"""
if self.next is not None:
move_gens_to_heap(self.sp())
if self.get_next() is not None:
move_gens_to_heap((<Gen>self.get_next()).sp())
return self.g

cdef GEN ref_target(self) except NULL:
Expand Down Expand Up @@ -1532,7 +1540,7 @@ cdef class Gen(Gen_base):
raise IndexError("column j(=%s) must be between 0 and %s" % (j, self.ncols()-1))

self.cache((i, j), x)
xt = x.ref_target()
xt = x.fixGEN()
set_gcoeff(self.g, i+1, j+1, xt)
return

Expand All @@ -1556,7 +1564,7 @@ cdef class Gen(Gen_base):
raise IndexError("index (%s) must be between 0 and %s" % (i, glength(self.g)-1))

self.cache(i, x)
xt = x.ref_target()
xt = x.fixGEN()
if typ(self.g) == t_LIST:
listput(self.g, xt, i+1)
else:
Expand Down Expand Up @@ -4631,7 +4639,7 @@ cdef class Gen(Gen_base):
cdef int Gen_clear(self) except -1:
"""
Implementation of tp_clear() for Gen. We need to override Cython's
default since we do not want self.next to be cleared: it is crucial
default since we do not want self._next to be cleared: it is crucial
that the next Gen stays alive until remove_from_pari_stack(self) is
called by __dealloc__.
"""
Expand Down
2 changes: 1 addition & 1 deletion cypari2/pari_instance.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ cdef class Pari(Pari_auto):
for j in range(n):
sig_check()
x = objtogen(entries[k])
set_gcoeff(A.g, i+1, j+1, x.ref_target())
set_gcoeff(A.g, i+1, j+1, x.fixGEN())
A.cache((i, j), x)
k += 1
return A
Expand Down
20 changes: 13 additions & 7 deletions cypari2/stack.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,12 @@ cdef void remove_from_pari_stack(Gen self) noexcept:
print(f"Expected: 0x{self.sp():x}")
print(f"Actual: 0x{avma:x}")
else:
warn(f"cypari2 leaked {self.sp() - avma} bytes on the PARI stack",
RuntimeWarning, stacklevel=2)
n = self.next
# This could only happen when a PARI computation is interrupted, in which
# case the avma needs to be reset.
reset_avma()
n = self.get_next()
stackbottom = <PyObject*>n
self.next = None
self.set_next(None)
reset_avma()


Expand All @@ -81,7 +82,7 @@ cdef inline Gen Gen_stack_new(GEN x):
# the PARI stack.
n = <Gen>stackbottom
z = Gen_new(x, <GEN>avma)
z.next = n
z.set_next(n)
stackbottom = <PyObject*>z
sz = z.sp()
sn = n.sp()
Expand Down Expand Up @@ -116,12 +117,17 @@ cdef int move_gens_to_heap(pari_sp lim) except -1:
Move some/all Gens from the PARI stack to the heap.

If lim == -1, move everything. Otherwise, keep moving as long as
avma <= lim.
avma < lim.
"""
while avma <= lim and stackbottom is not <PyObject*>top_of_stack:
while avma < lim and stackbottom is not <PyObject*>top_of_stack:
current = <Gen>stackbottom
sig_on()
current.g = gclone(current.g)
# If current is a container type (e.g., a matrix or a vector),
# its entries will be cloned during this move. Hence, the
# itemcache will contain obsolete objects that needs to be
# cleared.
current.itemcache = None
sig_block()
remove_from_pari_stack(current)
sig_unblock()
Expand Down
Loading