Merge pull request #1116 from gevent/issue755

Add Greenlet.minimal_ident, a small, sequential, reusable counter for greenlets

.gitignore

@@ -10,6 +10,7 @@ src/gevent/__pycache__
 src/gevent/_semaphore.c
 src/gevent/local.c
 src/gevent/greenlet.c
+src/gevent/_ident.c
 src/gevent/libev/corecext.c
 src/gevent/libev/corecext.h
 src/gevent/libev/_corecffi.c

.pylintrc

 [MASTER]
-extension-pkg-whitelist=gevent.libuv._corecffi,gevent.libev._corecffi,gevent.local
+extension-pkg-whitelist=gevent.libuv._corecffi,gevent.libev._corecffi,gevent.local,gevent._ident
 
 [MESSAGES CONTROL]
 

CHANGES.rst

@@ -89,12 +89,21 @@
   and the code location that spawned them, in addition to maintaining
   a "spawn tree local" mapping. Based on a proposal from PayPal and
   comments by Mahmoud Hashemi and Kurt Rose. See :issue:`755` and
-  :pr:`1115`.
+  :pr:`1115`. As always, feedback is appreciated.
 
 - The :mod:`gevent.greenlet` module is now compiled with Cython to
   offset any performance decrease due to :issue:`755`. Please open
   issues for any compatibility concerns. See :pr:`1115`.
 
+- Greenlet objects now have a ``minimal_ident`` property. It functions
+  similarly to ``Thread.ident`` or ``id`` by uniquely identifying the
+  greenlet object while it remains alive, and it can be reused after
+  the greenlet object is dead. It is different in that it is small and
+  sequential. Based on a proposal from PayPal and comments by Mahmoud
+  Hashemi and Kurt Rose. See :issue:`755`. As always, feedback is
+  appreciated.
+
+
 1.3a1 (2018-01-27)
 ==================
 

doc/gevent.rst

@@ -37,6 +37,10 @@ generated.
 .. automethod:: Greenlet.__init__
 
 .. autoattribute:: Greenlet.exception
+.. autoattribute:: Greenlet.minimal_ident
+
+.. rubric:: Methods
+
 .. automethod:: Greenlet.ready
 .. automethod:: Greenlet.successful
 .. automethod:: Greenlet.start

setup.py

@@ -71,17 +71,31 @@ if os.path.exists(venv_include_dir):
 
 
 GREENLET = Extension(name="gevent.greenlet",
-                     sources=["src/gevent/greenlet.py"],
-                     depends=['src/gevent/greenlet.pxd'],
+                     sources=[
+                         "src/gevent/greenlet.py",
+                     ],
+                     depends=[
+                         'src/gevent/greenlet.pxd',
+                         'src/gevent/_ident.pxd',
+                         'src/gevent/_ident.py'
+                     ],
                      include_dirs=include_dirs)
 GREENLET = cythonize1(GREENLET)
 
+
+IDENT = Extension(name="gevent._ident",
+                  sources=["src/gevent/_ident.py"],
+                  depends=['src/gevent/_ident.pxd'],
+                  include_dirs=include_dirs)
+IDENT = cythonize1(IDENT)
+
 EXT_MODULES = [
     CORE,
     ARES,
     SEMAPHORE,
     LOCAL,
     GREENLET,
+    IDENT,
 ]
 
 LIBEV_CFFI_MODULE = 'src/gevent/libev/_corecffi_build.py:ffi'
@@ -110,6 +124,7 @@ if PYPY:
     EXT_MODULES.remove(CORE)
     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

src/gevent/_ident.pxd

+cimport cython
+
+cdef extern from "Python.h":
+
+    ctypedef class weakref.ref [object PyWeakReference]:
+        pass
+
+cdef heappop
+cdef heappush
+
+@cython.internal
+@cython.final
+cdef class ValuedWeakRef(ref):
+    cdef object value
+
+@cython.final
+cdef class IdentRegistry:
+    cdef object _registry
+    cdef list _available_idents
+
+    cpdef object get_ident(self, obj)
+    cpdef _return_ident(self, ValuedWeakRef ref)

src/gevent/_ident.py

