[pypy-svn] r78745 - in pypy/trunk/pypy: interpreter interpreter/test module/signal module/signal/test module/thread module/thread/test translator/c/src

arigo at codespeak.net arigo at codespeak.net
Fri Nov 5 15:40:10 CET 2010


Author: arigo
Date: Fri Nov  5 15:40:08 2010
New Revision: 78745

Modified:
   pypy/trunk/pypy/interpreter/baseobjspace.py
   pypy/trunk/pypy/interpreter/executioncontext.py
   pypy/trunk/pypy/interpreter/test/test_executioncontext.py
   pypy/trunk/pypy/module/signal/__init__.py
   pypy/trunk/pypy/module/signal/interp_signal.py
   pypy/trunk/pypy/module/signal/test/test_signal.py
   pypy/trunk/pypy/module/thread/gil.py
   pypy/trunk/pypy/module/thread/test/test_gil.py
   pypy/trunk/pypy/translator/c/src/signals.h
Log:
Merge branch/signals.  Minor simplification, and fix for the issue of
ignored signals (e.g. Ctrl-C) that occurs about 20% of the time in
running "while 1: pass" on pypy-c-jit.


Modified: pypy/trunk/pypy/interpreter/baseobjspace.py
==============================================================================
--- pypy/trunk/pypy/interpreter/baseobjspace.py	(original)
+++ pypy/trunk/pypy/interpreter/baseobjspace.py	Fri Nov  5 15:40:08 2010
@@ -258,10 +258,9 @@
 
         self.interned_strings = {}
         self.actionflag = ActionFlag()    # changed by the signal module
+        self.check_signal_action = None   # changed by the signal module
         self.user_del_action = UserDelAction(self)
         self.frame_trace_action = FrameTraceAction(self)
-        self.actionflag.register_action(self.user_del_action)
-        self.actionflag.register_action(self.frame_trace_action)
 
         from pypy.interpreter.pycode import cpython_magic, default_magic
         self.our_magic = default_magic
@@ -299,8 +298,6 @@
                 self.timer.start("startup " + modname)
                 mod.init(self)
                 self.timer.stop("startup " + modname)
-        # Force the tick counter to have a valid value
-        self.actionflag.force_tick_counter()
 
     def finish(self):
         self.wait_for_thread_shutdown()

Modified: pypy/trunk/pypy/interpreter/executioncontext.py
==============================================================================
--- pypy/trunk/pypy/interpreter/executioncontext.py	(original)
+++ pypy/trunk/pypy/interpreter/executioncontext.py	Fri Nov  5 15:40:08 2010
@@ -171,19 +171,14 @@
         # this is split into a fast path and a slower path that is
         # not invoked every time bytecode_trace() is.
         actionflag = self.space.actionflag
-        ticker = actionflag.get()
-        if actionflag.has_bytecode_counter:    # this "if" is constant-folded
-            ticker += 1
-            actionflag.set(ticker)
-        if ticker & actionflag.interesting_bits:  # fast check
+        if actionflag.decrement_ticker() < 0:
             actionflag.action_dispatcher(self, frame)     # slow path
     bytecode_trace._always_inline_ = True
 
     def bytecode_trace_after_exception(self, frame):
         "Like bytecode_trace(), but without increasing the ticker."
         actionflag = self.space.actionflag
-        ticker = actionflag.get()
-        if ticker & actionflag.interesting_bits:  # fast check
+        if actionflag.get_ticker() < 0:
             actionflag.action_dispatcher(self, frame)     # slow path
     bytecode_trace_after_exception._always_inline_ = True
 
@@ -314,6 +309,13 @@
                 frame.last_exception = last_exception
                 self.is_tracing -= 1
 
