Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revamp dune utop for greater speed #6894

Closed
grayswandyr opened this issue Jan 16, 2023 · 15 comments · Fixed by #8631 or ocaml/opam-repository#24510
Closed

Revamp dune utop for greater speed #6894

grayswandyr opened this issue Jan 16, 2023 · 15 comments · Fixed by #8631 or ocaml/opam-repository#24510
Milestone

Comments

@grayswandyr
Copy link

After a public, then private discussion with @rgrinberg , I'm submitting this issue about dune utop. dune utop is very handy for our students but it's slow (> 5s on my 2019 laptop for a project only printing "hello world") when it needs to be rebuilt. The slow part is the linking phase building an ad hoc utop.exe.

While dune utop is perhaps (?) not that useful to an everyday developer, this is the command our students use the most and it seems important, in this respect, to make it fast.

@rgrinberg suggested using TM=$(mktemp) && dune ocaml top > "$TM" && utop -init "$TM" to have a fast experience. If this is indeed equivalent to dune utop in all cases, it might be interesting to redesign dune utop to perform these steps rather than rebuilding utop.exe.

@emillon
Copy link
Collaborator

emillon commented Jan 17, 2023

Is it equivalent to using #use_output "dune ocaml top" from within utop? in that case we could also add a shortcut to do that from within utop itself.

@grayswandyr
Copy link
Author

It is.
But from a UX point of view, I think it would be good to keep a single short command (such as dune utop) to get a "project-aware" utop environment, rather than launch utop and then, each time you run utop, issue a long utop directive that students will forget or mistype. But as long as we have such a short command, it doesn't really matter if it's a dune or utop feature (in the latter case, the only small issue is that it would be the only command not starting by dune ...).

@gasche
Copy link
Member

gasche commented Sep 9, 2023

If @rgrinberg did not suggest to reimplement dune utop in terms of whatever faster method is available, I suppose that there are downsides to the approach of dune ocaml top. Are those explained somewhere that you can point to? I hadn't heard of this before, I occasionally use dune utop and had remarked that it is frustratingly slow, and I wonder if there is documentation somewhere.

@rgrinberg
Copy link
Member

There's no upside to using dune utop over dune ocaml top. We just kept dune utop around not to break existing user workflows. Everybody is recommended to use dune ocaml top though.

As for why dune utop is slow, I have no idea but Etienne narrowed it done to the linking command. You can run $ dune utop --trace-file=foo.json and then observe what's slow in foo.json in a viewer like perfetto.

@gasche
Copy link
Member

gasche commented Sep 10, 2023

But in this case, why not reimplement dune utop in terms of dune ocaml top? Is this a good idea that you just haven't the time to implement, or are there other reasons? (What are the "user workflows" that would break if the dune utop command was reimplemented internally in this way?)

@rgrinberg
Copy link
Member

No strong reason, it just hasn't been done yet.

@emillon
Copy link
Collaborator

emillon commented Sep 11, 2023

As for why dune utop is slow, I have no idea but Etienne narrowed it done to the linking command. You can run $ dune utop --trace-file=foo.json and then observe what's slow in foo.json in a viewer like perfetto.

More specifically, in that configuration the ocaml compiler is generating a C file which embeds bytecode in it and at least gcc is pretty slow in compiling this. Not sure if that approach is inherently slow or if we can do something about that.

@rgrinberg
Copy link
Member

Why are we linking the toplevel this way? Shouldn't it be enough to just emit the bytecode and run it with the interpreter?

@nojb
Copy link
Collaborator

nojb commented Sep 11, 2023

Why are we linking the toplevel this way? Shouldn't it be enough to just emit the bytecode and run it with the interpreter?

I think that will require setting CAML_LD_LIBRARY_PATH, but I don't see why it would be an obstacle, given that Dune is the one launching utop and so it is free to do whatever it wants.

@nojb
Copy link
Collaborator

nojb commented Sep 11, 2023

Why are we linking the toplevel this way? Shouldn't it be enough to just emit the bytecode and run it with the interpreter?

I think that will require setting CAML_LD_LIBRARY_PATH, but I don't see why it would be an obstacle, given that Dune is the one launching utop and so it is free to do whatever it wants.

let utop_exe =
(* Use the [.exe] version. As the utop executable is declared with [(modes
(byte))], the [.exe] correspond the bytecode linked in custom mode. We do
that so that it works without hassle when generating a utop for a library
with C stubs. *)
Filename.concat utop_dir_basename (exe_name ^ Mode.exe_ext Mode.Native)
;;

@rgrinberg
Copy link
Member

Which reminds me, doesn't #use_output "dune ocaml top" not set CAML_LD_LIBRARY_PATH? So I guess it's currently broken with stubs?

@nojb
Copy link
Collaborator

nojb commented Sep 11, 2023

Which reminds me, doesn't #use_output "dune ocaml top" not set CAML_LD_LIBRARY_PATH? So I guess it's currently broken with stubs?

It doesn't need to, because the directories set using #directory are used to look up shared libraries for the stubs, so it should work even without setting CAML_LD_LIBRARY_PATH. Setting CAML_LD_LIBRARY_PATH is only necessary if we take the approach of doing a "normal" bytecode link of utop; if we use the init file then it shouldn't be necessary as far as I can see.

@rgrinberg
Copy link
Member

Okay in that case there's no point to change the link type and we can just launch utop and set #use_output "dune ocaml top".

@nojb
Copy link
Collaborator

nojb commented Sep 11, 2023

Okay in that case there's no point to change the link type and we can just launch utop and set #use_output "dune ocaml top".

I agree.

@nojb
Copy link
Collaborator

nojb commented Sep 14, 2023

In #8631 we switched to "normal" (instead of "custom") linking for the utop binary; this should alleviate some of the slowness of dune utop.

@rgrinberg rgrinberg added this to the 3.11.0 milestone Sep 14, 2023
emillon added a commit to emillon/opam-repository that referenced this issue Sep 14, 2023
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)
emillon added a commit to emillon/opam-repository that referenced this issue Sep 22, 2023
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment