Deterministic scheduling via Hypothesis

[Zac-HD]

Related to HypothesisWorks/hypothesis#1709 and python-trio/trio#239.

And now that HypothesisWorks/hypothesis#1741 has been released, this actually works and is ready to merge 🎉

[codecov]

Codecov Report

Merging #73 into master will increase coverage by 0.22%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
+ Coverage   99.56%   99.78%   +0.22%     
==========================================
  Files          19       19              
  Lines         461      476      +15     
  Branches       43       41       -2     
==========================================
+ Hits          459      475      +16     
+ Misses          2        1       -1

Impacted Files	Coverage Δ
pytest_trio/_tests/test_fixture_ordering.py	100% <ø> (ø)	⬆️
pytest_trio/_tests/test_hypothesis_interaction.py	100% <100%> (ø)	⬆️
pytest_trio/plugin.py	99.47% <100%> (+0.53%)	⬆️

[njsmith]

The other Trio part is python-trio/trio#840 which is merged and should be released as part of 0.10.0 any moment now.

[Zac-HD]

Ping @njsmith, we've just released the Hypothesis end of this 😃

[Zac-HD]

CC @pquentin, @touilleMan ?

[pquentin]

Sorry, I'm not familiar with Hypothesis. Is there a way to test manually that this integration between pytest-trio and Hypothesis is now working? eg. a test somewhere that is only deterministic after those register_random PRs. Thanks!

[Zac-HD]

Is there a way to test manually that this integration between pytest-trio and Hypothesis is now working? eg. a test somewhere that is only deterministic after those register_random PRs.

Hmm, I don't actually know the Trio internals well enough to say! We would need a test that depends on the ordering of a batch of tasks. Alternatively it would be easy but not that meaningful to test that the random instance in question is in fact seeded by Hypothesis...

[njsmith]

I guess what I'd do is write some code like:

async def scheduler_trace():
    trace = []
    async def tracer(name):
        for i in range(10):
            trace.append([(name, i)])
            await trio.sleep(0)
    async with trio.open_nursery() as nursery:
        for i in range(5):
            nursery.start_soon(tracer, i)
    return trace

And then run this code 3-4 times and check that trace comes out the same each time.

[pquentin]

I did not manage to make this work with hypothesis, and in fact I have not managed to prove that scheduler_trace is deterministic without shuffling. With 0b8f377e8a95103c7b4e4ca675ccb6ce942e897a checked out and shuffling disabled manually, python -R t.py > 1.txt && python -R t.py > 2.txt && diff -u 1.txt 2.txt often outputs a diff. Here's t.py:

import trio


async def scheduler_trace():
    trace = []

    async def tracer(name):
        for i in range(10):
            trace.append([(name, i)])
            await trio.sleep(0)

    async with trio.open_nursery() as nursery:
        for i in range(5):
            nursery.start_soon(tracer, i)

    print(trace)


async def run5():
    for _ in range(5):
        await scheduler_trace()


if __name__ == "__main__":
    print(trio.__version__)
    trio.run(run5)

[njsmith]

in fact I have not managed to prove that scheduler_trace is deterministic without shuffling

...huh. I have no idea why that would be. Sounds like you were absolutely right to ask for a test here! I probably would have let it slide, shows what I know...

[njsmith]

Oh you know what, I bet it's because when cancel scopes put themselves into the global deadline list, it's sorted by deadline... but if two scopes have the exact same deadline, then id(self) is used as the tie-breaker. (And checkpoint is currently implemented as a cancel scope that expires immediately.) And this doesn't directly explain it, because we currently process all deadlines, and then afterwards schedule all the newly-cancelled tasks. But in the mean time we dump the batch of newly-cancelled tasks into a set, and the set ordering is dependent on both id(task) and the order that the items are inserted. The details of this batching might change as part of python-trio/trio#860, but that won't resolve the issue...

huh, guaranteeing deterministic scheduling in the presence of multiple tasks with identical timeouts seems pretty non-trivial!

[Zac-HD]

It goes deeper - self._tasks is also a set, and thus another source of non-determinism. I suspect that's a fairly deep rabbit-hole, as making CancelScope._tasks a list doesn't change the result.

Unfortunately I don't have the time to take this on at the moment, but I could open an issue to make scheduling deterministic-except-for-shuffling if you agree this would be useful?

[njsmith]

I'm a bit wary of adding complexity to the core loop for this, for maintenance and speed reasons, but the use case does make sense, so I guess it depends on how unobtrusive we can make it.

I guess one idea would be to assign each Task an incrementing counter when it's created, and then at scheduling time sort the run queue by this counter before scrambling?

[pquentin]

OK, trio 0.11.0 is out. Let's cycle this pull request.

[pquentin]

@Zac-HD There's a yapf formatting issue left, but otherwise CI is now passing!

Oh, and it would be nice to require trio >= 0.11.0.

[Zac-HD]

And it's passing 🚀:sparkles:

[touilleMan]

This is reeeally cool, thanks a lot @Zac-HD ;-)

[pquentin]

Thank you for your determinism determination! This PR required work on three projects, and you never gave up. :)

[Zac-HD]

I'm pretty pleased with the end result, but also happy to be finished! It's lovely to work with projects that care so much about both correctness and usability 😄