feature: concurrency action by Alizter · Pull Request #6933 · ocaml/dune

Alizter · 2023-01-25T18:19:30Z

See the discussion: [RFC] concurrent progn action #6937

There are a few cases in which we need an action like (progn ) that runs the child actions concurrently. Take for instance:

Running inline tests in parallel Run inline tests in parallel #1516
Diffing multiple files at once without failing early. (See concurrent-multi-diff.t/run.t for an example).
Or more generally, running multiple actions at once without failing early. (See concurrent.t for an example).

We add a new action called (concurrent ) which acts just like (progn ) with the difference that it's child actions are run concurrently.

Documentation
Changelog

snowleopard

The current implementation doesn't interact with -j N setting in the right way. Surely, every action under conc should take a slot in the global job throttle.

snowleopard · 2023-01-26T08:46:12Z

(Less importantly, the pair of names progn and conc is pretty much impossible to guess. I think we should go for something like sequence and parallel, and deprecate progn.)

rgrinberg · 2023-01-26T18:03:50Z

The current implementation doesn't interact with -j N setting in the right way. Surely, every action under conc should take a slot in the global job throttle.

Are you sure? I think we only throttle spawning processes not actions.

(Less importantly, the pair of names progn and conc is pretty much impossible to guess. I think we should go for something like sequence and parallel, and deprecate progn.)

At least progn is a standard name from lisp. I have no idea what's a "conc".

snowleopard · 2023-01-27T05:46:24Z

Are you sure? I think we only throttle spawning processes not actions.

Well, and the PR description says this feature is going to be used for running tests in parallel, which are external processes.

I don't see much point in a combinator like conc if it doesn't interact with the -j setting, because that will become a footgun for rendering your machine unusable when running too much stuff in parallel without a throttle.

Basically, as soon as you start caring about action parallelism, you should start caring about the -j throttle.

snowleopard · 2023-01-27T05:55:28Z

OK, I might have been confused about the purpose of conc all along, as I've written in my comment here.

rgrinberg · 2023-01-27T06:53:30Z

I don't see much point in a combinator like conc if it doesn't interact with the -j setting, because that will become a footgun for rendering your machine unusable when running too much stuff in parallel without a throttle.

Note that it does interact with the throttle exactly the same as everything else in dune. For example, (conc (run ..) (run ..)) will respect -j as expected according to this implementation because the underlying calls to Process.run all take the throttle into account. It is only other actions that will run "concurrently". But in reality, there will be no concurrent execution of any other actions because dune just executes them all sequentially. But at least we'll be able to avoid the fail-on-first-error behavior. So (conc (diff ..) (diff ..)) will try both diffs even if one fails.

snowleopard · 2023-01-27T11:59:46Z

@rgrinberg Huh, sorry, I guess I had a different mental model about where throttling happens. That this does interact well with run is great! I'll re-review the PR some time next week.

test/blackbox-tests/test-cases/dune-cache/trim.t

Alizter · 2023-02-11T19:49:58Z

There is a little subtlety that I feel deserves some documentation. The actions are not maximally concurrent as one might expect from the name. For example, concurrent A B will deadlock/timeout if A waits for a message from B and Dune is running with -j 1.

Do you have an example? I don't see how you can write an action that waits on the output of another.

snowleopard · 2023-02-12T04:02:47Z

There is a little subtlety that I feel deserves some documentation. The actions are not maximally concurrent as one might expect from the name. For example, concurrent A B will deadlock/timeout if A waits for a message from B and Dune is running with -j 1.

Do you have an example? I don't see how you can write an action that waits on the output of another.

Sure, imagine writing an ad-hoc test that involves a client app talking to a server app via TCP. One might reach out for the concurrent action and write something like this:

(rule
 (deps (client.exe server.exe))
 (action
  (concurrent
   (with-stdout-to client_stdout (run client.exe))
   (with-stdout-to server_stdout (run server.exe)))))

... and it will work just fine, as long as you're not running with -j 1. Maybe in future, we could extend concurrent to take an argument specifying how many concurrent jobs are necessary for the rule to work, and Dune would fail if -j N is too low to be able to satisfy the requirement (or maybe override the parallelism limit just for this rule?).

snowleopard · 2023-02-12T04:08:44Z

Just to add: introducing the max_concurrent_jobs field to concurrent may also be useful for cases where you actually want no parallelism, e.g. where all you care about is just the right error-collection behaviour (as I explained in my earlier comment). In those cases, you'll be able to just say (max_concurrent_jobs 1).

But it seems better to do that in a separate PR.

Alizter · 2023-02-12T16:47:42Z

@snowleopard I'm not sure something about the number of allowed jobs really belongs in the engine. In that case I would say a field should be added to the rule stanza allowing it to override the job limit in the scheduler. In any case, I can add a note in the documentation about this use case, and a warning about the number of jobs.

test/blackbox-tests/test-cases/actions/concurrent.t

snowleopard · 2023-02-21T03:33:24Z

@snowleopard I'm not sure something about the number of allowed jobs really belongs in the engine. In that case I would say a field should be added to the rule stanza allowing it to override the job limit in the scheduler. In any case, I can add a note in the documentation about this use case, and a warning about the number of jobs.

Yes, please a note to the docs that the actual concurrency is limited by -j number.

Thanks for adding the test!

We add a (concurrent ) action which acts like (progn ) the difference being the actions contained within can be executed concurrently by Dune.  Signed-off-by: Ali Caglayan <alizter@gmail.com>

doc/concepts.rst

Co-authored-by: Andrey Mokhov <andrey.mokhov@gmail.com> Signed-off-by: Ali Caglayan <alizter@gmail.com>

Alizter · 2023-02-27T10:56:37Z

@snowleopard Thanks! That reads much better.

doc/concepts.rst

Signed-off-by: Andrey Mokhov <andrey.mokhov@gmail.com>

snowleopard

