Skip to content

Commit

Permalink
Honor PURE_PYTHON at runtime.
Browse files Browse the repository at this point in the history
Fixes #1118.

Also some speed improvements for semaphores.
  • Loading branch information
jamadden committed Feb 23, 2018
1 parent cfea657 commit 647918f
Show file tree
Hide file tree
Showing 22 changed files with 250 additions and 64 deletions.
5 changes: 5 additions & 0 deletions CHANGES.rst
Expand Up @@ -109,6 +109,11 @@
before. See :pr:`1117`. If there are any compatibility
problems, please open issues.

- On CPython, allow the pure-Python implementations of
``gevent.greenlet``, ``gevent.local`` and ``gevent.sempahore`` to be
used when the environment variable ``PURE_PYTHON`` is set. This is
not recommended except for debugging and testing. See :issue:`1118`.


1.3a1 (2018-01-27)
==================
Expand Down
5 changes: 4 additions & 1 deletion Makefile
Expand Up @@ -101,6 +101,9 @@ leaktest: test_prelim
@${PYTHON} scripts/travis.py fold_start leaktest "Running leak tests"
cd src/greentest && GEVENT_RESOLVER=thread GEVENTTEST_LEAKCHECK=1 ${PYTHON} testrunner.py --config known_failures.py --quiet --ignore tests_that_dont_do_leakchecks.txt
@${PYTHON} scripts/travis.py fold_end leaktest
@${PYTHON} scripts/travis.py fold_start default "Testing default backend pure python"
PURE_PYTHON=1 GEVENTTEST_COVERAGE=1 make basictest
@${PYTHON} scripts/travis.py fold_end default

bench:
${PYTHON} src/greentest/bench_sendall.py
Expand Down Expand Up @@ -179,7 +182,7 @@ develop:
@${PYTHON} scripts/travis.py fold_end install

test-py27: $(PY27)
PYTHON=python2.7.14 PATH=$(BUILD_RUNTIMES)/versions/python2.7.14/bin:$(PATH) make develop lint leaktest allbackendtest
PYTHON=python2.7.14 PATH=$(BUILD_RUNTIMES)/versions/python2.7.14/bin:$(PATH) make develop lint leaktest cffibackendtest coverage_combine

test-py34: $(PY34)
PYTHON=python3.4.7 PATH=$(BUILD_RUNTIMES)/versions/python3.4.7/bin:$(PATH) make develop basictest
Expand Down
11 changes: 8 additions & 3 deletions ci-requirements.txt
@@ -1,8 +1,13 @@
setuptools
wheel
# Python 3.7b1 requires at least this version.
# 0.28 is preferred due to 3.7 specific changes.
cython>=0.27.3

# Python 3.7 requires at least Cython 0.27.3.
# 0.28 is faster, and (important!) lets us specify the target module
# name to be created so that we can have both foo.py and _foo.so
# at the same time.
# This is an arbitrary commit that seems to work well.
-e git+https://github.com/cython/cython.git@63cd3bbb5eac22b92808eeb90b512359e3def20a#egg=cython

# Python 3.7b1 requires this.
greenlet>=0.4.13
pylint>=1.8.0
Expand Down
73 changes: 41 additions & 32 deletions setup.py
Expand Up @@ -50,11 +50,6 @@
from _setupares import ARES


SEMAPHORE = Extension(name="gevent._semaphore",
sources=["src/gevent/_semaphore.py"],
depends=['src/gevent/_semaphore.pxd'])
SEMAPHORE = cythonize1(SEMAPHORE)

# The sysconfig dir is not enough if we're in a virtualenv
# See https://github.com/pypa/pip/issues/4610
include_dirs = [sysconfig.get_path("include")]
Expand All @@ -64,32 +59,41 @@
if os.path.exists(venv_include_dir):
include_dirs.append(venv_include_dir)

SEMAPHORE = Extension(name="gevent.__semaphore",
sources=["src/gevent/_semaphore.py"],
depends=['src/gevent/__semaphore.pxd'],
include_dirs=include_dirs)

