-
Notifications
You must be signed in to change notification settings - Fork 195
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
Dune support #1687
Dune support #1687
Conversation
81ec160
to
5c9b714
Compare
<!-- ps-id: 954ea380-2db6-46f1-b33c-6f3384074297 --> Signed-off-by: Ali Caglayan <alizter@gmail.com>
5c9b714
to
c611d5a
Compare
I tested this and it works for me. I did notice that building with dune is ~5% slower than building with make with -j8 and about 8% slower with -j1. Not sure why this would be. But since this PR doesn't eliminate the use of make, that's ok. |
Very well, merging this now. I will investigate the performance issues, but my suspicion is the calls to |
<!-- ps-id: 954ea380-2db6-46f1-b33c-6f3384074297 --> Signed-off-by: Ali Caglayan <alizter@gmail.com>
@jdchristensen Would you be able to test with Dune 3.7? I fixed the slow calls of coqdep (they are now called once per coq.theory). There should be no observable difference between the Dune and Makefile at this point. |
I tested building Coq-HoTT commit 8d1c6a7 with Dune 3.7.0 vs GNU Make 4.3. With The parallel build time is very sensitive to the build order, since dependencies can prevent 8 jobs from being run at once. I know make is doing things suboptimally, but it's likely that dune by chance is choosing an even worse order. Does dune use any heuristics to decide which of the possible targets to build next? One possible heuristic is to prefer targets with long chains of dependencies relying on them. One could also imagine a build system that records timing info for each build and uses it when scheduling the jobs in the next run. An example illustrating what I mean is at https://lists.gnu.org/archive/html/bug-make/2009-12/msg00023.html (BTW, I ran dune with |
Another issue that might contribute to this is that the dune process uses a few seconds of cpu time. With -j1 that might not slow down the build, because it could happen on a different core. But when all cores are in use, it would slow down the build. make uses 0s of cpu time, to the nearest second. Most of the cpu time dune uses happens in the first two seconds, and then more gradually accumulates. Maybe some inner loops can be optimized? Does ocaml support profiling? |
Thanks a lot for the measurements @jdchristensen , I think there may have been some regression in Dune in some of the last recent releases, the overheard you are witnessing should not be happening, and I have noticed it in my own projects as well. We are investigating more. |
@Alizter points to some problems with the display setup in dune, an interesting option to try too is |
I tried with I did a bit more investigating, and some of the slowness of dune is definitely at the start. If I do
Both dune and make get the first file built in about 0.9s. But then dune waits almost 2s before doing the second file, while make gets the second file done within 0.04s of the first file. (The second file is very short.) Each of these files depends on the previous one, so there is no parallelization at this stage. I repeated the timings to make sure they are reproducible. Since the entire build takes ~40s with make and ~42s with dune, that 2s explains the 5% difference. With |
I wonder if the delay at the start has something to do with dune having 2770 targets to process, even though there are only 553 .v files. In the first second or two, it jumps up to 1693/2770 done. On another note, I was hoping that if I made an innocuous change to a file, e.g. just changing a comment, then after it got rebuilt, the hash of its output file would be unchanged, so that its dependencies wouldn't need to be rebuilt. But I'm not observing that. Is this because Coq build artefacts include some information about character positions in a file, or something like that? Even so, I would have hoped that second-generation dependencies then wouldn't need to be built, but they do. |
Thanks for all the testing @jdchristensen , a few comments:
|
I wasn't mentioning 2770 as a large number in absolute terms. But it is almost five times larger than the actual number of source files, so I wondered if there is some repetition causing an issue, especially since around 60% of the targets get handled during the 2s delay at the start. @Alizter probably knows where these phantom targets are coming from. (Also, @Alizter : I noticed that etc/generate_coqproject.sh doesn't realize we are in a git repo when the build is done using dune, probably because we have cd'd into a subdir. Maybe we should check the return value of Thanks for the other info. I would be really excited if incremental builds managed to avoid building certain things that aren't affected by the changes. Even if just whitespace / comment changes were ignored, that would be helpful. ... Ah, I see that if I change a comment without changing its length, no dependencies get rebuilt. So it does seem like it must be location information that is causing the issue, and certainly dependencies don't need to be rebuilt if only character positions in the file change. |
The number of targets is large because for each
Yes, that'd be great, that kind of increental setup already works with But indeed at some point we should be able to do that for whole projects (coq-lsp will only work inside the same file); please don't hesitate to open bugs upstream to signal these kind of issues. As of today Coq embeds very little location info into .vo files so it'd be good to know which particular part of the system is causing your issue. |
Ok, I see that in many cases, Coq will produce a bitwise identical .vo file, even if you use different tactics to produce the same proof term. This is great, and makes me a full convert to dune. I also managed to isolate some lines that caused this issue in my testing. I happened to test using Basics/Overture.v, which has lines that cause the misbehaviour. I opened coq/coq#17312 with specific examples. |
Thanks for the report, indeed it is important to understand how much of these cases happens as we are in the process of maybe adding some more metadata (such as documentation or location) to .vo files. |
@ejgallego By the way, I can't find anywhere in the dune documentation that it mentions that rebuilding is determined by the contents of the dependencies rather than their timestamps. This is a fundamental difference (and huge improvement) from what most people are used to, so I would expect it to be mentioned in the very first page (Overview) as well as in other pages. It's also not in the "dune-build" man page / For others listening in, I have to also advertise the caching feature of dune (which you have to enable in your config). When you move around in history, make copies of your tree and rebuild, or make a change and then undo it, your next build can be almost instantaneous if most parts have been built before. |
@jdchristensen indeed it seems documentation is non-existent on when rules are rebuilt. While I guess Dune developers didn't want to expose too much of the internals to users (as to for example allow them some room to change how rules are re-executed) I agree that documentation should say something about it. It is agreed upon that Dune documentation could use a lot of improvement. Would you mind opening a bug upstream https://github.com/ocaml/dune ? As to how rules are reexecuted, as of today that's a mix of factors, basically dune sees rules as pure functions, where the input is all the dependencies that are tracked, then the rule itself is hashes, so things are reexcuted when the rules change, or the dependencies of the rule are changed, this includes the commands involved in the rule action. |
Indeed the cache is great, and at some point people how to extend the caching to be more distributed so effectively you could checkout a HoTT branch and pull the build from a network cache. Caching is nice as thanks to Dune architecture all you have to do is to check if a rule hash is already in the cached db. By the way the 2sec delay likely comes from the rule analysis itself, but indeed it has to be some bad codepath due to newer Memoization tech, we are investigating. |
I mentioned the lack of documentation to the other dune devs by the way. It was agreed that we could probably have a high level sketch about how Dune works. |
I did some more detailed benchmarks yesterday using a tool called https://github.com/sharkdp/hyperfine. I tested We ran each builder at different levels of parallelism from 1 to 20 in steps of 3. They all seemed to agree that 7 was optimal, so I will only summarise those results (7 isn't special here, it could easily be 8 or 9). Here is a summary of the results:
Dune serialises data into a string (using OCaml marshalling) and then hashes it. So optimising the hashing of rules has two critical points. The hashing algorithm in dune today is md5. The xxH hashing algorithm is extremely fast which is evident from how much it was able to catch up to make. I suspect if work is done to avoid marshalling data, then we could probably even overtake make.
And here is what hyperfine did overnight: Raw command output
|
<!-- ps-id: 954ea380-2db6-46f1-b33c-6f3384074297 --> Signed-off-by: Ali Caglayan <alizter@gmail.com>
We switch the opam job to use Dune to build the library. The opam file is now generated by Dune. We also run coqchk in the opam job.
For regular users of make, nothing should change. I plan to switch completely to Dune in the future, but for that I need to do some more work to adapt our scripts.
This also means that if you have Dune >= 3.5 you can build the library using
dune build
and run tests (just coqchk for now) withdune build @runtest
. Dune has a number of advantages as a build system overmake
such as the ability to compose projects easily (i.e. plop coq and HoTT in the same workspace), cache build commands, etc. Dune doesn't work on timestamps like make but rather the hashed contents (digests) of the contents of the workspace. The build artifacts appear in_build/default
allowing for a clean separation from the sources. Editors should be able to work with Dune as I have patched _CoqProject to take into account the_build
directory. This might only work if the editor is started in the HoTT directory however.