Merge pull request #1117 from gevent/faster-locals

Speed up gevent.local.local more, especially for subclasses (2-3x faster)

CHANGES.rst

@@ -103,6 +103,12 @@
   Hashemi and Kurt Rose. See :issue:`755`. As always, feedback is
   appreciated.
 
+- Simple subclasses of ``gevent.local.local`` now have the same
+  (substantially improved) performance characteristics of plain
+  ``gevent.local.local`` itself, making them 2 to 3 times faster than
+  before. See :pr:`1117`. If there are any compatibility
+  problems, please open issues.
+
 
 1.3a1 (2018-01-27)
 ==================

setup.py

@@ -55,11 +55,6 @@ SEMAPHORE = Extension(name="gevent._semaphore",
                       depends=['src/gevent/_semaphore.pxd'])
 SEMAPHORE = cythonize1(SEMAPHORE)
 
-LOCAL = Extension(name="gevent.local",
-                  sources=["src/gevent/local.py"],
-                  depends=['src/gevent/local.pxd'])
-LOCAL = cythonize1(LOCAL)
-
 # 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")]
@@ -70,6 +65,13 @@ if os.path.exists(venv_include_dir):
     include_dirs.append(venv_include_dir)
 
 
+LOCAL = Extension(name="gevent.local",
+                  sources=["src/gevent/local.py"],
+                  depends=['src/gevent/local.pxd'],
+                  include_dirs=include_dirs)
+LOCAL = cythonize1(LOCAL)
+
+
 GREENLET = Extension(name="gevent.greenlet",
                      sources=[
                          "src/gevent/greenlet.py",

src/gevent/_ident.py

 # -*- coding: utf-8 -*-
 # Copyright 2018 gevent contributors. See LICENSE for details.
+# cython: auto_pickle=False,embedsignature=True,always_allow_keywords=False
 
 from __future__ import absolute_import
 from __future__ import division

src/gevent/greenlet.pxd

@@ -2,12 +2,31 @@
 
 cimport cython
 from gevent._ident cimport IdentRegistry
+cdef bint _greenlet_imported
+cdef bint _PYPY
 
 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 SpawnedLink:
     cdef public object callback
 
@@ -87,7 +106,6 @@ cdef class Greenlet(greenlet):
 cdef _greenlet__init__
 cdef get_hub
 cdef wref
-cdef getcurrent
 
 cdef Timeout
 cdef dump_traceback

src/gevent/greenlet.py

@@ -7,9 +7,7 @@ from weakref import ref as wref
 
 
 from greenlet import greenlet
-from greenlet import getcurrent
 
-from gevent._compat import PYPY
 from gevent._compat import reraise
 from gevent._tblib import dump_traceback
 from gevent._tblib import load_traceback
@@ -21,6 +19,9 @@ from gevent.hub import iwait
 from gevent.hub import wait
 from gevent.timeout import Timeout
 
+_PYPY = hasattr(sys, 'pypy_version_info')
+
+
 __all__ = [
     'Greenlet',
     'joinall',
@@ -28,7 +29,15 @@ __all__ = [
 ]
 
 
-if PYPY:
+
+# 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
+
+if _PYPY:
     import _continuation # pylint:disable=import-error
     _continulet = _continuation.continulet
 
@@ -254,7 +263,7 @@ class Greenlet(greenlet):
         # Failed with exception: (t, v, dump_traceback(tb)))
         self._exc_info = None
 
-        spawner = getcurrent()
+        spawner = getcurrent() # pylint:disable=undefined-variable
         self.spawning_greenlet = wref(spawner)
         try:
             self.spawn_tree_locals = spawner.spawn_tree_locals
@@ -317,7 +326,7 @@ class Greenlet(greenlet):
 
     ### Lifecycle
 
-    if PYPY:
+    if _PYPY:
         # oops - pypy's .dead relies on __nonzero__ which we overriden above
         @property
         def dead(self):
@@ -618,7 +627,7 @@ class Greenlet(greenlet):
         if not block:
             raise Timeout()
 
-        switch = getcurrent().switch
+        switch = getcurrent().switch # pylint:disable=undefined-variable
         self.rawlink(switch)
         try:
             t = Timeout._start_new_or_dummy(timeout)
@@ -652,7 +661,7 @@ class Greenlet(greenlet):
         if self.ready():
             return
 
-        switch = getcurrent().switch
+        switch = getcurrent().switch # pylint:disable=undefined-variable
         self.rawlink(switch)
         try:
             t = Timeout._start_new_or_dummy(timeout)
@@ -912,3 +921,8 @@ def killall(greenlets, exception=GreenletExit, block=True, timeout=None):
             t.cancel()
     else:
         loop.run_callback(_killall, greenlets, exception)
+
+def _init():
+    greenlet_init() # pylint:disable=undefined-variable
+
+_init()

src/gevent/local.pxd

@@ -2,6 +2,37 @@
 
 cimport cython
 
+cdef bint _PYPY
+cdef ref
+cdef copy
+
+cdef object _marker
+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()
+
 @cython.final
 @cython.internal
 cdef class _wrefdict(dict):
@@ -9,7 +40,7 @@ cdef class _wrefdict(dict):
 
 @cython.final
 @cython.internal
-cdef class _thread_deleted:
+cdef class _greenlet_deleted:
     cdef object idt
     cdef object wrdicts
 
@@ -19,7 +50,7 @@ cdef class _thread_deleted:
 cdef class _local_deleted:
     cdef str key
     cdef object wrthread
-    cdef _thread_deleted thread_deleted
+    cdef _greenlet_deleted greenlet_deleted
 
 @cython.final
 @cython.internal
@@ -30,13 +61,36 @@ cdef class _localimpl:
     cdef object __weakref__
 
 
+@cython.final
+@cython.internal
+cdef class _localimpl_dict_entry:
+    cdef object wrgreenlet
+    cdef dict localdict
+
+@cython.locals(localdict=dict, key=str, greenlet=greenlet,
+               greenlet_deleted=_greenlet_deleted,
+               local_deleted=_local_deleted)
 cdef dict _localimpl_create_dict(_localimpl self)
+
+@cython.locals(entry=_localimpl_dict_entry, greenlet=greenlet)
 cdef inline dict _localimpl_get_dict(_localimpl self)
 
+cdef set _local_attrs
 
 cdef class local:
     cdef _localimpl _local__impl
+    cdef set _local_type_get_descriptors
+    cdef set _local_type_set_or_del_descriptors
+    cdef set _local_type_del_descriptors
+    cdef set _local_type_set_descriptors
+    cdef set _local_type_vars
+    cdef type _local_type
 
+
+@cython.locals(impl=_localimpl,dct=dict)
 cdef inline dict _local_get_dict(local self)
 
+@cython.locals(entry=_localimpl_dict_entry)
 cdef _local__copy_dict_from(local self, _localimpl impl, dict duplicate)
+
+cdef tuple _local_find_descriptors(local self)

src/gevent/local.py

+# cython: auto_pickle=False,embedsignature=True,always_allow_keywords=False
 """
 Greenlet-local objects.
 
@@ -152,18 +153,32 @@ affects what we see:
 """
 from __future__ import print_function
 
+import sys
+_PYPY = hasattr(sys, 'pypy_version_info')
 from copy import copy
 from weakref import ref
-from gevent.hub import getcurrent
 
+locals()['getcurrent'] = __import__('greenlet').getcurrent
+locals()['greenlet_init'] = lambda: None
 
-__all__ = ["local"]
+
+__all__ = [
+    "local",
+]
 
 
 class _wrefdict(dict):
     """A dict that can be weak referenced"""
 
-class _thread_deleted(object):
+class _greenlet_deleted(object):
+    """
+    A weakref callback for when the greenlet
+    is deleted.
+
+    If the greenlet is a `gevent.greenlet.Greenlet` and
+    supplies ``rawlink``, that will be used instead of a
+    weakref.
+    """
     __slots__ = ('idt', 'wrdicts')
 
     def __init__(self, idt, wrdicts):
@@ -176,12 +191,12 @@ class _thread_deleted(object):
             dicts.pop(self.idt, None)
 
 class _local_deleted(object):
-    __slots__ = ('key', 'wrthread', 'thread_deleted')
+    __slots__ = ('key', 'wrthread', 'greenlet_deleted')
 
-    def __init__(self, key, wrthread, thread_deleted):
+    def __init__(self, key, wrthread, greenlet_deleted):
         self.key = key
         self.wrthread = wrthread
-        self.thread_deleted = thread_deleted
+        self.greenlet_deleted = greenlet_deleted
 
     def __call__(self, _unused):
         thread = self.wrthread()
@@ -191,7 +206,7 @@ class _local_deleted(object):
             except AttributeError:
                 pass
             else:
-                unlink(self.thread_deleted)
+                unlink(self.greenlet_deleted)
             del thread.__dict__[self.key]
 
 class _localimpl(object):
@@ -212,6 +227,19 @@ class _localimpl(object):
         # again ourselves. MUST do this before setting any attributes.
         _localimpl_create_dict(self)
 
+class _localimpl_dict_entry(object):
+    """
+    The object that goes in the ``dicts`` of ``_localimpl``
+    object for each thread.
+    """
+    # This is a class, not just a tuple, so that cython can optimize
+    # attribute access
+    __slots__ = ('wrgreenlet', 'localdict')
+
+    def __init__(self, wrgreenlet, localdict):
+        self.wrgreenlet = wrgreenlet
+        self.localdict = localdict
+
 # We use functions instead of methods so that they can be cdef'd in
 # local.pxd; if they were cdef'd as methods, they would cause
 # the creation of a pointer and a vtable. This happens
@@ -222,15 +250,16 @@ class _localimpl(object):
 def _localimpl_get_dict(self):
     """Return the dict for the current thread. Raises KeyError if none
     defined."""
-    thread = getcurrent()
-    return self.dicts[id(thread)][1]
+    greenlet = getcurrent() # pylint:disable=undefined-variable
+    entry = self.dicts[id(greenlet)]
+    return entry.localdict
 
 def _localimpl_create_dict(self):
     """Create a new dict for the current thread, and return it."""
     localdict = {}
     key = self.key
-    thread = getcurrent()
-    idt = id(thread)
+    greenlet = getcurrent() # pylint:disable=undefined-variable
+    idt = id(greenlet)
 
     wrdicts = ref(self.dicts)
 
@@ -243,24 +272,24 @@ def _localimpl_create_dict(self):
     # can pro-actively clear out with a link, avoiding the
     # issue described above.Use rawlink to avoid spawning any
     # more greenlets.
-    thread_deleted = _thread_deleted(idt, wrdicts)
+    greenlet_deleted = _greenlet_deleted(idt, wrdicts)
 
     try:
-        rawlink = thread.rawlink
+        rawlink = greenlet.rawlink
     except AttributeError:
-        wrthread = ref(thread, thread_deleted)
+        wrthread = ref(greenlet, greenlet_deleted)
     else:
-        rawlink(thread_deleted)
-        wrthread = ref(thread)
+        rawlink(greenlet_deleted)
+        wrthread = ref(greenlet)
 
     # When the localimpl is deleted, remove the thread attribute.
-    local_deleted = _local_deleted(key, wrthread, thread_deleted)
+    local_deleted = _local_deleted(key, wrthread, greenlet_deleted)
 
 
     wrlocal = ref(self, local_deleted)
-    thread.__dict__[key] = wrlocal
+    greenlet.__dict__[key] = wrlocal
 
-    self.dicts[idt] = wrthread, localdict
+    self.dicts[idt] = _localimpl_dict_entry(wrthread, localdict)
     return localdict
 
 
@@ -276,22 +305,45 @@ def _local_get_dict(self):
         self.__init__(*args, **kw)
     return dct
 
+def _init():
+    greenlet_init() # pylint:disable=undefined-variable
+
+_local_attrs = {
+    '_local__impl',
+    '_local_type_get_descriptors',
+    '_local_type_set_or_del_descriptors',
+    '_local_type_del_descriptors',
+    '_local_type_set_descriptors',
+    '_local_type',
+    '_local_type_vars',
+    '__class__',
+    '__cinit__',
+}
+
 class local(object):
     """
     An object whose attributes are greenlet-local.
     """
-    __slots__ = ('_local__impl',)
+    __slots__ = tuple(_local_attrs - {'__class__', '__cinit__'})
 
     def __cinit__(self, *args, **kw):
         if args or kw:
             if type(self).__init__ == object.__init__:
                 raise TypeError("Initialization arguments are not supported", args, kw)
         impl = _localimpl(args, kw)
-        self._local__impl = impl # pylint:disable=attribute-defined-outside-init
+        # pylint:disable=attribute-defined-outside-init
+        self._local__impl = impl
+        get, dels, sets_or_dels, sets = _local_find_descriptors(self)
+        self._local_type_get_descriptors = get
+        self._local_type_set_or_del_descriptors = sets_or_dels
+        self._local_type_del_descriptors = dels
+        self._local_type_set_descriptors = sets
+        self._local_type = type(self)
+        self._local_type_vars = set(dir(self._local_type))
 
     def __getattribute__(self, name): # pylint:disable=too-many-return-statements
-        if name in ('__class__', '_local__impl', '__cinit__'):
-            # The _local__impl and __cinit__ won't be hit by the
+        if name in _local_attrs:
+            # The _local__impl,  __cinit__, etc, won't be hit by the
             # Cython version, if we've done things right. If we haven't,
             # they will be, and this will produce an error.
             return object.__getattribute__(self, name)
@@ -311,11 +363,10 @@ class local(object):
         # Similarly, a __getattr__ will still be called by _oga() if needed
         # if it's not in the dict.
 
-        type_self = type(self)
         # Optimization: If we're not subclassed, then
         # there can be no descriptors except for methods, which will
         # never need to use __dict__.
-        if type_self is local:
+        if self._local_type is local:
             return dct[name] if name in dct else object.__getattribute__(self, name)
 
         # NOTE: If this is a descriptor, this will invoke its __get__.
@@ -323,9 +374,8 @@ class local(object):
         # a None for the instance argument could mess us up here.
         # But this is faster than a loop over mro() checking each class __dict__
         # manually.
-        type_attr = getattr(type_self, name, _marker)
         if name in dct:
-            if type_attr is _marker:
+            if name not in self._local_type_vars:
                 # If there is a dict value, and nothing in the type,
                 # it can't possibly be a descriptor, so it is just returned.
                 return dct[name]
@@ -337,33 +387,34 @@ class local(object):
             # descriptor at all (doesn't have __get__), the instance wins.
             # NOTE that the docs for descriptors say that these methods must be
             # defined on the *class* of the object in the type.
-            type_type_attr = type(type_attr)
-            if not hasattr(type_type_attr, '__get__'):
+            if name not in self._local_type_get_descriptors:
                 # Entirely not a descriptor. Instance wins.
                 return dct[name]
-            if hasattr(type_type_attr, '__set__') or hasattr(type_type_attr, '__delete__'):
+            if name in self._local_type_set_or_del_descriptors:
                 # A data descriptor.
                 # arbitrary code execution while these run. If they touch self again,
                 # they'll call back into us and we'll repeat the dance.
-                return type_type_attr.__get__(type_attr, self, type_self)
+                type_attr = getattr(self._local_type, name)
+                return type(type_attr).__get__(type_attr, self, self._local_type)
             # Last case is a non-data descriptor. Instance wins.
             return dct[name]
-        elif type_attr is not _marker:
+        elif name in self._local_type_vars:
+            type_attr = getattr(self._local_type, name)
+
             # It's not in the dict at all. Is it in the type?
-            type_type_attr = type(type_attr)
-            if not hasattr(type_type_attr, '__get__'):
+            if name not in self._local_type_get_descriptors:
                 # Not a descriptor, can't execute code
                 return type_attr
-            return type_type_attr.__get__(type_attr, self, type_self)
+            return type(type_attr).__get__(type_attr, self, self._local_type)
 
         # It wasn't in the dict and it wasn't in the type.
         # So the next step is to invoke type(self)__getattr__, if it
         # exists, otherwise raise an AttributeError.
         # we will invoke type(self).__getattr__ or raise an attribute error.
-        if hasattr(type_self, '__getattr__'):
-            return type_self.__getattr__(self, name)
+        if hasattr(self._local_type, '__getattr__'):
+            return self._local_type.__getattr__(self, name)
         raise AttributeError("%r object has no attribute '%s'"
-                             % (type_self.__name__, name))
+                             % (self._local_type.__name__, name))
 
     def __setattr__(self, name, value):
         if name == '__dict__':
@@ -371,25 +422,23 @@ class local(object):
                 "%r object attribute '__dict__' is read-only"
                 % type(self))
 
-        if name == '_local__impl':
-            object.__setattr__(self, '_local__impl', value)
+        if name in _local_attrs:
+            object.__setattr__(self, name, value)
             return
 
         dct = _local_get_dict(self)
 
-        type_self = type(self)
-        if type_self is local:
+        if self._local_type is local:
             # Optimization: If we're not subclassed, we can't
             # have data descriptors, so this goes right in the dict.
             dct[name] = value
             return
 
-        type_attr = getattr(type_self, name, _marker)
-        if type_attr is not _marker:
-            type_type_attr = type(type_attr)
-            if hasattr(type_type_attr, '__set__'):
+        if name in self._local_type_vars:
+            if name in self._local_type_set_descriptors:
+                type_attr = getattr(self._local_type, name, _marker)
                 # A data descriptor, like a property or a slot.
-                type_type_attr.__set__(type_attr, self, value)
+                type(type_attr).__set__(type_attr, self, value)
                 return
         # Otherwise it goes directly in the dict
         dct[name] = value
@@ -400,13 +449,11 @@ class local(object):
                 "%r object attribute '__dict__' is read-only"
                 % self.__class__.__name__)
 
