dune utop: do not do custom linking #8631

nojb · 2023-09-12T11:22:12Z

In #6894 it is mentioned that dune utop is very slow. @emillon mentions that this is because we link utop in custom mode (probably to avoid having to deal with loading shared C stubs, see this comment). This PR proposes switching to a "normal" bytecode link, instead of custom mode. The only complication is that we need to set CAML_LD_LIBRARY_PATH suitably so that shared C stubs can be found.

Another approach that was discussed in #6894 was to pass the output of dune ocaml top to utop (cc @rgrinberg). However, this has a number of limitations, see eg #8626; also, PPX does not seem to be supported by dune ocaml top. These limitations could/should be lifted, but this PR is meant to be a low-effort fix for the slow loading times in utop that can be done quickly right now.

I did some quick tests and this PR does seem to give a nice speedup, but I don't have any "large" examples to try it on. Maybe @grayswandyr can try it out and report back? Thanks.

Fixes #6894

grayswandyr · 2023-09-12T11:34:05Z

Thanks for addressing this. I will report back. Technically, to get your modified dune, it's enough for me to pull opam-pin this commit, right?

nojb · 2023-09-12T11:36:08Z

Technically, to get your modified dune, it's enough for me to pull opam-pin this commit, right?

I believe so.

rgrinberg · 2023-09-13T12:57:02Z

LGTM. I just did a minor tweak.

Do we have a test to make sure that stubs are still working?

grayswandyr · 2023-09-13T14:39:27Z

I did some quick tests and this PR does seem to give a nice speedup, but I don't have any "large" examples to try it on. Maybe @grayswandyr can try it out and report back? Thanks.

I also observe a good speedup (on rather small codebases). Thanks.

nojb · 2023-09-14T07:07:27Z

LGTM. I just did a minor tweak.

Thanks!

Do we have a test to make sure that stubs are still working?

I added one.

rgrinberg

LGTM. Just needs a doc/changes entry.

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

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

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

rgrinberg · 2023-09-14T07:26:16Z

CHANGES.md

@@ -5,6 +5,9 @@ If you're a contributor, please include your CHANGES entry in a file
 `doc/changes/$PR_NAME.md`. At release time, it will be incoporated into the
 changelog properly.

+- `dune utop` no longer links `utop` in "custom" mode, which should make this


notice the comment above this message :)

Argh, will fix. Thanks.

nojb · 2023-09-14T07:26:43Z

LGTM. Just needs a doc/changes entry.

Thanks, I added a changelog entry. I didn't touch the docs otherwise since this change is an implementation detail.

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

gasche · 2023-09-14T09:32:08Z

Thanks!

"A nice speedup" and "a good speedup" sound good, but very vague. Could you actually provide real numbers for one of your tests, so that we can get a ballpark estimate of the difference?

grayswandyr · 2023-09-14T09:38:18Z

On my examples, I got a 3x-4x speed up but these are few, rather small examples, on a basic laptop, so I don't think it's very meaningful. But it's very noticeable (almost instantaneous) when running lab-session exercises, while students used to complain of the several seconds it took to run their code each time it's modeified.

nojb · 2023-09-14T10:10:59Z

"A nice speedup" and "a good speedup" sound good, but very vague. Could you actually provide real numbers for one of your tests, so that we can get a ballpark estimate of the difference?

I tested by compiling the lib subdirectory of ocamlformat (using the @check alias) and then measured the time it takes to launch dune utop (which should consist essentially in the linking command).

Released version:

nojebar@PERVERSESHEAF:~/ocamlformat$ opam exec -- dune build @lib/check
nojebar@PERVERSESHEAF:~/ocamlformat$ time opam exec -- dune utop lib -- /dev/null

real    0m9.951s
user    0m13.594s
sys     0m1.242s

This PR:

nojebar@PERVERSESHEAF:~/ocamlformat$ opam exec -- ~/dune/dune.exe build @lib/check
nojebar@PERVERSESHEAF:~/ocamlformat$ time opam exec -- ~/dune/dune.exe utop lib -- /dev/null