LOCAL = Extension(name="gevent.local",

LOCAL = Extension(name="gevent._local",
sources=["src/gevent/local.py"],
depends=['src/gevent/local.pxd'],
depends=['src/gevent/_local.pxd'],
include_dirs=include_dirs)
LOCAL = cythonize1(LOCAL)


GREENLET = Extension(name="gevent.greenlet",
GREENLET = Extension(name="gevent._greenlet",
sources=[
"src/gevent/greenlet.py",
],
depends=[
'src/gevent/greenlet.pxd',
'src/gevent/_ident.pxd',
'src/gevent/_greenlet.pxd',
'src/gevent/__ident.pxd',
'src/gevent/_ident.py'
],
include_dirs=include_dirs)
GREENLET = cythonize1(GREENLET)


IDENT = Extension(name="gevent._ident",
IDENT = Extension(name="gevent.__ident",
sources=["src/gevent/_ident.py"],
depends=['src/gevent/_ident.pxd'],
depends=['src/gevent/__ident.pxd'],
include_dirs=include_dirs)
IDENT = cythonize1(IDENT)

_to_cythonize = [
SEMAPHORE,
LOCAL,
GREENLET,
IDENT,
]

EXT_MODULES = [
CORE,
Expand Down Expand Up @@ -120,28 +124,33 @@
# but manylinux1 has only 2.5, so we set SKIP_LIBUV in the script make-manylinux
cffi_modules.append(LIBUV_CFFI_MODULE)

install_requires = [
# We need to watch our greenlet version fairly carefully,
# since we compile cython code that extends the greenlet object.
# Binary compatibility would break if the greenlet struct changes.
'greenlet >= 0.4.13; platform_python_implementation=="CPython"',
]

setup_requires = [
]

if PYPY:
install_requires = []
setup_requires = []
EXT_MODULES.remove(CORE)
# These use greenlet/greenlet.h, which doesn't exist on PyPy
EXT_MODULES.remove(LOCAL)
EXT_MODULES.remove(GREENLET)
EXT_MODULES.remove(IDENT)
EXT_MODULES.remove(SEMAPHORE)
# By building the semaphore with Cython under PyPy, we get
# atomic operations (specifically, exiting/releasing), at the
# cost of some speed (one trivial semaphore micro-benchmark put the pure-python version
# at around 1s and the compiled version at around 4s). Some clever subclassing
# and having only the bare minimum be in cython might help reduce that penalty.
# NOTE: You must use version 0.23.4 or later to avoid a memory leak.
# https://mail.python.org/pipermail/cython-devel/2015-October/004571.html
# However, that's all for naught on up to and including PyPy 4.0.1 which
# have some serious crashing bugs with GC interacting with cython,
# so this is disabled
else:
install_requires = ['greenlet >= 0.4.10'] # TODO: Replace this with platform markers?
setup_requires = []

# As of PyPy 5.10, this builds, but won't import (missing _Py_ReprEnter)
EXT_MODULES.remove(CORE)

_to_cythonize.remove(LOCAL)
_to_cythonize.remove(GREENLET)
_to_cythonize.remove(SEMAPHORE)

for mod in _to_cythonize:
EXT_MODULES.remove(mod)
EXT_MODULES.append(cythonize1(mod))
del _to_cythonize

try:
cffi = __import__('cffi')
Expand Down
2 changes: 2 additions & 0 deletions src/gevent/_ident.pxd → src/gevent/__ident.pxd
Expand Up @@ -7,6 +7,8 @@ cdef extern from "Python.h":

cdef heappop
cdef heappush
cdef object WeakKeyDictionary
cdef type ref

@cython.internal
@cython.final
Expand Down
32 changes: 31 additions & 1 deletion src/gevent/_semaphore.pxd → src/gevent/__semaphore.pxd
@@ -1,7 +1,37 @@
# cython: auto_pickle=False

cdef Timeout
cdef get_hub

cdef bint _greenlet_imported

cdef extern from "greenlet/greenlet.h":

ctypedef class greenlet.greenlet [object PyGreenlet]:
pass

# These are actually macros and so much be included
# (defined) in each .pxd, as are the two functions
# that call them.
greenlet PyGreenlet_GetCurrent()
void PyGreenlet_Import()

cdef inline greenlet getcurrent():
return PyGreenlet_GetCurrent()

cdef inline void greenlet_init():
global _greenlet_imported
if not _greenlet_imported:
PyGreenlet_Import()
_greenlet_imported = True


cdef void _init()


cdef class Semaphore:
cdef public int counter
cdef readonly object _links
cdef readonly list _links
cdef readonly object _notifier
cdef public int _dirty
cdef object __weakref__
Expand Down
3 changes: 3 additions & 0 deletions src/gevent/_compat.py
Expand Up @@ -6,13 +6,16 @@
from __future__ import print_function, absolute_import, division

import sys
import os


PY2 = sys.version_info[0] == 2
PY3 = sys.version_info[0] >= 3
PYPY = hasattr(sys, 'pypy_version_info')
WIN = sys.platform.startswith("win")

PURE_PYTHON = PYPY or os.getenv('PURE_PYTHON')

## Types

if PY3:
Expand Down
2 changes: 1 addition & 1 deletion src/gevent/greenlet.pxd → src/gevent/_greenlet.pxd
@@ -1,7 +1,7 @@
# cython: auto_pickle=False

cimport cython
from gevent._ident cimport IdentRegistry
from gevent.__ident cimport IdentRegistry
cdef bint _greenlet_imported
cdef bint _PYPY

Expand Down
4 changes: 4 additions & 0 deletions src/gevent/_ident.py
Expand Up @@ -74,3 +74,7 @@ def _return_ident(self, vref):

def __len__(self):
return len(self._registry)


from gevent._util import import_c_accel
import_c_accel(globals(), 'gevent.__ident')
File renamed without changes.
45 changes: 39 additions & 6 deletions src/gevent/_semaphore.py
@@ -1,10 +1,22 @@
# cython: auto_pickle=False
# cython: auto_pickle=False,embedsignature=True,always_allow_keywords=False
from __future__ import print_function, absolute_import, division
import sys
from gevent.hub import get_hub, getcurrent

from gevent.hub import get_hub
from gevent.timeout import Timeout


__all__ = ['Semaphore', 'BoundedSemaphore']
__all__ = [
'Semaphore',
'BoundedSemaphore',
]

# In Cython, we define these as 'cdef inline' functions. The
# compilation unit cannot have a direct assignment to them (import
# is assignment) without generating a 'lvalue is not valid target'
# error.
locals()['getcurrent'] = __import__('greenlet').getcurrent
locals()['greenlet_init'] = lambda: None


class Semaphore(object):
Expand Down Expand Up @@ -101,7 +113,7 @@ def _notify_links(self):
try:
link(self) # Must use Cython >= 0.23.4 on PyPy else this leaks memory
except: # pylint:disable=bare-except
getcurrent().handle_error((link, self), *sys.exc_info())
getcurrent().handle_error((link, self), *sys.exc_info()) # pylint:disable=undefined-variable

This comment has been minimized.

Copy link
@asavah

asavah Feb 25, 2018

Cython doesn't like this at all.
Same error for python 2.7.14 and 3.6.4
cython 0.27.3
Building from git:

Error compiling Cython file:
------------------------------------------------------------
...
                    if self.counter <= 0:
                        return
                    try:
                        link(self) # Must use Cython >= 0.23.4 on PyPy else this leaks memory
                    except: # pylint:disable=bare-except
                        getcurrent().handle_error((link, self), *sys.exc_info()) # pylint:disable=undefined-variable
                       ^
------------------------------------------------------------

src/gevent/_semaphore.py:116:24: undeclared name not builtin: getcurrent

Error compiling Cython file:
------------------------------------------------------------
...
            raise self._OVER_RELEASE_ERROR("Semaphore released too many times")
        Semaphore.release(self)


def _init():
    greenlet_init() # pylint:disable=undefined-variable
   ^
------------------------------------------------------------

src/gevent/_semaphore.py:286:4: undeclared name not builtin: greenlet_init
Warning: Extension name 'gevent.__semaphore' does not match fully qualified name 'gevent._semaphore' of 'src/gevent/_semaphore.py'
Compiling src/gevent/_semaphore.py because it changed.

This comment has been minimized.

Copy link
@jamadden

jamadden Feb 25, 2018

Author Member

This is tested with Cython 0.28a0. The specific commit is in the requirements file.

This comment has been minimized.

Copy link
@asavah

asavah Feb 26, 2018

Doh! Sorry for the noise, I focused on the code itself and didn't see the cython bump. Cython@master builds this perfectly fine.

if self._dirty:
# We mutated self._links so we need to start over
break
Expand Down Expand Up @@ -158,7 +170,7 @@ def _do_wait(self, timeout):
elapses, return the exception. Otherwise, return None.
Raises timeout if a different timer expires.
"""
switch = getcurrent().switch
switch = getcurrent().switch # pylint:disable=undefined-variable
self.rawlink(switch)
try:
timer = Timeout._start_new_or_dummy(timeout)
Expand Down Expand Up @@ -267,4 +279,25 @@ def __init__(self, *args, **kwargs):
def release(self):
if self.counter >= self._initial_value:
raise self._OVER_RELEASE_ERROR("Semaphore released too many times")
return Semaphore.release(self)
Semaphore.release(self)


def _init():
greenlet_init() # pylint:disable=undefined-variable

_init()

# By building the semaphore with Cython under PyPy, we get
# atomic operations (specifically, exiting/releasing), at the
# cost of some speed (one trivial semaphore micro-benchmark put the pure-python version
# at around 1s and the compiled version at around 4s). Some clever subclassing
# and having only the bare minimum be in cython might help reduce that penalty.
# NOTE: You must use version 0.23.4 or later to avoid a memory leak.
# https://mail.python.org/pipermail/cython-devel/2015-October/004571.html
# However, that's all for naught on up to and including PyPy 4.0.1 which
# have some serious crashing bugs with GC interacting with cython.
# It hasn't been tested since then, and PURE_PYTHON is assumed to be true
# for PyPy in all cases anyway, so this does nothing.

from gevent._util import import_c_accel
import_c_accel(globals(), 'gevent.__semaphore')
30 changes: 30 additions & 0 deletions src/gevent/_util.py
Expand Up @@ -71,6 +71,36 @@ def copy_globals(source,

return copied

def import_c_accel(globs, cname):
"""
Import the C-accelerator for the __name__
and copy its globals.
"""

name = globs.get('__name__')

if not name or name == cname:
# Do nothing if we're being exec'd as a file (no name)
# or we're running from the C extension
return

import importlib
from gevent._compat import PURE_PYTHON
if PURE_PYTHON:
return

mod = importlib.import_module(cname)

# By adopting the entire __dict__, we get a more accurate
# __file__ and module repr, plus we don't leak any imported
# things we no longer need.
globs.clear()
globs.update(mod.__dict__)

if 'import_c_accel' in globs:
del globs['import_c_accel']


class Lazy(object):
"""
A non-data descriptor used just like @property. The
Expand Down
11 changes: 7 additions & 4 deletions src/gevent/greenlet.py
@@ -1,6 +1,6 @@
# Copyright (c) 2009-2012 Denis Bilenko. See LICENSE for details.
# cython: auto_pickle=False,embedsignature=True,always_allow_keywords=False
from __future__ import absolute_import
from __future__ import absolute_import, print_function, division

import sys
from weakref import ref as wref
Expand Down Expand Up @@ -476,9 +476,9 @@ def exc_info(self):
.. versionadded:: 1.1
"""
e = self._exc_info
if e is not None and e[0] is not None:
return (e[0], e[1], load_traceback(e[2]))
exc_info = self._exc_info
if exc_info is not None and exc_info[0] is not None:
return (exc_info[0], exc_info[1], load_traceback(exc_info[2]))

def throw(self, *args):
"""Immediately switch into the greenlet and raise an exception in it.
Expand Down Expand Up @@ -926,3 +926,6 @@ def _init():
greenlet_init() # pylint:disable=undefined-variable

_init()

from gevent._util import import_c_accel
import_c_accel(globals(), 'gevent._greenlet')
3 changes: 3 additions & 0 deletions src/gevent/local.py
Expand Up @@ -532,3 +532,6 @@ def __new__(cls, *args, **kw):
del sys

_init()

from gevent._util import import_c_accel
import_c_accel(globals(), 'gevent._local')
2 changes: 2 additions & 0 deletions src/greentest/greentest/__init__.py
Expand Up @@ -73,6 +73,8 @@
from greentest.skipping import skipOnLibuvOnCI
from greentest.skipping import skipOnLibuvOnCIOnPyPy
from greentest.skipping import skipOnLibuvOnPyPyOnWin
from greentest.skipping import skipOnPurePython
from greentest.skipping import skipWithCExtensions


from greentest.exception import ExpectedException
Expand Down

0 comments on commit 647918f

Please sign in to comment.