From d984f782b01fea55074a5dc3cecb4e430b719484 Mon Sep 17 00:00:00 2001 From: Greg Lucas Date: Wed, 31 May 2023 21:17:44 -0600 Subject: [PATCH 1/3] MNT/FIX: macosx change Timer to NSTimer instance Newer macos frameworks now support blocks, so we can take advantage of that and inline our method call while creating and scheduling the timer all at once. This removes some of the reference counting and alloc/dealloc record keeping. In the mac framework, an interval of 0 will only fire once, so if we aren't a singleshot timer we need to ensure that the interval is greater than 0. --- lib/matplotlib/backends/backend_macosx.py | 1 + .../tests/test_backends_interactive.py | 41 +++++++++++ src/_macosx.m | 73 ++++--------------- 3 files changed, 58 insertions(+), 57 deletions(-) diff --git a/lib/matplotlib/backends/backend_macosx.py b/lib/matplotlib/backends/backend_macosx.py index 204f81fd098b..b4cfcd480250 100644 --- a/lib/matplotlib/backends/backend_macosx.py +++ b/lib/matplotlib/backends/backend_macosx.py @@ -69,6 +69,7 @@ def callback_func(callback, timer): self._timers.remove(timer) timer.stop() timer = self.new_timer(interval=0) + timer.single_shot = True timer.add_callback(callback_func, callback, timer) self._timers.add(timer) timer.start() diff --git a/lib/matplotlib/tests/test_backends_interactive.py b/lib/matplotlib/tests/test_backends_interactive.py index bc0b7c75b922..82cb975292e7 100644 --- a/lib/matplotlib/tests/test_backends_interactive.py +++ b/lib/matplotlib/tests/test_backends_interactive.py @@ -624,3 +624,44 @@ def test_figure_leak_20490(env, time_mem): growth = int(result.stdout) assert growth <= acceptable_memory_leakage + + +def _impl_test_interactive_timers(): + # A timer with <1 millisecond gets converted to int and therefore 0 + # milliseconds, which the mac framework interprets as singleshot. + # We only want singleshot if we specify that ourselves, otherwise we want + # a repeating timer + import os + from unittest.mock import Mock + import matplotlib.pyplot as plt + # increase pause duration on CI to let things spin up + # particularly relevant for gtk3cairo + pause_time = 5 if os.getenv("CI") else 0.5 + fig = plt.figure() + plt.pause(pause_time) + timer = fig.canvas.new_timer(0.1) + mock = Mock() + timer.add_callback(mock) + timer.start() + plt.pause(pause_time) + timer.stop() + assert mock.call_count > 1 + + # Now turn it into a single shot timer and verify only one gets triggered + mock.call_count = 0 + timer.single_shot = True + timer.start() + plt.pause(pause_time) + assert mock.call_count == 1 + + # Make sure we can start the timer a second time + timer.start() + plt.pause(pause_time) + assert mock.call_count == 2 + plt.close("all") + + +@pytest.mark.parametrize("env", _get_testable_interactive_backends()) +def test_interactive_timers(env): + _run_helper(_impl_test_interactive_timers, + timeout=_test_timeout, extra_env=env) diff --git a/src/_macosx.m b/src/_macosx.m index 7042f6da35cc..a9fb2b8edc78 100755 --- a/src/_macosx.m +++ b/src/_macosx.m @@ -1811,7 +1811,8 @@ - (void)flagsChanged:(NSEvent *)event typedef struct { PyObject_HEAD - CFRunLoopTimerRef timer; + NSTimer* timer; + } Timer; static PyObject* @@ -1819,7 +1820,9 @@ - (void)flagsChanged:(NSEvent *)event { lazy_init(); Timer* self = (Timer*)type->tp_alloc(type, 0); - if (!self) { return NULL; } + if (!self) { + return NULL; + } self->timer = NULL; return (PyObject*) self; } @@ -1827,35 +1830,16 @@ - (void)flagsChanged:(NSEvent *)event static PyObject* Timer_repr(Timer* self) { - return PyUnicode_FromFormat("Timer object %p wrapping CFRunLoopTimerRef %p", + return PyUnicode_FromFormat("Timer object %p wrapping NSTimer %p", (void*) self, (void*)(self->timer)); } -static void timer_callback(CFRunLoopTimerRef timer, void* info) -{ - gil_call_method(info, "_on_timer"); -} - -static void context_cleanup(const void* info) -{ - Py_DECREF((PyObject*)info); -} - static PyObject* Timer__timer_start(Timer* self, PyObject* args) { - CFRunLoopRef runloop; - CFRunLoopTimerRef timer; - CFRunLoopTimerContext context; - CFAbsoluteTime firstFire; - CFTimeInterval interval; + NSTimeInterval interval; PyObject* py_interval = NULL, * py_single = NULL, * py_on_timer = NULL; int single; - runloop = CFRunLoopGetMain(); - if (!runloop) { - PyErr_SetString(PyExc_RuntimeError, "Failed to obtain run loop"); - return NULL; - } if (!(py_interval = PyObject_GetAttrString((PyObject*)self, "_interval")) || ((interval = PyFloat_AsDouble(py_interval) / 1000.), PyErr_Occurred()) || !(py_single = PyObject_GetAttrString((PyObject*)self, "_single")) @@ -1863,41 +1847,17 @@ static void context_cleanup(const void* info) || !(py_on_timer = PyObject_GetAttrString((PyObject*)self, "_on_timer"))) { goto exit; } - // (current time + interval) is time of first fire. - firstFire = CFAbsoluteTimeGetCurrent() + interval; - if (single) { - interval = 0; - } if (!PyMethod_Check(py_on_timer)) { PyErr_SetString(PyExc_RuntimeError, "_on_timer should be a Python method"); goto exit; } - Py_INCREF(self); - context.version = 0; - context.retain = NULL; - context.release = context_cleanup; - context.copyDescription = NULL; - context.info = self; - timer = CFRunLoopTimerCreate(kCFAllocatorDefault, - firstFire, - interval, - 0, - 0, - timer_callback, - &context); - if (!timer) { - PyErr_SetString(PyExc_RuntimeError, "Failed to create timer"); - goto exit; - } - if (self->timer) { - CFRunLoopTimerInvalidate(self->timer); - CFRelease(self->timer); - } - CFRunLoopAddTimer(runloop, timer, kCFRunLoopCommonModes); - /* Don't release the timer here, since the run loop may be destroyed and - * the timer lost before we have a chance to decrease the reference count - * of the attribute */ - self->timer = timer; + + // hold a reference to the timer so we can invalidate/stop it later + self->timer = [NSTimer scheduledTimerWithTimeInterval: interval + repeats: !single + block: ^(NSTimer *timer) { + gil_call_method((PyObject*)self, "_on_timer"); + }]; exit: Py_XDECREF(py_interval); Py_XDECREF(py_single); @@ -1913,8 +1873,7 @@ static void context_cleanup(const void* info) Timer__timer_stop(Timer* self) { if (self->timer) { - CFRunLoopTimerInvalidate(self->timer); - CFRelease(self->timer); + [self->timer invalidate]; self->timer = NULL; } Py_RETURN_NONE; @@ -1935,7 +1894,7 @@ static void context_cleanup(const void* info) .tp_repr = (reprfunc)Timer_repr, .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, .tp_new = (newfunc)Timer_new, - .tp_doc = "A Timer object wraps a CFRunLoopTimerRef and can add it to the event loop.", + .tp_doc = "A Timer object that contains an NSTimer that gets added to the event loop when started.", .tp_methods = (PyMethodDef[]){ // All docstrings are inherited. {"_timer_start", (PyCFunction)Timer__timer_start, From 7103c37e41af393314ff2a40523cf1a7dcc7a74b Mon Sep 17 00:00:00 2001 From: Greg Lucas Date: Thu, 1 Jun 2023 19:21:44 -0600 Subject: [PATCH 2/3] MNT: Ensure positive interval for timers Some backends don't handle 0 interval timers, so we can just set a minimum of 1 millisecond for the resolution of the timer to ensure a positive interval passed to all backend timers. --- lib/matplotlib/backend_bases.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/matplotlib/backend_bases.py b/lib/matplotlib/backend_bases.py index 6520b1d83ee3..75d0e6e60a4f 100644 --- a/lib/matplotlib/backend_bases.py +++ b/lib/matplotlib/backend_bases.py @@ -1164,7 +1164,8 @@ def interval(self): def interval(self, interval): # Force to int since none of the backends actually support fractional # milliseconds, and some error or give warnings. - interval = int(interval) + # Some backends also fail when interval == 0, so ensure >= 1 msec + interval = max(int(interval), 1) self._interval = interval self._timer_set_interval() From dc381289c58d35a5fd3970aaee65e54d0e27c28d Mon Sep 17 00:00:00 2001 From: Greg Lucas Date: Sat, 3 Jun 2023 11:24:55 -0600 Subject: [PATCH 3/3] CI: Skip gtk3cairo and wx interactive timer test --- lib/matplotlib/tests/test_backends_interactive.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/matplotlib/tests/test_backends_interactive.py b/lib/matplotlib/tests/test_backends_interactive.py index 82cb975292e7..5e9db4b66cc5 100644 --- a/lib/matplotlib/tests/test_backends_interactive.py +++ b/lib/matplotlib/tests/test_backends_interactive.py @@ -636,7 +636,7 @@ def _impl_test_interactive_timers(): import matplotlib.pyplot as plt # increase pause duration on CI to let things spin up # particularly relevant for gtk3cairo - pause_time = 5 if os.getenv("CI") else 0.5 + pause_time = 2 if os.getenv("CI") else 0.5 fig = plt.figure() plt.pause(pause_time) timer = fig.canvas.new_timer(0.1) @@ -663,5 +663,9 @@ def _impl_test_interactive_timers(): @pytest.mark.parametrize("env", _get_testable_interactive_backends()) def test_interactive_timers(env): + if env["MPLBACKEND"] == "gtk3cairo" and os.getenv("CI"): + pytest.skip("gtk3cairo timers do not work in remote CI") + if env["MPLBACKEND"] == "wx": + pytest.skip("wx backend is deprecated; tests failed on appveyor") _run_helper(_impl_test_interactive_timers, timeout=_test_timeout, extra_env=env)