real    0m2.849s
user    0m4.118s
sys     0m0.226s

emillon · 2023-09-14T13:42:18Z

This also completely removes utop tests from the list of slowest tests in dune. Huge thanks.

@Alizter

CHANGES: - Modules that were declared in `(modules_without_implementation)`, `(private_modules)` or `(virtual_modules)` but not declared in `(modules)` will raise an error. (ocaml/dune#7674, @Alizter) - `enabled_if` now supports `arch_sixtyfour` variable (ocaml/dune#8023, fixes ocaml/dune#7997, @Alizter)- Use `posix_spawn` instead of `fork` on MacOS. This gives us a performance boost and allows us to re-enable thread. (ocaml/dune#8090, @rgrinberg) - Experimental: Added a `$ dune monitor` command that can connect to a running `dune build` in watch mode and display the errors and progress. (ocaml/dune#8152, @Alizter) - No longer emit linkopts(javascript) in META files (ocaml/dune#8168, @hhugo) - The `progress` RPC procedure now has an extra field for the `In_progress` constructor for the number of failed jobs. (ocaml/dune#8212, @Alizter) - Add a `--preview` flag to `dune fmt` which causes it to print out the changes it would make without applying them (ocaml/dune#8289, @gridbugs) - Introduce `(source_trees ..)` to the install stanza to allow installing entire source trees. (ocaml/dune#8349, @rgrinberg) - Deprecate install destination paths beginning with ".." to prevent packages escaping their designated installation directories. (ocaml/dune#8350, @gridbugs) - Stop signing source files with substitutions. Sign only binaries instead (ocaml/dune#8361, fixes ocaml/dune#8360, @anmonteiro) - Add `--stop-on-first-error` option to `dune build` which will terminate the build when the first error is encountered. (ocaml/dune#8400, @pmwhite and @Alizter)- Dune now displays the number of errors when waiting for changes in watch mode. (ocaml/dune#8408, fixes ocaml/dune#6889, @Alizter) - Add `with_prefix` keyword for changing the prefix of the destination of installed files matched by globs. (ocaml/dune#8416, @gridbugs) - Added experimental `--display tui` option for Dune that opens an interactive Terminal User Interface (TUI) when Dune is running. Press '?' to open up a help screen when running for more information. (ocaml/dune#8429, @Alizter and @rgrinberg) - Add a `warnings` field to `dune-project` files as a unified mechanism to enable or disable dune warnings (@rgrinberg, 8448) - `dune exec`: support syntax like `%{bin:program}`. This can appear anywhere in the command line, so things like `dune exec time %{bin:program}` now work. (ocaml/dune#6035, ocaml/dune#8474, fixes ocaml/dune#2691, @emillon, @Leonidas-from-XIV) - RPC message styles are now serialised meaning that RPC diagnostics keep their Ansi styling. (ocaml/dune#8516, fixes ocaml/dune#6921, @Alizter)- Ignore internal promote rules when `--ignore-promoted-rules` is set (ocaml/dune#8518, fix ocaml/dune#8417, @rgrinberg) - Truncate output from actions that produce too much output (@tov, ocaml/dune#8351) - Allow libraries to shadow OCaml builtin libraries. Previously, builtin libraries would always take precedence. (@rgrinberg, ocaml/dune#8558) - Remove warning against `.dune` files generated by pre dune 2.0 (ocaml/dune#8611, @rgrinberg) - `dune utop` no longer links `utop` in "custom" mode, which should make this command considerably faster. (ocaml/dune#8631, fixes ocaml/dune#6894, @nojb) - Ensure that package names in `dune-project` are valid opam package names. (ocaml/dune#8331, @emillon) - dune init: parse `--public` as a public name (ocaml/dune#8603, fixes ocaml/dune#7108, @emillon) - Remove versions 0.1 and 0.2 of the experimental ctypes extension. (ocaml/dune#8293, @emillon)

@Alizter

CHANGES: - `enabled_if` now supports `arch_sixtyfour` variable (ocaml/dune#8023, fixes ocaml/dune#7997, @Alizter) - Use `posix_spawn` instead of `fork` on MacOS. This gives us a performance boost and allows us to re-enable thread. (ocaml/dune#8090, @rgrinberg) - Experimental: Added a `$ dune monitor` command that can connect to a running `dune build` in watch mode and display the errors and progress. (ocaml/dune#8152, @Alizter) - The `progress` RPC procedure now has an extra field for the `In_progress` constructor for the number of failed jobs. (ocaml/dune#8212, @Alizter) - Add a `--preview` flag to `dune fmt` which causes it to print out the changes it would make without applying them (ocaml/dune#8289, @gridbugs) - Introduce `(source_trees ..)` to the install stanza to allow installing entire source trees. (ocaml/dune#8349, @rgrinberg) - Add `--stop-on-first-error` option to `dune build` which will terminate the build when the first error is encountered. (ocaml/dune#8400, @pmwhite and @Alizter) - Dune now displays the number of errors when waiting for changes in watch mode. (ocaml/dune#8408, fixes ocaml/dune#6889, @Alizter) - Add `with_prefix` keyword for changing the prefix of the destination of installed files matched by globs. (ocaml/dune#8416, @gridbugs) - Added experimental `--display tui` option for Dune that opens an interactive Terminal User Interface (TUI) when Dune is running. Press '?' to open up a help screen when running for more information. (ocaml/dune#8429, @Alizter and @rgrinberg) - Add a `warnings` field to `dune-project` files as a unified mechanism to enable or disable dune warnings (@rgrinberg, 8448) - `dune exec`: support syntax like `%{bin:program}`. This can appear anywhere in the command line, so things like `dune exec time %{bin:program}` now work. (ocaml/dune#6035, ocaml/dune#8474, fixes ocaml/dune#2691, @emillon, @Leonidas-from-XIV) - Make copy sandbox support directory targets. (ocaml/dune#8705, fixes ocaml/dune#7724, @emillon) - Add a new alias `@doc-json` to build odoc documentation in JSON format. This output can be consumed by external tools. (ocaml/dune#8178, @emillon) - Modules that were declared in `(modules_without_implementation)`, `(private_modules)` or `(virtual_modules)` but not declared in `(modules)` will raise an error. (ocaml/dune#7674, @Alizter) - No longer emit linkopts(javascript) in META files (ocaml/dune#8168, @hhugo) - Deprecate install destination paths beginning with ".." to prevent packages escaping their designated installation directories. (ocaml/dune#8350, @gridbugs) - RPC message styles are now serialised meaning that RPC diagnostics keep their Ansi styling. (ocaml/dune#8516, fixes ocaml/dune#6921, @Alizter) - Truncate output from actions that produce too much output (@tov, ocaml/dune#8351) - Allow libraries to shadow OCaml builtin libraries. Previously, builtin libraries would always take precedence. (@rgrinberg, ocaml/dune#8558) - Remove warning against `.dune` files generated by pre dune 2.0 (ocaml/dune#8611, @rgrinberg) - `dune utop` no longer links `utop` in "custom" mode, which should make this command considerably faster. (ocaml/dune#8631, fixes ocaml/dune#6894, @nojb) - Ensure that package names in `dune-project` are valid opam package names. (ocaml/dune#8331, @emillon) - init: check that module names are valid (ocaml/dune#8644, fixes ocaml/dune#8252, @emillon) - dune init: parse `--public` as a public name (ocaml/dune#8603, fixes ocaml/dune#7108, @emillon) - Stop signing source files with substitutions. Sign only binaries instead (ocaml/dune#8361, fixes ocaml/dune#8360, @anmonteiro) - Remove versions 0.1 and 0.2 of the experimental ctypes extension. (ocaml/dune#8293, @emillon)

Khady · 2023-11-14T07:51:44Z

We have one lib at ahrefs where the speedup is 20x, from 40s to 2s. Thanks

nojb · 2023-11-14T07:56:03Z

r approach that was discussed in #6894 was to pass the output of dune ocaml top to utop (cc @rgrinberg). However, this has a number of limitations, see eg #8626; also, PPX does not seem to be supported by dune ocaml top. These limitations could/should be lifted, but this PR is meant to be a low-effort fix for the slow loadi

Thanks for letting us know!

@Alizter

CHANGES: - `enabled_if` now supports `arch_sixtyfour` variable (ocaml/dune#8023, fixes ocaml/dune#7997, @Alizter) - Use `posix_spawn` instead of `fork` on MacOS. This gives us a performance boost and allows us to re-enable thread. (ocaml/dune#8090, @rgrinberg) - Experimental: Added a `$ dune monitor` command that can connect to a running `dune build` in watch mode and display the errors and progress. (ocaml/dune#8152, @Alizter) - The `progress` RPC procedure now has an extra field for the `In_progress` constructor for the number of failed jobs. (ocaml/dune#8212, @Alizter) - Add a `--preview` flag to `dune fmt` which causes it to print out the changes it would make without applying them (ocaml/dune#8289, @gridbugs) - Introduce `(source_trees ..)` to the install stanza to allow installing entire source trees. (ocaml/dune#8349, @rgrinberg) - Add `--stop-on-first-error` option to `dune build` which will terminate the build when the first error is encountered. (ocaml/dune#8400, @pmwhite and @Alizter) - Dune now displays the number of errors when waiting for changes in watch mode. (ocaml/dune#8408, fixes ocaml/dune#6889, @Alizter) - Add `with_prefix` keyword for changing the prefix of the destination of installed files matched by globs. (ocaml/dune#8416, @gridbugs) - Added experimental `--display tui` option for Dune that opens an interactive Terminal User Interface (TUI) when Dune is running. Press '?' to open up a help screen when running for more information. (ocaml/dune#8429, @Alizter and @rgrinberg) - Add a `warnings` field to `dune-project` files as a unified mechanism to enable or disable dune warnings (@rgrinberg, 8448) - `dune exec`: support syntax like `%{bin:program}`. This can appear anywhere in the command line, so things like `dune exec time %{bin:program}` now work. (ocaml/dune#6035, ocaml/dune#8474, fixes ocaml/dune#2691, @emillon, @Leonidas-from-XIV) - Make copy sandbox support directory targets. (ocaml/dune#8705, fixes ocaml/dune#7724, @emillon) - Add a new alias `@doc-json` to build odoc documentation in JSON format. This output can be consumed by external tools. (ocaml/dune#8178, @emillon) - Modules that were declared in `(modules_without_implementation)`, `(private_modules)` or `(virtual_modules)` but not declared in `(modules)` will raise an error. (ocaml/dune#7674, @Alizter) - No longer emit linkopts(javascript) in META files (ocaml/dune#8168, @hhugo) - Deprecate install destination paths beginning with ".." to prevent packages escaping their designated installation directories. (ocaml/dune#8350, @gridbugs) - RPC message styles are now serialised meaning that RPC diagnostics keep their Ansi styling. (ocaml/dune#8516, fixes ocaml/dune#6921, @Alizter) - Truncate output from actions that produce too much output (@tov, ocaml/dune#8351) - Allow libraries to shadow OCaml builtin libraries. Previously, builtin libraries would always take precedence. (@rgrinberg, ocaml/dune#8558) - Remove warning against `.dune` files generated by pre dune 2.0 (ocaml/dune#8611, @rgrinberg) - `dune utop` no longer links `utop` in "custom" mode, which should make this command considerably faster. (ocaml/dune#8631, fixes ocaml/dune#6894, @nojb) - Ensure that package names in `dune-project` are valid opam package names. (ocaml/dune#8331, @emillon) - init: check that module names are valid (ocaml/dune#8644, fixes ocaml/dune#8252, @emillon) - dune init: parse `--public` as a public name (ocaml/dune#8603, fixes ocaml/dune#7108, @emillon) - Stop signing source files with substitutions. Sign only binaries instead (ocaml/dune#8361, fixes ocaml/dune#8360, @anmonteiro) - Remove versions 0.1 and 0.2 of the experimental ctypes extension. (ocaml/dune#8293, @emillon)

rgrinberg approved these changes Sep 14, 2023

View reviewed changes

rgrinberg added this to the 3.11.0 milestone Sep 14, 2023

nojb force-pushed the utop_simple_linking branch from f838542 to bf2ab44 Compare September 14, 2023 07:25

nojb and others added 3 commits September 14, 2023 09:25

dune utop: do not do custom linking

f2c9a16

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

refactor: move CAML_LD_LIBRARY_PATH to a single place

d8a59d2

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

Add test

7a3bf06

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

nojb force-pushed the utop_simple_linking branch from bf2ab44 to 94b02fb Compare September 14, 2023 07:25

rgrinberg reviewed Sep 14, 2023

View reviewed changes

CHANGES.md

76edb4d

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

nojb force-pushed the utop_simple_linking branch from 94b02fb to 76edb4d Compare September 14, 2023 07:28

nojb merged commit cebb6b5 into ocaml:main Sep 14, 2023
18 of 20 checks passed

nojb deleted the utop_simple_linking branch September 14, 2023 08:06

nojb mentioned this pull request Sep 14, 2023

Revamp dune utop for greater speed #6894

Closed

emillon mentioned this pull request Sep 14, 2023

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

Closed

frejsoya mentioned this pull request Sep 19, 2023

Use custom build mode for mdx to work with cstubs. #8704

Closed

emillon mentioned this pull request Sep 22, 2023

[new release] dune (15 packages) (3.11.0) ocaml/opam-repository#24510

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dune utop: do not do custom linking #8631

dune utop: do not do custom linking #8631

nojb commented Sep 12, 2023 •

edited

Loading

grayswandyr commented Sep 12, 2023

nojb commented Sep 12, 2023

rgrinberg commented Sep 13, 2023

grayswandyr commented Sep 13, 2023

nojb commented Sep 14, 2023

rgrinberg left a comment

rgrinberg Sep 14, 2023

nojb Sep 14, 2023

nojb commented Sep 14, 2023

gasche commented Sep 14, 2023

grayswandyr commented Sep 14, 2023

nojb commented Sep 14, 2023 •

edited

Loading

emillon commented Sep 14, 2023

Khady commented Nov 14, 2023 •

edited

Loading

nojb commented Nov 14, 2023 •

edited

Loading

dune utop: do not do custom linking #8631

dune utop: do not do custom linking #8631

Conversation

nojb commented Sep 12, 2023 • edited Loading

grayswandyr commented Sep 12, 2023

nojb commented Sep 12, 2023

rgrinberg commented Sep 13, 2023

grayswandyr commented Sep 13, 2023

nojb commented Sep 14, 2023

rgrinberg left a comment

Choose a reason for hiding this comment

rgrinberg Sep 14, 2023

Choose a reason for hiding this comment

nojb Sep 14, 2023

Choose a reason for hiding this comment

nojb commented Sep 14, 2023

gasche commented Sep 14, 2023

grayswandyr commented Sep 14, 2023

nojb commented Sep 14, 2023 • edited Loading

emillon commented Sep 14, 2023

Khady commented Nov 14, 2023 • edited Loading

nojb commented Nov 14, 2023 • edited Loading

nojb commented Sep 12, 2023 •

edited

Loading

nojb commented Sep 14, 2023 •

edited

Loading

Khady commented Nov 14, 2023 •

edited

Loading

nojb commented Nov 14, 2023 •

edited

Loading