Skip to content

Commit eadb499

Browse files
njsmithnjsmith
authored
Merge pull request #1588 from njsmith/autojump-without-a-task
Remove the autojump task
2 parents d1a0a2c + 6b4341c commit eadb499

File tree

8 files changed

+284
-242
lines changed

8 files changed

+284
-242
lines changed

‎newsfragments/1587.misc.rst‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
We refactored `trio.testing.MockClock` so that it no longer needs to
2+
run an internal task to manage autojumping. This should be mostly
3+
invisible to users, but there is one semantic change: the interaction
4+
between `trio.testing.wait_all_tasks_blocked` and the autojump clock
5+
was fixed. Now, the autojump will always wait until after all
6+
`~trio.testing.wait_all_tasks_blocked` calls have finished before
7+
firing, instead of it depending on which threshold values you passed.

‎trio/_core/__init__.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@
7171

7272
from ._thread_cache import start_thread_soon
7373

74+
from ._mock_clock import MockClock
75+
7476
# Kqueue imports
7577
try:
7678
from ._run import current_kqueue, monitor_kevent, wait_kevent

trio/testing/_mock_clock.py renamed to trio/_core/_mock_clock.py

Lines changed: 34 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from math import inf
33

44
from .. import _core
5+
from ._run import GLOBAL_RUN_CONTEXT
56
from .._abc import Clock
67
from .._util import SubclassingDeprecatedIn_v0_15_0
78

