Add STM_domain_eio and Lin_domain_eio libraries to run multicore tests with Eio

[lyrm]

This PR adds qcheck-stm-eio.domain and qcheck-lin-eio.domain libraries that provide Eio-based alternatives to the existing Domain-based libraries, spawning parallel tasks via Eio.Domain_manager instead of Domain.spawn directly.

This is needed for libraries built on Eio, such as Irmin and Lavyek.

Below, some technicals decisions that need discussions.

Dune rules to cp files

The shared code between STM_domain/STM_domain_eio and Lin_domain/Lin_domain_eio is factorised into *_common modules to minimise code duplication. To avoid creating a new library for each of these files, I added a dune rule to copy/paste the file. There may be a better way to do that.

New packages

I added two packages (qcheck-stm-eio.domain and qcheck-lin-eio.domain ) instead of adding eio as a dependency for both qcheck-lin and qcheck-stm packages.

TODO

• Add tests for qcheck-stm.domain_eio
• Add tests for qcheck-lin.domain_eio
• Add new *_eio libraries to the CI checks.

[jmid]

Wow thanks for the contribution! 🥳

I've had a quick look and here are a bunch of non-organized impressions:

• This seems like a nice addition! 🎉
• I completely agree on making the Eio packages separate
• I see some white space changes which makes the diff bigger than it needs to be
• For internal testing of STM and Lin there are a bunch of tests in src/neg_tests to confirm that things that are unsafe are indeed caught as expected.
• The Common magic constants are specific to the backend (Threads, Domain, ...). I've arrived at them empirically by just trial and error ("no need to repeat more than necessary", "we should repeat enough to always trigger an expected counterexample to prevent flakiness", etc.) . I would expect that for Eio the constants may be different from the existing ones.
• Basic STM works on ocaml.4.12 too (sans STM-domain). I would expect the Eio packages to require 5.0+
• I'm a bit hesitant about putting an Eio dependency on the test suite in multicoretests as that increases the dependency cone quite a bit. We've previously worked to reduce it, e.g., eliminating a ppx_deriving dependency. I wouldn't like trunk testing to halt because an Eio or one of its dependencies stops working with trunk
• We should probably move the src/neg_tests tests out of (package multicoretests) and onto the relevant library packages (qcheck-stm.sequential, ...) and then do the same thing for the new Eio packages and tests. That's a change going beyond this PR though.
• I'm undecided on the sharing of code between the "parallel back ends". I completely follow you on the general software engineering principle of "don't copy-paste code, share it instead". I also value "make testing code as clear as possible", like we would for unit testing code, which is the reason for me sometimes preferring some duplication in those definitions (different Common magic constants also play in here). After some revisions and additions I don't think the code necessarily is as clear as possible, but at least now you know what I strive for and why.

Thanks again! 😃

[Julow]

Hi!

I'd like to help merging this patch as I also want to use qcheck-stm with Eio (and why not with Gospel later). My patch is here: lyrm#1

For internal testing of STM and Lin there are a bunch of tests in src/neg_tests to confirm that things that are unsafe are indeed caught as expected.

I've added neg_tests variations for the new packages. The runtest time goes from 10 to 16 minutes on my machine, I hope that's ok. I think this should run in CI with the opam workflow.

The Common magic constants are specific to the backend (Threads, Domain, ...). I've arrived at them empirically by just trial and error ("no need to repeat more than necessary", "we should repeat enough to always trigger an expected counterexample to prevent flakiness", etc.) . I would expect that for Eio the constants may be different from the existing ones.

Do you have guidelines on how to test these constants ?

I'm a bit hesitant about putting an Eio dependency on the test suite in multicoretests as that increases the dependency cone quite a bit. We've previously worked to reduce it, e.g., eliminating a ppx_deriving dependency. I wouldn't like trunk testing to halt because an Eio or one of its dependencies stops working with trunk

I removed the dependency to eio in multicoretests.

We should probably move the src/neg_tests tests out of (package multicoretests) and onto the relevant library packages (qcheck-stm.sequential, ...) and then do the same thing for the new Eio packages and tests. That's a change going beyond this PR though.

Happy to work on this in a follow up PR.

I'm undecided on the sharing of code between the "parallel back ends". I completely follow you on the general software engineering principle of "don't copy-paste code, share it instead". I also value "make testing code as clear as possible", like we would for unit testing code, which is the reason for me sometimes preferring some duplication in those definitions (different Common magic constants also play in here). After some revisions and additions I don't think the code necessarily is as clear as possible, but at least now you know what I strive for and why.

This makes the code specific to the Eio packages very small and easy to maintain. I'm not sure the shared code makes the less clear. This code is very specific and not easy to write (I think) so I don't think having twice as many is good.
The information that the code is identical for the eio and non-eio packages is also extremely valuable. Without it, it would loose properties that few people can notice.
Adding more to the functor parameter later (including the constants) should be easy.