+    def checksignals(self):
+        """Similar to PyErr_CheckSignals().  If called in the main thread,
+        and if signals are pending for the process, deliver them now
+        (i.e. call the signal handlers)."""
+        if self.space.check_signal_action is not None:
+            self.space.check_signal_action.perform(self, None)
+
     def _freeze_(self):
         raise Exception("ExecutionContext instances should not be seen during"
                         " translation.  Now is a good time to inspect the"
@@ -321,118 +323,88 @@
 
 
 class AbstractActionFlag(object):
-    """This holds the global 'action flag'.  It is a single bitfield
-    integer, with bits corresponding to AsyncAction objects that need to
-    be immediately triggered.  The correspondance from bits to
-    AsyncAction instances is built at translation time.  We can quickly
-    check if there is anything at all to do by checking if any of the
-    relevant bits is set.  If threads are enabled, they consume the 20
-    lower bits to hold a counter incremented at each bytecode, to know
-    when to release the GIL.
+    """This holds in an integer the 'ticker'.  If threads are enabled,
+    it is decremented at each bytecode; when it reaches zero, we release
+    the GIL.  And whether we have threads or not, it is forced to zero
+    whenever we fire any of the asynchronous actions.
     """
     def __init__(self):
         self._periodic_actions = []
         self._nonperiodic_actions = []
-        self.unused_bits = self.FREE_BITS[:]
         self.has_bytecode_counter = False
-        self.interesting_bits = 0
+        self.fired_actions = None
         self._rebuild_action_dispatcher()
 
     def fire(self, action):
-        """Request for the action to be run before the next opcode.
-        The action must have been registered at space initalization time."""
-        ticker = self.get()
-        self.set(ticker | action.bitmask)
-
-    def register_action(self, action):
-        "NOT_RPYTHON"
-        assert isinstance(action, AsyncAction)
-        if action.bitmask == 0:
-            while True:
-                action.bitmask = self.unused_bits.pop(0)
-                if not (action.bitmask & self.interesting_bits):
-                    break
-        self.interesting_bits |= action.bitmask
-        if action.bitmask & self.BYTECODE_COUNTER_OVERFLOW_BIT:
-            assert action.bitmask == self.BYTECODE_COUNTER_OVERFLOW_BIT
-            self._periodic_actions.append(action)
+        """Request for the action to be run before the next opcode."""
+        if not action._fired:
+            action._fired = True
+            if self.fired_actions is None:
+                self.fired_actions = []
+            self.fired_actions.append(action)
+            # set the ticker to -1 in order to force action_dispatcher()
+            # to run at the next possible bytecode
+            self.reset_ticker(-1)
+
+    def register_periodic_action(self, action, use_bytecode_counter):
+        """NOT_RPYTHON:
+        Register the PeriodicAsyncAction action to be called whenever the
+        tick counter becomes smaller than 0.  If 'use_bytecode_counter' is
+        True, make sure that we decrease the tick counter at every bytecode.
+        This is needed for threads.  Note that 'use_bytecode_counter' can be
+        False for signal handling, because whenever the process receives a
+        signal, the tick counter is set to -1 by C code in signals.h.
+        """
+        assert isinstance(action, PeriodicAsyncAction)
+        self._periodic_actions.append(action)
+        if use_bytecode_counter:
             self.has_bytecode_counter = True
-            self.force_tick_counter()
-        else:
-            self._nonperiodic_actions.append((action, action.bitmask))
         self._rebuild_action_dispatcher()
 
     def setcheckinterval(self, space, interval):
-        if interval < self.CHECK_INTERVAL_MIN:
-            interval = self.CHECK_INTERVAL_MIN
-        elif interval > self.CHECK_INTERVAL_MAX:
-            interval = self.CHECK_INTERVAL_MAX
+        if interval < 1:
+            interval = 1
         space.sys.checkinterval = interval
-        self.force_tick_counter()
-
-    def force_tick_counter(self):
-        # force the tick counter to a valid value -- this actually forces
-        # it to reach BYTECODE_COUNTER_OVERFLOW_BIT at the next opcode.
-        ticker = self.get()
-        ticker &= ~ self.BYTECODE_COUNTER_OVERFLOW_BIT
-        ticker |= self.BYTECODE_COUNTER_MASK
-        self.set(ticker)
 
     def _rebuild_action_dispatcher(self):
         periodic_actions = unrolling_iterable(self._periodic_actions)
-        nonperiodic_actions = unrolling_iterable(self._nonperiodic_actions)
-        has_bytecode_counter = self.has_bytecode_counter
 
         @jit.dont_look_inside
         def action_dispatcher(ec, frame):
-            # periodic actions
-            if has_bytecode_counter:
-                ticker = self.get()
-                if ticker & self.BYTECODE_COUNTER_OVERFLOW_BIT:
-                    # We must run the periodic actions now, but first
-                    # reset the bytecode counter (the following line
-                    # works by assuming that we just overflowed the
-                    # counter, i.e. BYTECODE_COUNTER_OVERFLOW_BIT is
-                    # set but none of the BYTECODE_COUNTER_MASK bits
-                    # are).
-                    ticker -= ec.space.sys.checkinterval
-                    self.set(ticker)
-                    for action in periodic_actions:
-                        action.perform(ec, frame)
+            # periodic actions (first reset the bytecode counter)
+            self.reset_ticker(ec.space.sys.checkinterval)
+            for action in periodic_actions:
+                action.perform(ec, frame)
 
             # nonperiodic actions
-            for action, bitmask in nonperiodic_actions:
-                ticker = self.get()
-                if ticker & bitmask:
-                    self.set(ticker & ~ bitmask)
+            list = self.fired_actions
+            if list is not None:
+                self.fired_actions = None
+                for action in list:
+                    action._fired = False
                     action.perform(ec, frame)
 
         action_dispatcher._dont_inline_ = True
         self.action_dispatcher = action_dispatcher
 
-    # Bits reserved for the bytecode counter, if used
-    BYTECODE_COUNTER_MASK = (1 << 20) - 1
-    BYTECODE_COUNTER_OVERFLOW_BIT = (1 << 20)
-
-    # Free bits
-    FREE_BITS = [1 << _b for _b in range(21, LONG_BIT-1)]
-
-    # The acceptable range of values for sys.checkinterval, so that
-    # the bytecode_counter fits in 20 bits
-    CHECK_INTERVAL_MIN = 1
-    CHECK_INTERVAL_MAX = BYTECODE_COUNTER_OVERFLOW_BIT
-
 
 class ActionFlag(AbstractActionFlag):
     """The normal class for space.actionflag.  The signal module provides
     a different one."""
-    _flags = 0
+    _ticker = 0
+
+    def get_ticker(self):
+        return self._ticker
 
-    def get(self):
-        return self._flags
+    def reset_ticker(self, value):
+        self._ticker = value
 
-    def set(self, value):
-        self._flags = value
+    def decrement_ticker(self):
+        value = self._ticker
+        if self.has_bytecode_counter:    # this 'if' is constant-folded
+            value -= 1
+            self._ticker = value
+        return value
 
 
 class AsyncAction(object):
@@ -440,7 +412,7 @@
     asynchronously with regular bytecode execution, but that still need
     to occur between two opcodes, not at a completely random time.
     """
-    bitmask = 0      # means 'please choose one bit automatically'
+    _fired = False
 
     def __init__(self, space):
         self.space = space
@@ -453,10 +425,11 @@
     def fire_after_thread_switch(self):
         """Bit of a hack: fire() the action but only the next time the GIL
         is released and re-acquired (i.e. after a potential thread switch).
-        Don't call this if threads are not enabled.
+        Don't call this if threads are not enabled.  Currently limited to
+        one action (i.e. reserved for CheckSignalAction from module/signal).
         """
         from pypy.module.thread.gil import spacestate
-        spacestate.set_actionflag_bit_after_thread_switch |= self.bitmask
+        spacestate.action_after_thread_switch = self
 
     def perform(self, executioncontext, frame):
         """To be overridden."""
@@ -466,7 +439,6 @@
     """Abstract base class for actions that occur automatically
     every sys.checkinterval bytecodes.
     """
-    bitmask = ActionFlag.BYTECODE_COUNTER_OVERFLOW_BIT
 
 
 class UserDelAction(AsyncAction):

Modified: pypy/trunk/pypy/interpreter/test/test_executioncontext.py
==============================================================================
--- pypy/trunk/pypy/interpreter/test/test_executioncontext.py	(original)
+++ pypy/trunk/pypy/interpreter/test/test_executioncontext.py	Fri Nov  5 15:40:08 2010
@@ -19,7 +19,6 @@
 
         space = self.space
         a1 = DemoAction(space)
-        space.actionflag.register_action(a1)
         for i in range(20):
             # assert does not raise:
             space.appexec([], """():
@@ -50,7 +49,7 @@
 
         space = self.space
         a2 = DemoAction(space)
-        space.actionflag.register_action(a2)
+        space.actionflag.register_periodic_action(a2, True)
         try:
             for i in range(500):
                 space.appexec([], """():
@@ -59,7 +58,7 @@
                 """)
         except Finished:
             pass
-        assert space.sys.checkinterval / 10 < i < space.sys.checkinterval * 3
+        assert space.sys.checkinterval / 10 < i < space.sys.checkinterval * 1.1
 
     def test_llprofile(self):
         l = []

Modified: pypy/trunk/pypy/module/signal/__init__.py
==============================================================================
--- pypy/trunk/pypy/module/signal/__init__.py	(original)
+++ pypy/trunk/pypy/module/signal/__init__.py	Fri Nov  5 15:40:08 2010
@@ -34,9 +34,7 @@
         MixedModule.__init__(self, space, *args)
         # add the signal-checking callback as an action on the space
         space.check_signal_action = interp_signal.CheckSignalAction(space)
-        space.actionflag.register_action(space.check_signal_action)
-        # use the C-level pypysig_occurred variable as the action flag
-        # (the result is that the C-level signal handler will directly
-        # set the flag for the CheckSignalAction)
+        space.actionflag.register_periodic_action(space.check_signal_action,
+                                                  use_bytecode_counter=False)
         space.actionflag.__class__ = interp_signal.SignalActionFlag
         # xxx yes I know the previous line is a hack

Modified: pypy/trunk/pypy/module/signal/interp_signal.py
==============================================================================
--- pypy/trunk/pypy/module/signal/interp_signal.py	(original)
+++ pypy/trunk/pypy/module/signal/interp_signal.py	Fri Nov  5 15:40:08 2010
@@ -1,6 +1,7 @@
 from pypy.interpreter.error import OperationError
 from pypy.interpreter.baseobjspace import W_Root, ObjSpace
 from pypy.interpreter.executioncontext import AsyncAction, AbstractActionFlag
+from pypy.interpreter.executioncontext import PeriodicAsyncAction
 import signal as cpy_signal
 from pypy.rpython.lltypesystem import lltype, rffi
 from pypy.translator.tool.cbuild import ExternalCompilationInfo
@@ -52,19 +53,29 @@
 
 
 class SignalActionFlag(AbstractActionFlag):
-    def get(self):
+    # This class uses the C-level pypysig_counter variable as the tick
+    # counter.  The C-level signal handler will reset it to -1 whenever
+    # a signal is received.
+
+    def get_ticker(self):
         p = pypysig_getaddr_occurred()
         return p.c_value
-    def set(self, value):
+
+    def reset_ticker(self, value):
         p = pypysig_getaddr_occurred()
         p.c_value = value
 
+    def decrement_ticker(self):
+        p = pypysig_getaddr_occurred()
+        value = p.c_value
+        if self.has_bytecode_counter:    # this 'if' is constant-folded
+            value -= 1
+            p.c_value = value
+        return value
 
-class CheckSignalAction(AsyncAction):
-    """An action that is automatically invoked when a signal is received."""
 
-    # The C-level signal handler sets the bit 30 of pypysig_occurred:
-    bitmask = 1 << 30
+class CheckSignalAction(PeriodicAsyncAction):
+    """An action that is automatically invoked when a signal is received."""
 
     def __init__(self, space):
         AsyncAction.__init__(self, space)
@@ -73,7 +84,6 @@
             # need a helper action in case signals arrive in a non-main thread
             self.pending_signals = {}
             self.reissue_signal_action = ReissueSignalAction(space)
-            space.actionflag.register_action(self.reissue_signal_action)
         else:
             self.reissue_signal_action = None
 

Modified: pypy/trunk/pypy/module/signal/test/test_signal.py
==============================================================================
--- pypy/trunk/pypy/module/signal/test/test_signal.py	(original)
+++ pypy/trunk/pypy/module/signal/test/test_signal.py	Fri Nov  5 15:40:08 2010
@@ -1,6 +1,38 @@
 import os, py
+import signal as cpy_signal
 from pypy.conftest import gettestobjspace
 
+
+class TestCheckSignals:
+
+    def setup_class(cls):
+        if not hasattr(os, 'kill') or not hasattr(os, 'getpid'):
+            py.test.skip("requires os.kill() and os.getpid()")
+        cls.space = gettestobjspace(usemodules=['signal'])
+
+    def test_checksignals(self):
+        space = self.space
+        w_received = space.appexec([], """():
+            import signal
+            received = []
+            def myhandler(signum, frame):
+                received.append(signum)
+            signal.signal(signal.SIGUSR1, myhandler)
+            return received""")
+        #
+        assert not space.is_true(w_received)
+        #
+        # send the signal now
+        os.kill(os.getpid(), cpy_signal.SIGUSR1)
+        #
+        # myhandler() should not be immediately called
+        assert not space.is_true(w_received)
+        #
+        # calling ec.checksignals() should call it
+        space.getexecutioncontext().checksignals()
+        assert space.is_true(w_received)
+
+
 class AppTestSignal:
 
     def setup_class(cls):
@@ -24,18 +56,12 @@
         signal.signal(signal.SIGUSR1, myhandler)
 
         posix.kill(posix.getpid(), signal.SIGUSR1)
-        for i in range(10000):
-             # wait a bit for the signal to be delivered to the handler
-            if received:
-                break
+        # the signal should be delivered to the handler immediately
         assert received == [signal.SIGUSR1]
         del received[:]
 
         posix.kill(posix.getpid(), signal.SIGUSR1)
-        for i in range(10000):
-             # wait a bit for the signal to be delivered to the handler
-            if received:
-                break
+        # the signal should be delivered to the handler immediately
         assert received == [signal.SIGUSR1]
         del received[:]
 

Modified: pypy/trunk/pypy/module/thread/gil.py
==============================================================================
--- pypy/trunk/pypy/module/thread/gil.py	(original)
+++ pypy/trunk/pypy/module/thread/gil.py	Fri Nov  5 15:40:08 2010
@@ -20,7 +20,8 @@
 
     def initialize(self, space):
         # add the GIL-releasing callback as an action on the space
-        space.actionflag.register_action(GILReleaseAction(space))
+        space.actionflag.register_periodic_action(GILReleaseAction(space),
+                                                  use_bytecode_counter=True)
 
     def setup_threads(self, space):
         """Enable threads in the object space, if they haven't already been."""
@@ -44,7 +45,6 @@
         # test_compile_lock.  As a workaround, we repatch these global
         # fields systematically.
         spacestate.ll_GIL = self.ll_GIL
-        spacestate.actionflag = space.actionflag
         invoke_around_extcall(before_external_call, after_external_call)
         return result
 
@@ -68,18 +68,17 @@
 
     def _freeze_(self):
         self.ll_GIL = thread.null_ll_lock
-        self.actionflag = None
-        self.set_actionflag_bit_after_thread_switch = 0
+        self.action_after_thread_switch = None
+        # ^^^ set by AsyncAction.fire_after_thread_switch()
         return False
 
     def after_thread_switch(self):
         # this is support logic for the signal module, to help it deliver
         # signals to the main thread.
-        actionflag = self.actionflag
-        if actionflag is not None:
-            flag = actionflag.get()
-            flag |= self.set_actionflag_bit_after_thread_switch
-            actionflag.set(flag)
+        action = self.action_after_thread_switch
+        if action is not None:
+            self.action_after_thread_switch = None
+            action.fire()
 
 spacestate = SpaceState()
 spacestate._freeze_()

Modified: pypy/trunk/pypy/module/thread/test/test_gil.py
==============================================================================
--- pypy/trunk/pypy/module/thread/test/test_gil.py	(original)
+++ pypy/trunk/pypy/module/thread/test/test_gil.py	Fri Nov  5 15:40:08 2010
@@ -8,7 +8,7 @@
     pass
 
 class FakeActionFlag(object):
-    def register_action(self, action):
+    def register_periodic_action(self, action, use_bytecode_counter):
         pass
     def get(self):
         return 0

Modified: pypy/trunk/pypy/translator/c/src/signals.h
==============================================================================
--- pypy/trunk/pypy/translator/c/src/signals.h	(original)
+++ pypy/trunk/pypy/translator/c/src/signals.h	Fri Nov  5 15:40:08 2010
@@ -47,16 +47,12 @@
 /* utility to poll for signals that arrived */
 int pypysig_poll(void);   /* => signum or -1 */
 
-/* When a signal is received, the bit 30 of pypysig_occurred is set.
-   After all signals are processed by pypysig_poll(), the bit 30 is
-   cleared again.  The variable is exposed and RPython code is free to
-   use the other bits in any way. */
-#define PENDING_SIGNAL_BIT   (1 << 30)
+/* When a signal is received, pypysig_counter is set to -1. */
 /* This is a struct for the JIT. See interp_signal.py. */
 struct pypysig_long_struct {
     long value;
 };
-extern struct pypysig_long_struct pypysig_occurred;
+extern struct pypysig_long_struct pypysig_counter;
 
 /* some C tricks to get/set the variable as efficiently as possible:
    use macros when compiling as a stand-alone program, but still
@@ -64,18 +60,20 @@
 #undef pypysig_getaddr_occurred
 void *pypysig_getaddr_occurred(void);
 #ifndef PYPY_NOT_MAIN_FILE
-void *pypysig_getaddr_occurred(void) { return (void *)(&pypysig_occurred); }
+void *pypysig_getaddr_occurred(void) { return (void *)(&pypysig_counter); }
 #endif
-#define pypysig_getaddr_occurred()   ((void *)(&pypysig_occurred))
+#define pypysig_getaddr_occurred()   ((void *)(&pypysig_counter))
 
 /************************************************************/
 /* Implementation                                           */
 
 #ifndef PYPY_NOT_MAIN_FILE
 
-struct pypysig_long_struct pypysig_occurred;
-static volatile long *pypysig_occurred_v = (volatile long *)&pypysig_occurred.value;
-static volatile int pypysig_flags[NSIG];
+struct pypysig_long_struct pypysig_counter = {0};
+static char volatile pypysig_flags[NSIG] = {0};
+static int volatile pypysig_occurred = 0;
+/* pypysig_occurred is only an optimization: it tells if any
+   pypysig_flags could be set. */
 
 void pypysig_ignore(int signum)
 {
@@ -108,10 +106,11 @@
 static void signal_setflag_handler(int signum)
 {
     if (0 <= signum && signum < NSIG)
+      {
         pypysig_flags[signum] = 1;
-    /* the point of "*pypysig_occurred_v" instead of just "pypysig_occurred"
-       is the volatile declaration */
-    *pypysig_occurred_v |= PENDING_SIGNAL_BIT;
+        pypysig_occurred = 1;
+        pypysig_counter.value = -1;
+      }
 }
 
 void pypysig_setflag(int signum)
@@ -130,27 +129,21 @@
 
 int pypysig_poll(void)
 {
-  /* the two commented out lines below are useful for performance in
-     normal usage of pypysig_poll(); however, pypy/module/signal/ is
-     not normal usage.  It only calls pypysig_poll() if the
-     PENDING_SIGNAL_BIT is set, and it clears that bit first. */
-
-/* if (pypysig_occurred & PENDING_SIGNAL_BIT) */
+  if (pypysig_occurred)
     {
-        int i;
-/*     pypysig_occurred &= ~PENDING_SIGNAL_BIT; */
-        for (i=0; i<NSIG; i++)
-            if (pypysig_flags[i])
-            {
-                pypysig_flags[i] = 0;
-                /* maybe another signal is pending: */
-                pypysig_occurred.value |= PENDING_SIGNAL_BIT;
-                return i;
-            }
+      int i;
+      pypysig_occurred = 0;
+      for (i=0; i<NSIG; i++)
+        if (pypysig_flags[i])
+          {
+            pypysig_flags[i] = 0;
+            pypysig_occurred = 1;   /* maybe another signal is pending */
+            return i;
+          }
     }
-    return -1;  /* no pending signal */
+  return -1;  /* no pending signal */
 }
 
-#endif
+#endif  /* !PYPY_NOT_MAIN_FILE */
 
 #endif



More information about the Pypy-commit mailing list