-        type_self = type(self)
-        type_attr = getattr(type_self, name, _marker)
-        if type_attr is not _marker:
-            type_type_attr = type(type_attr)
-            if hasattr(type_type_attr, '__delete__'):
+        if name in self._local_type_vars:
+            if name in self._local_type_del_descriptors:
                 # A data descriptor, like a property or a slot.
-                type_type_attr.__delete__(type_attr, self)
+                type_attr = getattr(self._local_type, name, _marker)
+                type(type_attr).__delete__(type_attr, self)
                 return
         # Otherwise it goes directly in the dict
 
@@ -431,14 +478,32 @@ class local(object):
         return instance
 
 def _local__copy_dict_from(self, impl, duplicate):
-    current = getcurrent()
+    current = getcurrent() # pylint:disable=undefined-variable
     currentId = id(current)
     new_impl = self._local__impl
     assert new_impl is not impl
-    tpl = new_impl.dicts[currentId]
-    new_impl.dicts[currentId] = (tpl[0], duplicate)
-
+    entry = new_impl.dicts[currentId]
+    new_impl.dicts[currentId] = _localimpl_dict_entry(entry.wrgreenlet, duplicate)
 
+def _local_find_descriptors(self):
+    type_self = type(self)
+    gets = set()
+    dels = set()
+    set_or_del = set()
+    sets = set()
+
+    for attr_name in dir(type_self):
+        attr = getattr(type_self, attr_name)
+        type_attr = type(attr)
+        if hasattr(type_attr, '__get__'):
+            gets.add(attr_name)
+        if hasattr(type_attr, '__delete__'):
+            dels.add(attr_name)
+            set_or_del.add(attr_name)
+        if hasattr(type_attr, '__set__'):
+            sets.add(attr_name)
+
+    return (gets, dels, set_or_del, sets)
 
 # Cython doesn't let us use __new__, it requires
 # __cinit__. But we need __new__ if we're not compiled
@@ -457,8 +522,7 @@ try:
     # in different ways. In CPython and PyPy3, it must be wrapped with classmethod;
     # in PyPy2, it must not. In either case, the args that get passed to
     # it are stil wrong.
-    import sys
-    if hasattr(sys, 'pypy_version_info') and sys.version_info[:2] < (3, 0):
+    if _PYPY and sys.version_info[:2] < (3, 0):
         local.__new__ = __new__
     else:
         local.__new__ = classmethod(__new__)
@@ -466,3 +530,5 @@ except TypeError: # pragma: no cover
     pass
 finally:
     del sys
+
+_init()

src/greentest/test__local.py

@@ -46,11 +46,21 @@ class Sentinel(object):
 
 
 class MyLocal(local):
+
+    CLASS_PROP = 42
+
     def __init__(self):
         local.__init__(self)
         self.sentinel = Sentinel()
         created_sentinels.append(id(self.sentinel))
 
+    @property
+    def desc(self):
+        return self
+
+class MyLocalSubclass(MyLocal):
+    pass
+
 class WithGetattr(local):
 
     def __getattr__(self, name):
@@ -215,6 +225,22 @@ class GeventLocalTestCase(greentest.TestCase):
 
         self.assertNotEqual(a.path, b.path, 'The values in the two objects must be different')
 
+    def test_class_attr(self, kind=MyLocal):
+        mylocal = kind()
+        self.assertEqual(42, mylocal.CLASS_PROP)
+
+        mylocal.CLASS_PROP = 1
+        self.assertEqual(1, mylocal.CLASS_PROP)
+        self.assertEqual(mylocal.__dict__['CLASS_PROP'], 1)
+
+        del mylocal.CLASS_PROP
+        self.assertEqual(42, mylocal.CLASS_PROP)
+
+        self.assertIs(mylocal, mylocal.desc)
+
+    def test_class_attr_subclass(self):
+        self.test_class_attr(kind=MyLocalSubclass)
+
     def test_locals_collected_when_greenlet_dead_but_still_referenced(self):
         # https://github.com/gevent/gevent/issues/387
         import gevent