@@ -51,32 +52,13 @@ class MockClock(Clock, metaclass=SubclassingDeprecatedIn_v0_15_0):
5152
above) then just set it to zero, and the clock will jump whenever all
5253
tasks are blocked.
5354
54-
.. warning::
55-
56-
If you're using :func:`wait_all_tasks_blocked` and
57-
:attr:`autojump_threshold` together, then you have to be
58-
careful. Setting :attr:`autojump_threshold` acts like a background
59-
task calling::
60-
61-
while True:
62-
await wait_all_tasks_blocked(
63-
cushion=clock.autojump_threshold, tiebreaker=float("inf"))
64-
65-
This means that if you call :func:`wait_all_tasks_blocked` with a
66-
cushion *larger* than your autojump threshold, then your call to
67-
:func:`wait_all_tasks_blocked` will never return, because the
68-
autojump task will keep waking up before your task does, and each
69-
time it does it'll reset your task's timer. However, if your cushion
70-
and the autojump threshold are the *same*, then the autojump's
71-
tiebreaker will prevent them from interfering (unless you also set
72-
your tiebreaker to infinity for some reason. Don't do that). As an
73-
important special case: this means that if you set an autojump
74-
threshold of zero and use :func:`wait_all_tasks_blocked` with the
75-
default zero cushion, then everything will work fine.
76-
77-
**Summary**: you should set :attr:`autojump_threshold` to be at
78-
least as large as the largest cushion you plan to pass to
79-
:func:`wait_all_tasks_blocked`.
55+
.. note:: If you use ``autojump_threshold`` and
56+
`wait_all_tasks_blocked` at the same time, then you might wonder how
57+
they interact, since they both cause things to happen after the run
58+
loop goes idle for some time. The answer is:
59+
`wait_all_tasks_blocked` takes priority. If there's a task blocked
60+
in `wait_all_tasks_blocked`, then the autojump feature treats that
61+
as active task and does *not* jump the clock.
8062
8163
"""
8264

@@ -88,8 +70,6 @@ def __init__(self, rate=0.0, autojump_threshold=inf):
8870
self._virtual_base = 0.0
8971
self._rate = 0.0
9072
self._autojump_threshold = 0.0
91-
self._autojump_task = None
92-
self._autojump_cancel_scope = None
9373
# kept as an attribute so that our tests can monkeypatch it
9474
self._real_clock = time.perf_counter
9575

@@ -124,56 +104,39 @@ def autojump_threshold(self):
124104
@autojump_threshold.setter
125105
def autojump_threshold(self, new_autojump_threshold):
126106
self._autojump_threshold = float(new_autojump_threshold)
127-
self._maybe_spawn_autojump_task()
128-
if self._autojump_cancel_scope is not None:
129-
# Task is running and currently blocked on the old setting, wake
130-
# it up so it picks up the new setting
131-
self._autojump_cancel_scope.cancel()
132-
133-
async def _autojumper(self):
134-
while True:
135-
with _core.CancelScope() as cancel_scope:
136-
self._autojump_cancel_scope = cancel_scope
137-
try:
138-
# If the autojump_threshold changes, then the setter does
139-
# cancel_scope.cancel(), which causes the next line here
140-
# to raise Cancelled, which is absorbed by the cancel
141-
# scope above, and effectively just causes us to skip back
142-
# to the start the loop, like a 'continue'.
143-
await _core.wait_all_tasks_blocked(self._autojump_threshold, inf)
144-
statistics = _core.current_statistics()
145-
jump = statistics.seconds_to_next_deadline
146-
if 0 < jump < inf:
147-
self.jump(jump)
148-
else:
149-
# There are no deadlines, nothing is going to happen
150-
# until some actual I/O arrives (or maybe another
151-
# wait_all_tasks_blocked task wakes up). That's fine,
152-
# but if our threshold is zero then this will become a
153-
# busy-wait -- so insert a small-but-non-zero _sleep to
154-
# avoid that.
155-
if self._autojump_threshold == 0:
156-
await _core.wait_all_tasks_blocked(0.01)
157-
finally:
158-
self._autojump_cancel_scope = None
159-
160-
def _maybe_spawn_autojump_task(self):
161-
if self._autojump_threshold < inf and self._autojump_task is None:
162-
try:
163-
clock = _core.current_clock()
164-
except RuntimeError:
165-
return
166-
if clock is self:
167-
self._autojump_task = _core.spawn_system_task(self._autojumper)
107+
self._try_resync_autojump_threshold()
108+
109+
# runner.clock_autojump_threshold is an internal API that isn't easily
110+
# usable by custom third-party Clock objects. If you need access to this
111+
# functionality, let us know, and we'll figure out how to make a public
112+
# API. Discussion:
113+
#
114+
# https://github.com/python-trio/trio/issues/1587
115+
def _try_resync_autojump_threshold(self):
116+
try:
117+
runner = GLOBAL_RUN_CONTEXT.runner
118+
if runner.is_guest:
119+
runner.force_guest_tick_asap()
120+
except AttributeError:
121+
pass
122+
else:
123+
runner.clock_autojump_threshold = self._autojump_threshold
124+
125+
# Invoked by the run loop when runner.clock_autojump_threshold is
126+
# exceeded.
127+
def _autojump(self):
128+
statistics = _core.current_statistics()
129+
jump = statistics.seconds_to_next_deadline
130+
if 0 < jump < inf:
131+
self.jump(jump)
168132

169133
def _real_to_virtual(self, real):
170134
real_offset = real - self._real_base
171135
virtual_offset = self._rate * real_offset
172136
return self._virtual_base + virtual_offset
173137

174138
def start_clock(self):
175-
token = _core.current_trio_token()
176-
token.run_sync_soon(self._maybe_spawn_autojump_task)
139+
self._try_resync_autojump_threshold()
177140

178141
def current_time(self):
179142
return self._real_to_virtual(self._real_clock())

‎trio/_core/_run.py‎

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import collections.abc
1313
from contextlib import contextmanager, closing
1414
import warnings
15+
import enum
1516

1617
from contextvars import copy_context
1718
from math import inf
@@ -112,6 +113,11 @@ def deadline_to_sleep_time(self, deadline):
112113
return deadline - self.current_time()
113114

114115

116+
class IdlePrimedTypes(enum.Enum):
117+
WAITING_FOR_IDLE = 1
118+
AUTOJUMP_CLOCK = 2
119+
120+
115121
################################################################
116122
# CancelScope and friends
117123
################################################################
@@ -1209,6 +1215,9 @@ class Runner:
12091215
entry_queue = attr.ib(factory=EntryQueue)
12101216
trio_token = attr.ib(default=None)
12111217

1218+
# If everything goes idle for this long, we call clock._autojump()
1219+
clock_autojump_threshold = attr.ib(default=inf)
1220+
12121221
# Guest mode stuff
12131222
is_guest = attr.ib(default=False)
12141223
guest_tick_scheduled = attr.ib(default=False)
@@ -1999,12 +2008,18 @@ def unrolled_run(runner, async_fn, args, host_uses_signal_set_wakeup_fd=False):
19992008
timeout = _MAX_TIMEOUT
20002009
timeout = min(max(0, timeout), _MAX_TIMEOUT)
20012010

2002-
idle_primed = False
2011+
idle_primed = None
20032012
if runner.waiting_for_idle:
20042013
cushion, tiebreaker, _ = runner.waiting_for_idle.keys()[0]
20052014
if cushion < timeout:
20062015
timeout = cushion
2007-
idle_primed = True
2016+
idle_primed = IdlePrimedTypes.WAITING_FOR_IDLE
2017+
# We use 'elif' here because if there are tasks in
2018+
# wait_all_tasks_blocked, then those tasks will wake up without
2019+
# jumping the clock, so we don't need to autojump.
2020+
elif runner.clock_autojump_threshold < timeout:
2021+
timeout = runner.clock_autojump_threshold
2022+
idle_primed = IdlePrimedTypes.AUTOJUMP_CLOCK
20082023

20092024
if runner.instruments:
20102025
runner.instrument("before_io_wait", timeout)
@@ -2024,18 +2039,18 @@ def unrolled_run(runner, async_fn, args, host_uses_signal_set_wakeup_fd=False):
20242039
if deadline <= now:
20252040
# This removes the given scope from runner.deadlines:
20262041
cancel_scope.cancel()
2027-
idle_primed = False
2042+
idle_primed = None
20282043
else:
20292044
break
20302045

2031-
# idle_primed=True means: if the IO wait hit the timeout, and still
2032-
# nothing is happening, then we should start waking up
2033-
# wait_all_tasks_blocked tasks. But there are some subtleties in
2034-
# defining "nothing is happening".
2046+
# idle_primed != None means: if the IO wait hit the timeout, and
2047+
# still nothing is happening, then we should start waking up
2048+
# wait_all_tasks_blocked tasks or autojump the clock. But there
2049+
# are some subtleties in defining "nothing is happening".
20352050
#
20362051
# 'not runner.runq' means that no tasks are currently runnable.
20372052
# 'not events' means that the last IO wait call hit its full
2038-
# timeout. These are very similar, and if idle_primed=True and
2053+
# timeout. These are very similar, and if idle_primed != None and
20392054
# we're running in regular mode then they always go together. But,
20402055
# in *guest* mode, they can happen independently, even when
20412056
# idle_primed=True:
@@ -2049,14 +2064,18 @@ def unrolled_run(runner, async_fn, args, host_uses_signal_set_wakeup_fd=False):
20492064
# before we got here.
20502065
#
20512066
# So we need to check both.
2052-
if idle_primed and not runner.runq and not events:
2053-
while runner.waiting_for_idle:
2054-
key, task = runner.waiting_for_idle.peekitem(0)
2055-
if key[:2] == (cushion, tiebreaker):
2056-
del runner.waiting_for_idle[key]
2057-
runner.reschedule(task)
2058-
else:
2059-
break
2067+
if idle_primed is not None and not runner.runq and not events:
2068+
if idle_primed is IdlePrimedTypes.WAITING_FOR_IDLE:
2069+
while runner.waiting_for_idle:
2070+
key, task = runner.waiting_for_idle.peekitem(0)
2071+
if key[:2] == (cushion, tiebreaker):
2072+
del runner.waiting_for_idle[key]
2073+
runner.reschedule(task)
2074+
else:
2075+
break
2076+
else:
2077+
assert idle_primed is IdlePrimedTypes.AUTOJUMP_CLOCK
2078+
runner.clock._autojump()
20602079

20612080
# Process all runnable tasks, but only the ones that are already
20622081
# runnable now. Anything that becomes runnable during this cycle

‎trio/_core/tests/test_guest_mode.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import signal
88
import socket
99
import threading
10+
import time
1011

1112
import trio
1213
import trio.testing
@@ -473,3 +474,26 @@ async def trio_main_raising(in_host):
473474
assert excinfo.value.__context__ is final_exc
474475

475476
assert signal.getsignal(signal.SIGINT) is signal.default_int_handler
477+
478+
479+
def test_guest_mode_autojump_clock_threshold_changing():
480+
# This is super obscure and probably no-one will ever notice, but
481+
# technically mutating the MockClock.autojump_threshold from the host
482+
# should wake up the guest, so let's test it.
483+
484+
clock = trio.testing.MockClock()
485+
486+
DURATION = 120
487+
488+
async def trio_main(in_host):
489+
assert trio.current_time() == 0
490+
in_host(lambda: setattr(clock, "autojump_threshold", 0))
491+
await trio.sleep(DURATION)
492+
assert trio.current_time() == DURATION
493+
494+
start = time.monotonic()
495+
trivial_guest_run(trio_main, clock=clock)
496+
end = time.monotonic()
497+
# Should be basically instantaneous, but we'll leave a generous buffer to
498+
# account for any CI weirdness
499+
assert end - start < DURATION / 2

0 commit comments

Comments
 (0)