Skip to content

Commit

Permalink
Performance and correctness improvements for Greenlets.
Browse files Browse the repository at this point in the history
- Rework getting the stack, since we're no longer arbitrarily extending it. This is faster, and I found and fixed a bug on CPython.
- Let Cython do more of the work making sure we really have a hub as our parent.
  • Loading branch information
jamadden committed Mar 29, 2019
1 parent 45d0006 commit 422a712
Show file tree
Hide file tree
Showing 9 changed files with 212 additions and 119 deletions.
13 changes: 13 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,19 @@
- Win: Make ``examples/process.py`` do something useful. See
:pr:`1378` by Robert Iannucci.

- Fix certain operations on a Greenlet in an invalid state (with an
invalid parent) to raise a `TypeError` sooner rather than an
`AttributeError` later. This is also slightly faster on CPython with
Cython. Inspired by :issue:`1363` as reported by Carson Ip. This
means that some extreme corner cases that might have passed by
replacing a Greenlet's parent with something that's not a gevent hub
now no longer will.

- Fix: The ``spawning_stack`` for Greenlets on CPython should now have
correct line numbers in more cases.

- Spawning greenlets can be up to 10% faster.

1.4.0 (2019-01-04)
==================

Expand Down
2 changes: 1 addition & 1 deletion src/gevent/__hub_primitives.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ cdef inline void greenlet_init():


cdef class WaitOperationsGreenlet(SwitchOutGreenletWithLoop):

# The Hub will extend this class.
cpdef wait(self, watcher)
cpdef cancel_wait(self, watcher, error, close_watcher=*)
cpdef _cancel_wait(self, watcher, error, close_watcher)
Expand Down
2 changes: 1 addition & 1 deletion src/gevent/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#: .. deprecated:: 1.2
#: Use ``pkg_resources.parse_version(__version__)`` (or the equivalent
#: ``packaging.version.Version(__version__)``).
version_info = _version_info(1, 4, 0, 'dev', 0)
version_info = _version_info(1, 5, 0, 'dev', 0)

#: The human-readable PEP 440 version identifier.
#: Use ``pkg_resources.parse_version(__version__)`` or
Expand Down
63 changes: 44 additions & 19 deletions src/gevent/_greenlet.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ cimport cython
from gevent.__ident cimport IdentRegistry
from gevent.__hub_local cimport get_hub_noargs as get_hub
from gevent.__waiter cimport Waiter
from gevent.__greenlet_primitives cimport SwitchOutGreenletWithLoop

cdef bint _PYPY
cdef sys_getframe
Expand All @@ -15,7 +16,12 @@ cdef InvalidSwitchError
cdef extern from "greenlet/greenlet.h":

ctypedef class greenlet.greenlet [object PyGreenlet]:
pass
# Defining this as a void* means we can't access it as a python attribute
# in the Python code; but we can't define it as a greenlet because that doesn't
# properly handle the case that it can be NULL. So instead we inline a getparent
# function that does the same thing as the green_getparent accessor but without
# going through the overhead of generic attribute lookup.
cdef void* parent

# These are actually macros and so much be included
# (defined) in each .pxd, as are the two functions
Expand All @@ -27,6 +33,17 @@ cdef extern from "greenlet/greenlet.h":
cdef inline greenlet getcurrent():
return PyGreenlet_GetCurrent()

cdef inline object get_generic_parent(greenlet s):
# We don't use any typed functions on the return of this,
# so save the type check by making it just an object.
if s.parent != NULL:
return <object>s.parent

cdef inline SwitchOutGreenletWithLoop get_my_hub(greenlet s):
# Must not be called with s = None
if s.parent != NULL:
return <object>s.parent

cdef bint _greenlet_imported

cdef inline void greenlet_init():
Expand All @@ -44,12 +61,25 @@ cdef extern from "frameobject.h":

