dune utop: do not do custom linking by nojb · Pull Request #8631 · ocaml/dune

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

 `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

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

Conversation

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

Uh oh!

Uh oh!

grayswandyr commented Sep 12, 2023

Uh oh!

nojb commented Sep 12, 2023

Uh oh!

rgrinberg commented Sep 13, 2023

Uh oh!

grayswandyr commented Sep 13, 2023

Uh oh!

nojb commented Sep 14, 2023

Uh oh!

rgrinberg left a comment

Choose a reason for hiding this comment

Uh oh!

rgrinberg Sep 14, 2023

Choose a reason for hiding this comment

Uh oh!

nojb Sep 14, 2023

Choose a reason for hiding this comment

Uh oh!

nojb commented Sep 14, 2023

Uh oh!

gasche commented Sep 14, 2023

Uh oh!

grayswandyr commented Sep 14, 2023

Uh oh!

nojb commented Sep 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emillon commented Sep 14, 2023

Uh oh!

Khady commented Nov 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nojb commented Nov 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

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