feature: use clonefile on macos #7210

rgrinberg · 2023-03-02T23:53:45Z

Use mac's [clonefile] instead of manually copying. [clonefile] is like
hardlink but it will copy-on-write when edited.

The attached benchmark before the optimization:

real    0m3.593s
user    0m0.249s
sys     0m2.849s

And after:

real    0m2.044s
user    0m0.083s
sys     0m1.954s

So about a 45% reduction. Not bad. The benchmark also includes how long it took to delete the temp directory at the end (Around 10k additional unlinks), so 45% is likely pessimistic.

anmonteiro · 2023-03-03T03:06:39Z

otherlibs/stdune/src/copyfile_stubs.c

+  const char *from = caml_stat_strdup(String_val(v_from));
+  const char *to = caml_stat_strdup(String_val(v_to));
+  caml_release_runtime_system();
+  copyfile(from, to, state, COPYFILE_CLONE);


based on https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/copyfile.3.html, this can fail with errors. Should we be handling them?

the current copy_file function has exception wrapping around it, and the new one doesn't. Should we also set that up?

Thanks. I added error handling.

I don't think we need protect, because we don't need to worry about cleaning up resources like fd's.

Since you've expressed interested, I've switched you to be the reviewer (I don't think Andrey uses macos anyway).

@nojb can you have a look as well?

nojb

LGTM, modulo a couple of remarks

otherlibs/stdune/src/copyfile_stubs.c

nojb

I believe a couple of pointer casts are not actually needed.

otherlibs/stdune/src/copyfile_stubs.c

nojb

LGTM

otherlibs/stdune/src/copyfile_stubs.c

rgrinberg · 2023-03-06T05:19:37Z

ping @anmonteiro

anmonteiro · 2023-03-06T16:45:59Z

The macOS testsuite isn't very happy.

rgrinberg · 2023-03-06T18:54:05Z

@anmonteiro should be good now.

anmonteiro

Looks good

anmonteiro · 2023-03-07T03:03:09Z

There are still some realpath() errors in the macOS tests btw.

Use mac's [clonefile] instead of manually copying. [clonefile] is like hardlink but it will copy-on-write when edited. Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg · 2023-03-07T16:38:14Z

There are still some realpath() errors in the macOS tests btw.

All fixed now

@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)

rgrinberg requested a review from snowleopard March 2, 2023 23:53

anmonteiro reviewed Mar 3, 2023

View reviewed changes

rgrinberg force-pushed the ps/rr/feature__use_clonefile_on_macos branch 3 times, most recently from 8885a5c to 2a4c1bb Compare March 3, 2023 07:12

rgrinberg removed the request for review from snowleopard March 3, 2023 07:14

nojb approved these changes Mar 3, 2023

View reviewed changes

otherlibs/stdune/src/copyfile_stubs.c Outdated Show resolved Hide resolved

otherlibs/stdune/src/copyfile_stubs.c Outdated Show resolved Hide resolved

rgrinberg force-pushed the ps/rr/feature__use_clonefile_on_macos branch from 2a4c1bb to dab1d5f Compare March 3, 2023 18:03

rgrinberg added this to the 3.8.0 milestone Mar 3, 2023

rgrinberg force-pushed the ps/rr/feature__use_clonefile_on_macos branch 2 times, most recently from e34ef14 to 188eb20 Compare March 3, 2023 18:16

nojb reviewed Mar 3, 2023

View reviewed changes

otherlibs/stdune/src/copyfile_stubs.c Outdated Show resolved Hide resolved

otherlibs/stdune/src/copyfile_stubs.c Outdated Show resolved Hide resolved

rgrinberg force-pushed the ps/rr/feature__use_clonefile_on_macos branch from 188eb20 to 943a9da Compare March 3, 2023 18:30

nojb approved these changes Mar 3, 2023

View reviewed changes

otherlibs/stdune/src/copyfile_stubs.c Outdated Show resolved Hide resolved

rgrinberg force-pushed the ps/rr/feature__use_clonefile_on_macos branch 3 times, most recently from 762d558 to 22c9ba0 Compare March 4, 2023 05:41

rgrinberg force-pushed the ps/rr/feature__use_clonefile_on_macos branch 3 times, most recently from 3b6e771 to 2ac5af9 Compare March 6, 2023 18:29

rgrinberg force-pushed the ps/rr/feature__use_clonefile_on_macos branch from 2ac5af9 to be7dfb6 Compare March 6, 2023 19:14

anmonteiro approved these changes Mar 6, 2023

View reviewed changes

rgrinberg force-pushed the ps/rr/feature__use_clonefile_on_macos branch 3 times, most recently from 0c9df5d to 960853b Compare March 7, 2023 00:10

rgrinberg force-pushed the ps/rr/feature__use_clonefile_on_macos branch from 960853b to 1da9a3d Compare March 7, 2023 05:17

rgrinberg force-pushed the ps/rr/feature__use_clonefile_on_macos branch from 1da9a3d to cb25941 Compare March 7, 2023 15:00

feature: use clonefile on macos

05273df

Use mac's [clonefile] instead of manually copying. [clonefile] is like hardlink but it will copy-on-write when edited. Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg force-pushed the ps/rr/feature__use_clonefile_on_macos branch from cb25941 to 05273df Compare March 7, 2023 15:06

rgrinberg merged commit ac0f503 into main Mar 7, 2023

rgrinberg deleted the ps/rr/feature__use_clonefile_on_macos branch March 7, 2023 16:38

anmonteiro mentioned this pull request Mar 10, 2023

test: demonstrate Io.copy_file difference between macOS / other OS #7270

Merged

anmonteiro mentioned this pull request Mar 31, 2023

fix #6907 #7441

Merged

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

Alizter mentioned this pull request Oct 14, 2023

[macOS] dune 3.11.1 errors (build itself works now, but building some of dependents fails) #8941

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: use clonefile on macos #7210

feature: use clonefile on macos #7210

rgrinberg commented Mar 2, 2023 •

edited

Loading

anmonteiro Mar 3, 2023

rgrinberg Mar 3, 2023

rgrinberg Mar 3, 2023 •

edited

Loading

nojb left a comment

nojb left a comment

nojb left a comment

rgrinberg commented Mar 6, 2023

anmonteiro commented Mar 6, 2023

rgrinberg commented Mar 6, 2023

anmonteiro left a comment

anmonteiro commented Mar 7, 2023

rgrinberg commented Mar 7, 2023

feature: use clonefile on macos #7210

feature: use clonefile on macos #7210

Conversation

rgrinberg commented Mar 2, 2023 • edited Loading

anmonteiro Mar 3, 2023

Choose a reason for hiding this comment

rgrinberg Mar 3, 2023

Choose a reason for hiding this comment

rgrinberg Mar 3, 2023 • edited Loading

Choose a reason for hiding this comment

nojb left a comment

Choose a reason for hiding this comment

nojb left a comment

Choose a reason for hiding this comment

nojb left a comment

Choose a reason for hiding this comment

rgrinberg commented Mar 6, 2023

anmonteiro commented Mar 6, 2023

rgrinberg commented Mar 6, 2023

anmonteiro left a comment

Choose a reason for hiding this comment

anmonteiro commented Mar 7, 2023

rgrinberg commented Mar 7, 2023

rgrinberg commented Mar 2, 2023 •

edited

Loading

rgrinberg Mar 3, 2023 •

edited

Loading