Looks good, thanks! (I made one last little tweak to the doc, hope it's OK.)

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

@rgrinberg

CHANGES: - When a rule's action is interrupted, delete any leftover directory targets. This is consistent with how we treat file targets. (@rgrinberg, 7564) - Fix plugin loading with findlib. The functionality was broken in 3.7.0. (ocaml/dune#7556, @anmonteiro) - Introduce a `public_headers` field on libraries. This field is like `install_c_headers`, but it allows to choose the extension and choose the paths for the installed headers. (ocaml/dune#7512, @rgrinberg) - Load the host context `findlib.conf` when cross-compiling (ocaml/dune#7428, fixes ocaml/dune#1701, @rgrinberg, @anmonteiro) - Resolve `ppx_runtime_libraries` in the target context when cross compiling (ocaml/dune#7450, fixes ocaml/dune#2794, @anmonteiro) - Use `$PKG_CONFIG`, when set, to find the `pkg-config` binary (ocaml/dune#7469, fixes ocaml/dune#2572, @anmonteiro) - Preliminary support for Coq compiled intefaces (`.vos` files) enabled via `(mode vos)` in `coq.theory` stanzas. This can be used in combination with `dune coq top` to obtain fast re-building of dependencies (with no checking of proofs) prior to stepping into a file. (ocaml/dune#7406, @rlepigre) - Fix dune crashing on MacOS in watch mode whenever `$PATH` contains `$PWD` (ocaml/dune#7441, fixes ocaml/dune#6907, @rgrinberg) - Fix `dune install` when cross compiling (ocaml/dune#7410, fixes ocaml/dune#6191, @anmonteiro, @rizo) - Find `pps` dependencies in the host context when cross-compiling, (ocaml/dune#7410, fixes ocaml/dune#4156, @anmonteiro) - Dune in watch mode no longer builds concurrent rules in serial (ocaml/dune#7395 @rgrinberg, @jchavarri) - `dune coq top` now correctly respects the project root when called from a subdirectory. However, absolute filenames passed to `dune coq top` are no longer supported (due to being buggy) (ocaml/dune#7357, fixes ocaml/dune#7344, @rlepigre and @Alizter) - Added a `--no-build` option to `dune coq top` for avoiding rebuilds (ocaml/dune#7380, fixes ocaml/dune#7355, @Alizter) - RPC: Ignore SIGPIPE when clients suddenly disconnect (ocaml/dune#7299, ocaml/dune#7319, fixes ocaml/dune#6879, @rgrinberg) - Always clean up the UI on exit. (ocaml/dune#7271, fixes ocaml/dune#7142 @rgrinberg) - Bootstrap: remove reliance on shell. Previously, we'd use the shell to get the number of processors. (ocaml/dune#7274, @rgrinberg) - Bootstrap: correctly detect the number of processors by allowing `nproc` to be looked up in `$PATH` (ocaml/dune#7272, @Alizter) - Speed up file copying on macos by using `clonefile` when available (@rgrinberg, ocaml/dune#7210) - Adds support for loading plugins in toplevels (ocaml/dune#6082, fixes ocaml/dune#6081, @ivg, @richardlford) - Support commands that output 8-bit and 24-bit colors in the terminal (ocaml/dune#7188, @Alizter) - Speed up rule generation for libraries and executables with many modules (ocaml/dune#7187, @jchavarri) - Add `--watch-exclusions` to Dune build options (ocaml/dune#7216, @jonahbeckford) - Do not re-render UI on every frame if the UI doesn't change (ocaml/dune#7186, fix ocaml/dune#7184, @rgrinberg) - Make coq_db creation in scope lazy (@ejgallego, ocaml/dune#7133) - Non-user proccesses such as version control or config checking are now run silently. (ocaml/dune#6994, fixes ocaml/dune#4066, @Alizter) - Add the `--display-separate-messages` flag to separate the error messages produced by commands with a blank line. (ocaml/dune#6823, fixes ocaml/dune#6158, @esope) - Accept the Ordered Set Language for the `modes` field in `library` stanzas (ocaml/dune#6611, @anmonteiro). - dune install now respects --display quiet mode (ocaml/dune#7116, fixes ocaml/dune#4573, fixes ocaml/dune#7106, @Alizter) - Stub shared libraries (dllXXX_stubs.so) in Dune-installed libraries could not be used as dependencies of libraries in the workspace (eg when compiling to bytecode and/or Javascript). This is now fixed. (ocaml/dune#7151, @nojb) - Allow the main module of a library with `(stdlib ...)` to depend on other libraries (ocaml/dune#7154, @anmonteiro). - Bytecode executables built for JSOO are linked with `-noautolink` and no longer depend on the shared stubs of their dependent libraries (ocaml/dune#7156, @nojb) - Added a new user action `(concurrent )` which is like `(progn )` but runs the actions concurrently. (ocaml/dune#6933, @Alizter) - Allow `(stdlib ...)` to be used with `(wrapped false)` in library stanzas (ocaml/dune#7139, @anmonteiro). - Allow parallel execution of inline tests partitions (ocaml/dune#7012, @hhugo) - Support `(link_flags ...)` in `(cinaps ...)` stanza. (ocaml/dune#7423, fixes ocaml/dune#7416, @nojb) - Allow `(package ...)` in any position within `(rule ...)` stanza (ocaml/dune#7445, @Leonidas-from-XIV) - Always include `opam` files in the generated `.install` file. Previously, it would not be included whenever `(generate_opam_files true)` was set and the `.install` file wasn't yet generated. (ocaml/dune#7547, @rgrinberg)

@rleshchinskiy

CHANGES: - Fix string quoting in the json file written by `--trace-file` (ocaml/dune#7773, @rleshchinskiy) - Read `pkg-config` arguments from the `PKG_CONFIG_ARGN` environment variable (ocaml/dune#1492, ocaml/dune#7734, @anmonteiro) - Correctly set `MANPATH` in `dune exec`. Previously, we would use the `bin/` directory of the context. (ocaml/dune#7655, @rgrinberg) - Allow overriding the `ocaml` binary with findlib configuration (ocaml/dune#7648, @rgrinberg) - merlin: ignore instrumentation settings for preprocessing. (ocaml/dune#7606, fixes ocaml/dune#7465, @Alizter) - When a rule's action is interrupted, delete any leftover directory targets. This is consistent with how we treat file targets. (ocaml/dune#7564, @rgrinberg) - Fix plugin loading with findlib. The functionality was broken in 3.7.0. (ocaml/dune#7556, @anmonteiro) - Introduce a `public_headers` field on libraries. This field is like `install_c_headers`, but it allows to choose the extension and choose the paths for the installed headers. (ocaml/dune#7512, @rgrinberg) - Load the host context `findlib.conf` when cross-compiling (ocaml/dune#7428, fixes ocaml/dune#1701, @rgrinberg, @anmonteiro) - Add a `coqdoc_flags` field to the `coq.theory` stanza allowing the user to pass extra arguments to `coqdoc`. (ocaml/dune#7676, fixes ocaml/dune#7954 @Alizter) - Resolve `ppx_runtime_libraries` in the target context when cross compiling (ocaml/dune#7450, fixes ocaml/dune#2794, @anmonteiro) - Use `$PKG_CONFIG`, when set, to find the `pkg-config` binary (ocaml/dune#7469, fixes ocaml/dune#2572, @anmonteiro) - Modules that were declared in `(modules_without_implementation)`, `(private_modules)` or `(virtual_modules)` but not declared in `(modules)` will cause Dune to emit a warning which will become an error in 3.9. (ocaml/dune#7608, fixes ocaml/dune#7026, @Alizter) - Preliminary support for Coq compiled intefaces (`.vos` files) enabled via `(mode vos)` in `coq.theory` stanzas. This can be used in combination with `dune coq top` to obtain fast re-building of dependencies (with no checking of proofs) prior to stepping into a file. (ocaml/dune#7406, @rlepigre) - Fix dune crashing on MacOS in watch mode whenever `$PATH` contains `$PWD` (ocaml/dune#7441, fixes ocaml/dune#6907, @rgrinberg) - Fix `dune install` when cross compiling (ocaml/dune#7410, fixes ocaml/dune#6191, @anmonteiro, @rizo) - Find `pps` dependencies in the host context when cross-compiling, (ocaml/dune#7410, fixes ocaml/dune#4156, @anmonteiro) - Dune in watch mode no longer builds concurrent rules in serial (ocaml/dune#7395 @rgrinberg, @jchavarri) - Dune can now detect Coq theories from outside the workspace. This allows for composition with installed theories (not necessarily installed with Dune). (ocaml/dune#7047, @Alizter, @ejgallego) - `dune coq top` now correctly respects the project root when called from a subdirectory. However, absolute filenames passed to `dune coq top` are no longer supported (due to being buggy) (ocaml/dune#7357, fixes ocaml/dune#7344, @rlepigre and @Alizter) - Added a `--no-build` option to `dune coq top` for avoiding rebuilds (ocaml/dune#7380, fixes ocaml/dune#7355, @Alizter) - RPC: Ignore SIGPIPE when clients suddenly disconnect (ocaml/dune#7299, ocaml/dune#7319, fixes ocaml/dune#6879, @rgrinberg) - Always clean up the UI on exit. (ocaml/dune#7271, fixes ocaml/dune#7142 @rgrinberg) - Bootstrap: remove reliance on shell. Previously, we'd use the shell to get the number of processors. (ocaml/dune#7274, @rgrinberg) - Bootstrap: correctly detect the number of processors by allowing `nproc` to be looked up in `$PATH` (ocaml/dune#7272, @Alizter) - Speed up file copying on macos by using `clonefile` when available (@rgrinberg, ocaml/dune#7210) - Adds support for loading plugins in toplevels (ocaml/dune#6082, fixes ocaml/dune#6081, @ivg, @richardlford) - Support commands that output 8-bit and 24-bit colors in the terminal (ocaml/dune#7188, @Alizter) - Speed up rule generation for libraries and executables with many modules (ocaml/dune#7187, @jchavarri) - Add `--watch-exclusions` to Dune build options (ocaml/dune#7216, @jonahbeckford) - Do not re-render UI on every frame if the UI doesn't change (ocaml/dune#7186, fix ocaml/dune#7184, @rgrinberg) - Make coq_db creation in scope lazy (@ejgallego, ocaml/dune#7133) - Non-user proccesses such as version control or config checking are now run silently. (ocaml/dune#6994, fixes ocaml/dune#4066, @Alizter) - Add the `--display-separate-messages` flag to separate the error messages produced by commands with a blank line. (ocaml/dune#6823, fixes ocaml/dune#6158, @esope) - Accept the Ordered Set Language for the `modes` field in `library` stanzas (ocaml/dune#6611, @anmonteiro). - dune install now respects --display quiet mode (ocaml/dune#7116, fixes ocaml/dune#4573, fixes ocaml/dune#7106, @Alizter) - Stub shared libraries (dllXXX_stubs.so) in Dune-installed libraries could not be used as dependencies of libraries in the workspace (eg when compiling to bytecode and/or Javascript). This is now fixed. (ocaml/dune#7151, @nojb) - Allow the main module of a library with `(stdlib ...)` to depend on other libraries (ocaml/dune#7154, @anmonteiro). - Bytecode executables built for JSOO are linked with `-noautolink` and no longer depend on the shared stubs of their dependent libraries (ocaml/dune#7156, @nojb) - Added a new user action `(concurrent )` which is like `(progn )` but runs the actions concurrently. (ocaml/dune#6933, @Alizter) - Allow `(stdlib ...)` to be used with `(wrapped false)` in library stanzas (ocaml/dune#7139, @anmonteiro). - Allow parallel execution of inline tests partitions (ocaml/dune#7012, @hhugo) - Support `(link_flags ...)` in `(cinaps ...)` stanza. (ocaml/dune#7423, fixes ocaml/dune#7416, @nojb) - Allow `(package ...)` in any position within `(rule ...)` stanza (ocaml/dune#7445, @Leonidas-from-XIV) - Always include `opam` files in the generated `.install` file. Previously, it would not be included whenever `(generate_opam_files true)` was set and the `.install` file wasn't yet generated. (ocaml/dune#7547, @rgrinberg) - Fix regression where Merlin was unable to handle filenames with uppercase letters under Windows. (ocaml/dune#7577, @nojb) - On nix+macos, pass `-f` to the codesign hook to avoid errors when the binary is already signed (ocaml/dune#7183, fixes ocaml/dune#6265, @greedy) - Fix bug where RPC clients built with dune-rpc-lwt would crash when closing their connection to the server (ocaml/dune#7581, @gridbugs) - Introduce mdx stanza 0.4 requiring mdx >= 2.3.0 which updates the default list of files to include `*.mld` files (ocaml/dune#7582, @Leonidas-from-XIV) - Fix RPC server on Windows (used for OCaml-LSP). (ocaml/dune#7666, @nojb) - Coq language versions less 0.8 are deprecated, and will be removed in an upcoming Dune version. All users are required to migrate to `(coq lang 0.8)` which provides the right semantics for theories that have been globally installed, such as those coming from opam (@ejgallego, @Alizter) - Bump minimum version of the dune language for the melange syntax extension from 3.7 to 3.8 (ocaml/dune#7665, @jchavarri)

Alizter force-pushed the ps/rr/feature__concurrency_action branch 3 times, most recently from 9fb4807 to a392222 Compare January 25, 2023 19:30

Alizter requested review from rgrinberg and snowleopard January 25, 2023 19:32

Alizter force-pushed the ps/rr/feature__concurrency_action branch 5 times, most recently from ebcb8b8 to ea9d993 Compare January 25, 2023 20:43

Alizter requested a review from christinerose as a code owner January 25, 2023 20:43

Alizter force-pushed the ps/rr/feature__concurrency_action branch 5 times, most recently from 26da924 to 12ac66f Compare January 25, 2023 20:52

Alizter mentioned this pull request Jan 26, 2023

[RFC] concurrent progn action #6937

Closed

snowleopard requested changes Jan 26, 2023

View reviewed changes

This was referenced Jan 26, 2023

Diffing of inline tests is not concurrent #6942

Closed

Run inline tests in parallel #1516

Closed

rgrinberg reviewed Jan 27, 2023

View reviewed changes

test/blackbox-tests/test-cases/dune-cache/trim.t Outdated Show resolved Hide resolved

Alizter force-pushed the ps/rr/feature__concurrency_action branch from 12ac66f to 90560f2 Compare February 5, 2023 23:39

Alizter requested review from rgrinberg and snowleopard February 5, 2023 23:51

rgrinberg linked an issue Feb 12, 2023 that may be closed by this pull request

diffing directories #3567

Closed

Alizter added this to the 3.8.0 milestone Feb 12, 2023

Alizter force-pushed the ps/rr/feature__concurrency_action branch from 1ded117 to 6f5d7da Compare February 19, 2023 14:28

snowleopard reviewed Feb 21, 2023

View reviewed changes

test/blackbox-tests/test-cases/actions/concurrent.t Show resolved Hide resolved

snowleopard reviewed Feb 21, 2023

View reviewed changes

test/blackbox-tests/test-cases/actions/concurrent.t Show resolved Hide resolved

snowleopard reviewed Feb 21, 2023

View reviewed changes

test/blackbox-tests/test-cases/actions/concurrent.t Show resolved Hide resolved

Alizter force-pushed the ps/rr/feature__concurrency_action branch from 85deed2 to 489e8eb Compare February 26, 2023 17:52

Alizter requested a review from snowleopard February 26, 2023 17:53

Alizter force-pushed the ps/rr/feature__concurrency_action branch from 489e8eb to 829a3b6 Compare February 26, 2023 18:56

feature: concurrency action

d978ea0

We add a (concurrent ) action which acts like (progn ) the difference being the actions contained within can be executed concurrently by Dune.  Signed-off-by: Ali Caglayan <alizter@gmail.com>

Alizter force-pushed the ps/rr/feature__concurrency_action branch from 829a3b6 to d978ea0 Compare February 27, 2023 00:00

snowleopard reviewed Feb 27, 2023

View reviewed changes

doc/concepts.rst Outdated Show resolved Hide resolved

Update doc/concepts.rst

5ad2d85

Co-authored-by: Andrey Mokhov <andrey.mokhov@gmail.com> Signed-off-by: Ali Caglayan <alizter@gmail.com>

hhugo approved these changes Feb 27, 2023

View reviewed changes

snowleopard reviewed Feb 27, 2023

View reviewed changes

doc/concepts.rst Outdated Show resolved Hide resolved

Update doc/concepts.rst

b5e9974

Signed-off-by: Andrey Mokhov <andrey.mokhov@gmail.com>

snowleopard approved these changes Feb 27, 2023

View reviewed changes

_

7d5e967

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg merged commit ea424da into ocaml:main Feb 27, 2023

emillon mentioned this pull request Apr 18, 2023

[new release] dune (15 packages) (3.8.0~alpha1) ocaml/opam-repository#23683

Closed

emillon mentioned this pull request May 23, 2023

[new release] dune (15 packages) (3.8.0) ocaml/opam-repository#23814

Merged

rgrinberg removed a link to an issue Mar 22, 2026

diffing directories #3567

Closed

Conversation

Alizter commented Jan 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snowleopard left a comment

Choose a reason for hiding this comment

Uh oh!

snowleopard commented Jan 26, 2023

Uh oh!

rgrinberg commented Jan 26, 2023

Uh oh!

snowleopard commented Jan 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snowleopard commented Jan 27, 2023

Uh oh!

rgrinberg commented Jan 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snowleopard commented Jan 27, 2023

Uh oh!

Uh oh!

Alizter commented Feb 11, 2023

Uh oh!

snowleopard commented Feb 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snowleopard commented Feb 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Alizter commented Feb 12, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snowleopard commented Feb 21, 2023

Uh oh!

Uh oh!

Alizter commented Feb 27, 2023

Uh oh!

Uh oh!

snowleopard left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Alizter commented Jan 25, 2023 •

edited

Loading

snowleopard commented Jan 27, 2023 •

edited

Loading

rgrinberg commented Jan 27, 2023 •

edited

Loading

snowleopard commented Feb 12, 2023 •

edited

Loading

snowleopard commented Feb 12, 2023 •

edited

Loading