+# -*- coding: utf-8 -*-
+# Copyright 2018 gevent contributors. See LICENSE for details.
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+
+
+from weakref import WeakKeyDictionary
+from weakref import ref
+
+from heapq import heappop
+from heapq import heappush
+
+__all__ = [
+    'IdentRegistry',
+]
+
+class ValuedWeakRef(ref):
+    """
+    A weak ref with an associated value.
+    """
+    # This seems entirely spurious; even on Python 2.7
+    # weakref.ref descends from object
+    # pylint: disable=slots-on-old-class
+    __slots__ = ('value',)
+
+
+class IdentRegistry(object):
+    """
+    Maintains a unique mapping of (small) positive integer identifiers
+    to objects that can be weakly referenced.
+
+    It is guaranteed that no two objects will have the the same
+    identifier at the same time, as long as those objects are
+    also uniquely hashable.
+    """
+
+    def __init__(self):
+        # {obj -> (ident, wref(obj))}
+        self._registry = WeakKeyDictionary()
+
+        # A heap of numbers that have been used and returned
+        self._available_idents = []
+
+    def get_ident(self, obj):
+        """
+        Retrieve the identifier for *obj*, creating one
+        if necessary.
+        """
+
+        try:
+            return self._registry[obj][0]
+        except KeyError:
+            pass
+
+        if self._available_idents:
+            # Take the smallest free number
+            ident = heappop(self._available_idents)
+        else:
+            # Allocate a bigger one
+            ident = len(self._registry)
+
+        vref = ValuedWeakRef(obj, self._return_ident)
+        vref.value = ident # pylint:disable=assigning-non-slot,attribute-defined-outside-init
+        self._registry[obj] = (ident, vref)
+        return ident
+
+    def _return_ident(self, vref):
+        # By the time this is called, self._registry has been
+        # updated
+        heappush(self._available_idents, vref.value)
+
+    def __len__(self):
+        return len(self._registry)

src/gevent/greenlet.pxd

 # cython: auto_pickle=False
 
 cimport cython
+from gevent._ident cimport IdentRegistry
 
 cdef extern from "greenlet/greenlet.h":
 
@@ -41,7 +42,6 @@ cdef class Greenlet(greenlet):
     cdef readonly args
     cdef readonly object spawning_greenlet
     cdef public dict spawn_tree_locals
-
     cdef readonly _Frame spawning_stack
 
     cdef list _links
@@ -50,6 +50,7 @@ cdef class Greenlet(greenlet):
     cdef object _start_event
     cdef dict _kwargs
     cdef str _formatted_info
+    cdef object _ident
 
     cpdef bint has_links(self)
     cpdef join(self, timeout=*)
@@ -58,6 +59,9 @@ cdef class Greenlet(greenlet):
     cpdef rawlink(self, object callback)
     cpdef str _formatinfo(self)
 
+    @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)

src/gevent/greenlet.py

@@ -146,6 +146,8 @@ class Greenlet(greenlet):
             object. Previously, passing a non-callable object would fail after the greenlet
             was spawned.
 
+        .. rubric:: Attributes
+
         .. attribute:: value
 
             Holds the value returned by the function if the greenlet has
@@ -245,6 +247,7 @@ class Greenlet(greenlet):
         self._notifier = None
         self._formatted_info = None
         self._links = []
+        self._ident = None
 
         # Initial state: None.
         # Completed successfully: (None, None, None)
@@ -266,6 +269,30 @@ class Greenlet(greenlet):
         self.spawning_stack = _extract_stack(self.spawning_stack_limit,
                                              getattr(spawner, 'spawning_stack', None))
 
+    def _get_minimal_ident(self):
+        reg = self.parent.ident_registry
+        return reg.get_ident(self)
+
+    @property
+    def minimal_ident(self):
+        """
+        A small, unique integer that identifies this object.
+
+        This is similar to :attr:`threading.Thread.ident` (and `id`)
+        in that as long as this object is alive, no other greenlet *in
+        this hub* will have the same id, but it makes a stronger
+        guarantee that the assigned values will be small and
+        sequential. Sometime after this object has died, the value
+        will be available for reuse.
+
+        To get ids that are unique across all hubs, combine this with
+        the hub's ``minimal_ident``.
+
+        .. versionadded:: 1.3a2
+        """
+        if self._ident is None:
+            self._ident = self._get_minimal_ident()
+        return self._ident
 
     @property
     def kwargs(self):

src/gevent/hub.py

@@ -32,6 +32,8 @@ from gevent._compat import string_types
 from gevent._compat import xrange
 from gevent._util import _NONE
 from gevent._util import readproperty
+from gevent._util import Lazy
+from gevent._ident import IdentRegistry
 
 if sys.version_info[0] <= 2:
     import thread # pylint:disable=import-error
@@ -419,7 +421,7 @@ def _config(default, envvar):
         return result.split(',')
     return result
 
-
+hub_ident_registry = IdentRegistry()
 
 class Hub(RawGreenlet):
     """
@@ -470,6 +472,11 @@ class Hub(RawGreenlet):
         self._resolver = None
         self._threadpool = None
         self.format_context = config.format_context
+        self.minimal_ident = hub_ident_registry.get_ident(self)
+
+    @Lazy
+    def ident_registry(self):
+        return IdentRegistry()
 
     @property
     def loop_class(self):

src/gevent/threading.py

@@ -54,7 +54,7 @@ Lock = _allocate_lock
 
 
 def _cleanup(g):
-    __threading__._active.pop(id(g), None)
+    __threading__._active.pop(_get_ident(g), None)
 
 def _make_cleanup_id(gid):
     def _(_r):
@@ -111,10 +111,12 @@ class _DummyThread(_DummyThread_):
         # It'd be nice to use a pattern like "greenlet-%d", but maybe somebody out
         # there is checking thread names...
         self._name = self._Thread__name = __threading__._newname("DummyThread-%d")
+        # All dummy threads in the same native thread share the same ident
+        # (that of the native thread)
         self._set_ident()
 
         g = getcurrent()
-        gid = _get_ident(g) # same as id(g)
+        gid = _get_ident(g)
         __threading__._active[gid] = self
         rawlink = getattr(g, 'rawlink', None)
         if rawlink is not None:

src/greentest/test___ident.py

+# -*- coding: utf-8 -*-
+# copyright 2018 gevent contributors. See LICENSE for details.
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+
+import gc
+
+
+import greentest
+from gevent._ident import IdentRegistry
+from gevent._compat import PYPY
+
+class Target(object):
+    pass
+
+class TestIdent(greentest.TestCase):
+
+    def setUp(self):
+        self.reg = IdentRegistry()
+
+    def tearDown(self):
+        self.reg = None
+
+    def test_basic(self):
+        target = Target()
+        self.assertEqual(0, self.reg.get_ident(target))
+        self.assertEqual(1, len(self.reg))
+
+        self.assertEqual(0, self.reg.get_ident(target))
+        self.assertEqual(1, len(self.reg))
+
+        target2 = Target()
+        self.assertEqual(1, self.reg.get_ident(target2))
+        self.assertEqual(2, len(self.reg))
+
+        self.assertEqual(1, self.reg.get_ident(target2))
+        self.assertEqual(2, len(self.reg))
+
+        self.assertEqual(0, self.reg.get_ident(target))
+
+        # When an object dies, we can re-use
+        # its id. Under PyPy we need to collect garbage first.
+        del target
+        if PYPY:
+            for _ in range(3):
+                gc.collect()
+
+        self.assertEqual(1, len(self.reg))
+
+        target3 = Target()
+        self.assertEqual(1, self.reg.get_ident(target2))
+        self.assertEqual(0, self.reg.get_ident(target3))
+        self.assertEqual(2, len(self.reg))
+
+    @greentest.skipOnPyPy("This would need to GC very frequently")
+    def test_circle(self):
+        keep_count = 3
+        keepalive = [None] * keep_count
+
+        for i in range(1000):
+            target = Target()
+            # Drop an old one.
+            keepalive[i % keep_count] = target
+            self.assertLessEqual(self.reg.get_ident(target), keep_count)
+
+
+
+
+if __name__ == '__main__':
+    greentest.main()

src/greentest/test__greenlet.py

@@ -255,6 +255,12 @@ class TestRaise_link_exception(TestRaise_link):
 
 class TestStuff(greentest.TestCase):
 
+    def test_minimal_id(self):
+        g = gevent.spawn(lambda: 1)
+        self.assertGreaterEqual(g.minimal_ident, 0)
+        self.assertGreaterEqual(g.parent.minimal_ident, 0)
+        g.join() # don't leave dangling, breaks the leak checks
+
     def test_wait_noerrors(self):
         x = gevent.spawn(lambda: 1)
         y = gevent.spawn(lambda: 2)

src/greentest/tests_that_dont_monkeypatch.txt

@@ -14,3 +14,4 @@ test__socket_send_memoryview.py
 test__socket_timeout.py
 test__examples.py
 test__issue330.py
+test___ident.py

src/greentest/tests_that_dont_use_resolver.txt

@@ -127,3 +127,4 @@ test_asyncore.py
 test___config.py
 test__destroy_default_loop.py
 test__util.py
+test___ident.py