Skip to content

Add STM_domain_eio and Lin_domain_eio libraries to run multicore tests with Eio #581

Open
lyrm wants to merge 6 commits intoocaml-multicore:mainfrom
lyrm:eio
Open

Add STM_domain_eio and Lin_domain_eio libraries to run multicore tests with Eio #581
lyrm wants to merge 6 commits intoocaml-multicore:mainfrom
lyrm:eio

Conversation

@lyrm
Copy link

@lyrm lyrm commented Feb 19, 2026

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
Copy link
Collaborator

jmid commented Feb 22, 2026

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! 😃

@zshipko zshipko mentioned this pull request Feb 26, 2026
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants