Refining dependencies

[arozovyk]

This PR addresses the problem of unnecessary module recompilations occurring on library interface changes despite the fact that the module being recompiled does not directly depend on that library (only some other module(s) in the library/executable do).

The cause of this seems to be the fact that dune indiscriminately associates a Dep.Fact.File_selector (representing a set of Files.tof a library) to every single module of executable/library compiled, when only a subset of modules does in fact depend on it. We can observe it in Build_system.Exported.execute_rule_impl where the Action.t is run, returning a set of Dep.Fact.t used to compute the rule digest.

The rule responsible for this association is produced by Command.run in Module_compilation.build_cm. One of its arguments (the 4th) contains Command.Args.Hidden_deps (Lib_file_deps.deps requires), requires being the list of library dependencies defined by compile info (either Lib.Compile.direct_requires or Lib.Compile.requires_link, depending on implicit_transitive_deps value). On rule execution, the command is expanded and the aforementioned list of library dependencies is registered asDep.Fact.File_selectors for targets being produced.

In this PR I attempt using ocamldeps output in order to filter the list of "requires" preventing them from being registered as Dep.Fact.File_selector for modules that do not need it.

The solution is structured as following :

1. In the Ocamldep module, instead of keeping only intra-library/executable modules, keep the full ocamldeps output and write it to all-deps file. Introduce new type Module_dep.t = | Local of Module.t | External of External_name.t.  When parsing all-deps file, return Local of Module.t when the module is found (old behavior), otherwise return External of External_name.t. Modify the Dep_graph.t to contain both types of dependencies.
2. When creating Compilation_context.Includes.t (where we make Command.Args.Hidden_deps) add 2 arguments: module that is being compiled and dependency graph. (Not sure what is the proper way of doing that, here I just transformed Compilation_context.t.includes into a partially applied function that waits for a module). Given the additional information, filter "requires" of type Lib_flags.L.t that are local (as opposed to the ones coming from the "installed world") in order to only keep the "real" dependencies indicated by ocamldep.

[arozovyk]

Added some maps to get entry modules for lib closure in order to compare it to ocamldeps output. Dune is compiling now with 2930 File_selectors less. So the PR is ready for some broader testing.
The code needs some refactoring and optimization though.

[Alizter]

@arozovyk If you want to get quicker feedback from the CI, make a small PR (like docs or comments) so that the CI will run as you will lose the first-time contributor status here.

[arozovyk]

@Alizter
Just tried to open a new PR to test the CI, but it still had to be approved in order to run. I'm not sure I'm getting the "like docs or comments" part. Should I specify something before opening the PR ?

[Alizter]

@arozovyk The PR has to be merged which is why I suggested a small improvement like documentation.

[Alizter]

It appears that bootstrapping is broken, hence why most of the jobs fail. Try debugging why make -B _build/dune.exe is not working. Typically it is a missing library or something.

[arozovyk]

Yea, I'm breaking it from time to time :) But now it's "Install Deps on Unix" that's failing. I think it's due to "-open" flags. Just added some (well overdue) checks to try to fix it.

[arozovyk]

So skipping filtering for unwrapped libs seems to have worked. Now I'm breaking a decent amonut of tests but it looks like much of it is due to the changes to Ocamldep module and to the order of execution.

[Alizter]

@arozovyk How is it going by the way?

[arozovyk]

@Alizter Haven't been able to work much on it last week due to a conference, but I managed to fix the obvious mess-ups leading to insane memory usage, and now I'm testing it against a (not very large) monorepo.

Currently it uses like 40-60% more memory but I have few ideas how to do things better algorithmically, might take some time though.
the PR:

Main:

I dont have the screenshot for the version @nojb tested, but it was something like 50G of memory used for the example above:)

(Also, I'm missing some specific cases from what i see in the CI, but it should be easy to fix now).

[Alizter]

Are you saying that it used 60% more allocations? From the graph you just posted it appears it is even using less memory only 280MB whereas main is using 300MB. Or are the pictures swapped?

[arozovyk]

I meant total allocations, yea. Hm, the pictures are in the right order, so I think I'm misunderstanding the result and the PR indeed has less memory usage on average (?)

[nojb]

Hello! What is the status of the PR?

I ran a few rough benchmarks with LexiFi's codebase and compared it with main at the time this PR was branched off (9ebbb77). The good news is that Dune no longer runs out of memory :)

However, I observed a slowdown in the build proper: for a full build (starting from nothing), the build time goes from ~1m55 to ~2m5 (roughly). The slowdown in zero builds (starting from a fully built tree) is much more marked: it goes from ~3s to ~15s.

Independently of that, could you state precisely what is the optimization that is implemented by this PR? I did a few tests that I thought would trigger the optimization but there wasn't any difference as far as I could see, but perhaps I am missing something.

[arozovyk]

Hello !
Well, the branch you tested does indeed slow down Dune by around ~5-10% when building from zero. I'm getting similar results running it againstbench/monorepo.

There are two reasons for that, fisrt, currently Dune keeps only a part of ocamldep's output that it writes to .d and .all-deps files. Keeping the full output in order to reuse that later for filtring and not radically changing the current approach means writing and then reading (a lot) more lines, which, for instance, makes bench/monorepo full build run from ~24m16 to ~24m39. Here are memtraces of just the change (no filtering) and that of main for comparison: full_ocamldep_output.mem.zip , main.mem.zip note the Stdune__Io.Make.with_file_in difference.

Secondly, in order to filter dependencies , we have to work with just the module names as strings given by ocamldep. This is problematic for several reasons: A library name (Lib.name) often has nothing to do with its entry module names that have to be computed for each library we are trying to filter. Also, sometimes having ocamldep module name is just not enough. A concrete example:

Given a file alcotest-async.1.6.0/test/e2e/alcotest-lwt/failing/async_failure.ml
, ocamldep gives us Alcotest_lwt Lwt Lwt_main Lwt_switch Lwt_unix as modules that we depend on.
Corresponding dune file declares following dependencies:
(libraries alcotest alcotest.stdlib_ext alcotest-lwt lwt lwt.unix)
So when considering alcotest (having single entry module name Alcotest) as a dependency for current file, we will not find it in ocamldeps output and remove it. Which would be false, since the file does in fact depend on it trough the use of

val test_case :
    string ->
    Alcotest.speed_level ->

declared in Alcotest_lwt's interface.

The only solution that I'm currently aware of is to try and find the corresponding instance of Lib.t for Alcotest_lwt (again, having just the string) and compute the closure of the singleton that would tell us that we do depend on Alcotest transitively.

Moreover, as you know, depending on the value of implicit_transitive_deps we either use the direct requires (as declared by user), or a complete set of transitive dependencies (linking_requires) resulting from closure computation (without the information about which libraries are the direct requires that are "roots" and which are transitive dependencies of them).

The branch @nojb tested solves the aforementioned problem in a "silly" (but rather precise) way: computing the mapping of each lib (by its entry names) in linking_requires to the closure of the singleton of this lib. This is extremely costly, however, it seems to me that the fact that clean build is only slightly slower is in part due to the fact that the high closure computation cost is somewhat compensated by the parallelism due to better information about dependencies, (but also due the fact that in full build the time spent in the build system is, as a percentage, less that for an incremental build). So, for the rebuilds, it is no surprise that they are way slower as a lot more things are being computed.

Currently, I'm implementing a slightly less silly solution that changes the type of linking_requires to be a mapping of the "root" libraries (coming from the direct_requires) to the resulting sets of closures. Here, there are two cases to consider : in the case of (implicit_transitive_deps==true), which is the more complicated one, the ocamldep can sometimes point us to the libraries that are not part of direct_requires ("roots"), in which case, we have to chose between a precise solution, that computes yet another closure from the missing libraries. Or the less precise solution that is to iterrate on the "roots" that were not required according to ocamldep until we find a subset that covers the missing libraries.

In the second case, (implicit_transitive_deps==false) this mapping of transitive closures will also be necessary for the problem with ocamldepthat was mentionner earlier, i.e. even when it doesn't mention a library doesn't necessarily mean it is not needed.

In both cases, independently of the value implicit_transitive_deps, it means that the closures would have to be computed, which is a slight tradeoff. Still, it should reduce the overall number of closures computed drastically compared to the current version of this PRs branch.

Also, for the rebuild problem, even if we manage to find a viable solution using just ocamldep, it would still result in an overhead due to closure computations. For this, it would be great to instead use the information from the already generated .cmx via ocamlobjinfo.

I'll get back to you as soon as I have any new results.

[rgrinberg]

Would it make sense to only enable this optimization when implicit_transitive_deps is enabled? This mode is already required to improve compilation performance, so I'm not against moving additional optimizations there.

[nojb]

Hello, sorry for the delay, I've been trying to understand things better before responding. I'm not sure I follow all the details in your response, so I thought I would spell out what I understand about this topic and we can see if it maches up with what this PR is doing.

Currently, for every module in a "buildable" (library or executable), Dune declares that the .cmx of the module depends on the "globs" $libdir/byte/*.cmi and $libdir/native/*.cmx where $libdir ranges over the directories of every library mentioned in the (libraries) field of the buildable. This logic lives in the Compilation_context.Includes module. The actual resolution of the "glob" into individual files happens later in the process. Just to emphasize, this is where the inefficiency comes from: the same set of dependencies is used for every module. As far as I undersetand, the set of libraries considered here is not transitively closed; it consists essentially of those libraries mentioned in the (libraries) field of the buildable.

Given this, the naïve approach would be to approximate, for each module being compiled, a subset of the libraries listed in the (libraries) field so that a narrower set of globs may be declared as dependencies. To do so, one should take the output of ocamldep for the module and compare this information with the list of modules of each library to decided whether to include that library as a dependency or not (perhaps this is the step that is a bit complicated because this information is not available at the point where this information is needed?).

My questions are:

• Is this the approach that the PR is taking? If not, could you explain what the PR is doing, in terms of differences between what Dune does today and what is done in the PR?
• Why does the code need to calculate transitive closure of dependencies, if that does not seem needed by the current code?
• Could you provide a small example that shows the optimization kicking in? Concretetly, I would like to see a description of a small Dune project and a sequence of actions that would show rebuilding taking place when using stock Dune but not when using the PR.

Sorry for the many questions, I am trying to understand what is going on :) Thanks!

[arozovyk]

Hello, @nojb

Also sorry for the delay, I've been working on the new implementation (described in my previous post) and testing it out to also get a better understanding of the performance side of things.

Just to start answering your questions :

Is this the approach that the PR is taking? If not, could you explain what the PR is doing, in terms of differences between what Dune does today and what is done in the PR?

No, see details below.

Why does the code need to calculate transitive closure of dependencies, if that does not seem needed by the current code?

The current code does need to compute transitive closures when implicit_transitive_deps is true

Could you provide a small example that shows the optimization kicking in?

Sure,

1. First, please clone https://github.com/arozovyk/deps_test. It is a project with a binary and two libs (Libtoto and Libfoo), where only one module (Main_b) of a binary depends on Libtoto , but not the other one. There is also the same kind of dependency between the two libraries.
2. Start by building it with the stock dune.
3. Change the interface of any of the modules of Libtoto, (like by removing the let d ()= () in lib_c.ml for example)
4. Rebuild it with stock dune.
5. See the log_main, where you'll find that along with Libtoto, the depending module Main_b, but also the module Main_a (that doesn't use Libtoto) have all been recompiled.
6. Run dune clean, then rebuild the project with current PR
7. Change the interface again, same as in 3.
8. Rebuild it again with dune from the PR.
9. See the new log_patched where you will find that the two commands ocamlc and ocamlopt compiling Main_a are no longer there.

Also, please observe the number of "-I" (grep -o "\-I " _build/log | wc -l) for full builds with each version of dune - there are 60 for main and 48 for the PR, meaning that the number of dependencies "module->library" (and not "module->"module_of_a_library") have been reduced.
Another thing to note, on both of my machines (an M1 Mac and a i7-3930K Linux) time shows that the total execution is faster for the PR (mean value of 0.41s out of 100 runs) compared to main (mean value of 0.49s).

---

In other cases, the PR seems to also detect the libraries that are not needed and doesn't even compile them :

• Building https://github.com/arozovyk/monorepo_csexp/tree/master with the main results in 2248 invocations of ocamloptand 2122 invocations of ocamlc. Taking ~82.57s. The number of "-I" is 77805.
• Building the same project with the PR, results in 1445 invocations of ocamloptand 1332 of ocamlc, taking ~70,97s. The number of -I is 65676.

That being said, there are cases where the total build time seems to be slightly slower.

• Building bench/monorepo with main takes around 23m50, number of "-I" in log file is 571999, total time spend in the compiler (sum of each ocamlc + ocamlopt with -dtimingsflag on) is 6465,14s, and a mean value of 0,2530s per file.
• While the pr takes 24m14. Number of "-I" is 524335 ( 8,3% less than main), with total time in the compiler : 6414,18s and a mean time per file : 0.2519s.

---

As for the "how it works":

To answer your question about Compilation_context.Includes: you said it right, it does define the logic of associating the set of library "globs" as dependencies to the modules being compiled. And yes, the problem is indeed the fact that this set is associated with every single module, regardless of the real usage. However, there are two kinds of sets of libraries depending on the value of implicit-transitive-deps. When the value is true, ( and it is by default), then the set of libraries being associated is in fact transitively closed. When the value is false, then we only consider the set of libraries defined in (libraries), as you describe it.

So how to narrow down this set of libraries? Intuitively, it should have been as easy as you describe it in your third paragraph.
Turns out, it is not quite as simple, not only for the reason you mentioned, supposing you mean having ocamldeps output of the module at the moment Compilation_context.Includes are being defined. (It took a while to get it working, but it was relatively "easy" compared to other problems.)

First, as I said in my previous post, you can't simply compare a Lib.t with ocamldeps output, for this you have to know its entry module names computed the same way as in odoc.ml. (Just to note again to be perfectly clear : we use library modules, but we are trying to remove the dependency on the whole library)

Secondly, there is a quite common exception (also described in my prev. post) where you can't simply rely on ocamldeps output to filter a set of libraries. It happens when a type, defined in a library, is mentioned in .mli but not in the .ml file. In which case ocamldep will give too little information that leads to libraries being wrongly removed. It would be nice to detect such cases, but I don't see an easy way to do it as for now.

Why closures? Rather that comparing a library entry module names with ocamldeps output, we compare the entry modules of each library X (from (libraries)) plus libraries coming from transitive closure of [X] to the set of modules, returned by ocamldep.

There are quite a few exceptions where it is still not enough, and I identified (and found a way to avoid filtering) the most of them (virtual libraries, anything related to Melange, unwrapped libraries, etc.). Some cases are yet to be indentified (maybe 5-10 of them) though, it just a pain to debug for multiple reasons.

Also for the perfomance, I'm sure there are few things that can be done to filter libraries faster, but it will also take time.

Hope that clarifies some things.
Feel free to ask any questions.

Cheers

[alainfrisch]

I randomly happened to look at the PR, and found this spot. Any reason not to use a tailrec List.fold_left here?

Performance wise, it could also be the case that using a Hashtbl to collect "seen" objects would work better here (I've no idea whether this is a bottleneck!). You could also filter out already-seen values and add objects to the table with a single pass on the h list.

[lthls]

Some update on this PR: unfortunately @arozovyk and I will no longer have time to work on this.

Here is a summary of the current state, for people who might be interested to either triage this or resume work:

• This branch can successfully remove dependencies from single files to libraries it doesn't actually depend on. Typically, if you have an executable/library with several files, and update a dependency which is only needed (transitively) by a few of the files, then only these few files will be recompiled.
• This has an impact on performance. On full builds, this can be either slightly positive (more parallelism, less bottlenecks) or slightly negative (a bit more time spent in the build system). On zero builds (all is already built), this can be more problematic, with reports of up to 100% more time spent. In partial build settings, this can speed up the build significantly if the refined dependencies can prune enough branches, but this is not always the case.
• The dependency removal is not optimal: for performance reasons, instead of computing the minimal set of library dependencies (including transitively), we compute a minimal subset of the declared dependencies that transitively cover all of the actual dependencies. An initial version of this PR performed the optimal version but was much more expensive.
• We are aware of cases where the new dependency graph is missing edges, leading to to compilation errors. In some cases (like Melange) we disable the new feature, but there are other cases that we haven't had time to investigate.
• The current PR provides no option to turn this feature on or off. Realistically, we would have included a flag to control the feature, which could be enabled or disabled by either command-line flags, configuration options, or other means. Typically watch mode would likely benefit from this, manual builds less so, and for CI builds it would depend on the actual code being built.

We also no longer have access to an industrial user to validate the usefulness of this work, and without such experimental validation it does not seem productive to keep working on this PR.

[rgrinberg]

@alainfrisch @nojb if you're interested with experimenting with this feature, you could refine it so that it can be experimentally enabled for your projects so that you can study the performance further and perhaps optimize it.

I don't see an issue with merging an experimental feature as long as it doesn't impact the rest of dune.

[rgrinberg]

@alainfrisch @nojb if you're interested with experimenting with this feature, you could refine it so that it can be experimentally enabled for your projects so that you can study the performance further and perhaps optimize it.

I don't see an issue with merging an experimental feature as long as it doesn't impact the rest of dune.

[alainfrisch]

The dependency removal is not optimal: for performance reasons, instead of computing the minimal set of library dependencies (including transitively), we compute a minimal subset of the declared dependencies that transitively cover all of the actual dependencies. An initial version of this PR performed the optimal version but was much more expensive.

Just to clarify (and confirm our understanding), following @nojb experiments, we observed the following:

• assume we have a tree with libA, libB, libC
• libB depends on libA; libC depends on libA and libB
• some module X in libC doesn't actually depend on any modules from libB but only on modules from libA
• Then with the PR, dune will nevertheless recompile X when libB is modified (i.e. in @lthls's description above, transitively would be understood as following the dependency edges in reverse direction, so libA "covers" libB)

@lthls : is that right?

In our case, most of of internal libraries depend on a single "core helper" library (which would act as libA above), so in practice, any change in any "intermediate library" (libB) trigger full recompilation of all modules in the "terminal" program (libC). But even if that program globally depends on all intermediate libraries, many of those libraries are only used by just a few modules in that program; and the goal would precisely be to avoid recompiling those who don't depend (even transitively) on a given library when it is modified.

Given that a "zero-build" is already significantly slower with the PR than without (around 50%-100% longer), we fear that dropping the restriction mentioned by @lthls would make such "zero-build" even slower, to the point where we wouldn't use the refined semantics anyway.

I don't think we will pick up work on this topic on our side, but otherwise, we'll drop a note here!

FTR, we initially had in mind a even finer notion of dependencies, where dune would only recompile module whose actual dependencies (in terms of modules, not libraries) had been touched since the previous build. We agreed with OCamlPro to restrict it to libraries (i.e. only remember that a given module depends on such and such libraries, but not on individual modules in them); the goal was to keep the representation of the dependency graph reasonably compact, but apparently this was not enough.

I wonder if the refinement (even the finer one described in the previous paragraph) wouldn't be better addressed without touching this graph, simply by short-circuiting a recompilation when we observe it is unnecessary. Typically, if dune wants to call ocamlopt to rebuild m.cmx, if it knows that the existing m.cmx had been compiled previously with the right command-line (same flags, etc), then it could just check the digests (for .cmi and .cmx files) listed in the m.cmx and look if any of the actual dependencies (recorded in the .cmx) is now invalid; if all dependencies are still valid, no need to recompile. This would mean opening a bunch of .cmi/.cmx files, but dune could maintain of cache of those digests.

[lthls]

@lthls : is that right?

Note quite. In your description here, you say that libC depends on libA and libB. In this case, the current state of the PR works: we only need libA, so we can remove libB as a dependency.

In the example you actually sent us, libC depends on libB (and not libA), but its modules actually depend on libA and rely on transitive dependencies to resolve that. So we detect that we only need a dependency on libA, but our current heuristic only allows to cover the actual dependencies through declared dependencies, and so our only solution is to keep libB, which is the only declared dependency that covers libA.

[nojb]

@lthls : is that right?

Note quite. In your description here, you say that libC depends on libA and libB. In this case, the current state of the PR works: we only need libA, so we can remove libB as a dependency.

I don't know if I am missing something, but I don't see any difference between adding libA to the declared list of dependencies of libC or leaving it implicit. For future reference, here is the reproduction case:

• dune

  (library (name libA) (wrapped false) (modules a))
  (library (name libB) (wrapped false) (modules b) (libraries libA))
  (library (name libC) (wrapped false) (modules c) (libraries libA libB))
  

• a.ml

  let x = 42
  

• b.ml

  (* empty *)
  

• c.ml

  let x = A.x
  

We compile once (with the current tip of this branch a427786):

$ dune build

next, we touch b.ml: echo 'type t' > b.ml and recompile:

$ dune build --display short
    ocamldep .libB.objs/b.impl.d
      ocamlc .libB.objs/byte/b.{cmi,cmo,cmt}
      ocamlc libB.cma
      ocamlc .libC.objs/byte/c.{cmi,cmo,cmt} # <- should not be rebuilt!
    ocamlopt .libB.objs/native/b.{cmx,o}
    ocamlopt .libC.objs/native/c.{cmx,o}     # <- should not be rebuilt!
    ocamlopt libB.{a,cmxa}
    ocamlopt libB.cmxs

[lthls]

Ok, this is unexpected. That means there is a bug somewhere in this PR. I'm not sure why we didn't cover this case properly, but at this point I don't think we'll invest the time to fix it.

[rgrinberg]

Closing until someone decides to take over this PR.