ctypedef class types.FrameType [object PyFrameObject]:
cdef CodeType f_code
cdef int f_lineno
# We can't declare this in the object, because it's
# Accessing the f_lineno directly doesn't work. There is an accessor
# function, PyFrame_GetLineNumber that is needed to turn the raw line number
# into the executing line number.
# cdef int f_lineno
# We can't declare this in the object as an object, because it's
# allowed to be NULL, and Cython can't handle that.
# We have to go through the python machinery to get a
# proper None instead.
# cdef FrameType f_back
# proper None instead, or use an inline function.
cdef void* f_back

int PyFrame_GetLineNumber(FrameType frame)

@cython.nonecheck(False)
cdef inline FrameType get_f_back(FrameType frame):
if frame.f_back != NULL:
return <FrameType>frame.f_back

cdef inline int get_f_lineno(FrameType frame):
return PyFrame_GetLineNumber(frame)

cdef void _init()

Expand All @@ -75,25 +105,20 @@ cdef class _Frame:


@cython.final
@cython.locals(frames=list,frame=FrameType)
cdef inline list _extract_stack(int limit)

@cython.final
@cython.locals(previous=_Frame, frame=tuple, f=_Frame)
cdef _Frame _Frame_from_list(list frames)

@cython.locals(frame=FrameType,
newest_Frame=_Frame,
newer_Frame=_Frame,
older_Frame=_Frame)
cdef inline _Frame _extract_stack(int limit)

cdef class Greenlet(greenlet):
cdef readonly object value
cdef readonly tuple args
cdef readonly dict kwargs
cdef readonly object spawning_greenlet
cdef readonly _Frame spawning_stack
cdef public dict spawn_tree_locals

# This is accessed with getattr() dynamically so it
# must be visible to Python
cdef readonly list _spawning_stack_frames

cdef list _links
cdef tuple _exc_info
cdef object _notifier
Expand All @@ -108,10 +133,11 @@ cdef class Greenlet(greenlet):
cpdef rawlink(self, object callback)
cpdef str _formatinfo(self)

# This is a helper function for a @property getter;
# defining locals() for a @property doesn't seem to work.
@cython.locals(reg=IdentRegistry)
cdef _get_minimal_ident(self)


cdef bint __started_but_aborted(self)
cdef bint __start_cancelled_by_kill(self)
cdef bint __start_pending(self)
Expand All @@ -132,8 +158,6 @@ cdef class Greenlet(greenlet):
# TypeError: wrap() takes exactly one argument (0 given)
# cpdef _raise_exception(self)



# Declare a bunch of imports as cdefs so they can
# be accessed directly as static vars without
# doing a module global lookup. This is especially important
Expand Down Expand Up @@ -165,6 +189,7 @@ cdef class _dummy_event:
cdef _dummy_event _cancelled_start_event
cdef _dummy_event _start_completed_event

cpdef _kill(Greenlet glet, object exception, object waiter)

@cython.locals(diehards=list)
cdef _killall3(list greenlets, object exception, object waiter)
Expand Down
12 changes: 9 additions & 3 deletions src/gevent/_hub_primitives.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,15 @@ def wait(self, watcher):
try:
result = waiter.get()
if result is not waiter:
raise InvalidSwitchError('Invalid switch into %s: %r (expected %r)' % (
getcurrent(), # pylint:disable=undefined-variable
result, waiter))
raise InvalidSwitchError(
'Invalid switch into %s: got %r (expected %r; waiting on %r with %r)' % (
getcurrent(), # pylint:disable=undefined-variable
result,
waiter,
self,
watcher
)
)
finally:
watcher.stop()

Expand Down
2 changes: 1 addition & 1 deletion src/gevent/_ident.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class ValuedWeakRef(ref):

class IdentRegistry(object):
"""
Maintains a unique mapping of (small) positive integer identifiers
Maintains a unique mapping of (small) non-negative integer identifiers
to objects that can be weakly referenced.
It is guaranteed that no two objects will have the the same
Expand Down
Loading

0 comments on commit 422a712

Please sign in to comment.