feat: per-module inter-library dependency filtering (#4572)

doc/changes/added/14116.md

@@ -0,0 +1,6 @@
+- Use `ocamldep` output to filter inter-library dependencies on a
+  per-module basis. Eliminates unnecessary recompilations when a
+  dependency library changes but the importing module doesn't
+  reference it, and, for unwrapped dependency libraries, when the
+  importing module references a different module of the same
+  library than the one that changed. (#14116, fixes #4572, @robinbb)

src/dune_rules/compilation_context.ml

@@ -4,55 +4,29 @@ open Memo.O
 module Includes = struct
   type t = Command.Args.without_targets Command.Args.t Lib_mode.Cm_kind.Map.t
 
-  let make ~project ~opaque ~direct_requires ~hidden_requires lib_config
+  let make ~project ~direct_requires ~hidden_requires lib_config
     : _ Lib_mode.Cm_kind.Map.t
     =
-    (* TODO: some of the requires can filtered out using [ocamldep] info *)
     let open Resolve.Memo.O in
     let iflags direct_libs hidden_libs mode =
       Lib_flags.L.include_flags ~project ~direct_libs ~hidden_libs mode lib_config
     in
-    let make_includes_args ~mode groups =
+    let make_includes_args ~mode =
       (let+ direct_libs = direct_requires
        and+ hidden_libs = hidden_requires in
-       Command.Args.S
-         [ iflags direct_libs hidden_libs mode
-         ; Hidden_deps (Lib_file_deps.deps (direct_libs @ hidden_libs) ~groups)
-         ])
+       iflags direct_libs hidden_libs mode)
       |> Resolve.Memo.args
       |> Command.Args.memo
     in
     { ocaml =
-        (let cmi_includes = make_includes_args ~mode:(Ocaml Byte) [ Ocaml Cmi ] in
+        (let cmi_includes = make_includes_args ~mode:(Ocaml Byte) in
          { cmi = cmi_includes
          ; cmo = cmi_includes
-         ; cmx =
-             (let+ direct_libs = direct_requires
-              and+ hidden_libs = hidden_requires in
-              Command.Args.S
-                [ iflags direct_libs hidden_libs (Ocaml Native)
-                ; Hidden_deps
-                    (let libs = direct_libs @ hidden_libs in
-                     if opaque
-                     then
-                       List.map libs ~f:(fun lib ->
-                         ( lib
-                         , if Lib.is_local lib
-                           then [ Lib_file_deps.Group.Ocaml Cmi ]
-                           else [ Ocaml Cmi; Ocaml Cmx ] ))
-                       |> Lib_file_deps.deps_with_exts
-                     else
-                       Lib_file_deps.deps
-                         libs
-                         ~groups:[ Lib_file_deps.Group.Ocaml Cmi; Ocaml Cmx ])
-                ])
-             |> Resolve.Memo.args
-             |> Command.Args.memo
+         ; cmx = make_includes_args ~mode:(Ocaml Native)
          })
     ; melange =
-        { cmi = make_includes_args ~mode:Melange [ Melange Cmi ]
-        ; cmj = make_includes_args ~mode:Melange [ Melange Cmi; Melange Cmj ]
-        }
+        (let mel_includes = make_includes_args ~mode:Melange in
+         { cmi = mel_includes; cmj = mel_includes })
     }
   ;;
 
@@ -91,6 +65,8 @@ type t =
   ; parameters : Module_name.t list Resolve.Memo.t
   ; instances : Parameterised_instances.t Resolve.Memo.t option
   ; includes : Includes.t
+  ; lib_index : Lib_file_deps.Lib_index.t Resolve.t Memo.Lazy.t
+  ; has_virtual_impl : bool Resolve.t Memo.Lazy.t
   ; preprocessing : Pp_spec.t
   ; opaque : bool
   ; js_of_ocaml : Js_of_ocaml.In_context.t option Js_of_ocaml.Mode.Pair.t
@@ -118,6 +94,8 @@ let requires_hidden t = t.requires_hidden
 let requires_link t = Memo.Lazy.force t.requires_link
 let parameters t = t.parameters
 let includes t = t.includes
+let lib_index t = Memo.Lazy.force t.lib_index
+let has_virtual_impl t = Memo.Lazy.force t.has_virtual_impl
 let preprocessing t = t.preprocessing
 let opaque t = t.opaque
 let js_of_ocaml t = t.js_of_ocaml
@@ -147,6 +125,72 @@ let parameters_main_modules parameters =
         [ "param", Lib.to_dyn param ])
 ;;
 
+(* Build a [Lib_index] from [all_libs] for the per-module inter-library
+   dependency filter.
+
+   For each library, entry module names are collected (for wrapped
+   libraries, the wrapper; for unwrapped, each public module). Hidden
+   libraries are included so that consumers which transitively
+   reference a hidden library's entry module still produce a build
+   dependency on it.
+
+   Local libraries whose ocamldep is short-circuited by
+   [Dep_rules.skip_ocamldep] (unwrapped single-module stanzas without
+   direct lib deps) are collected into [no_ocamldep] so the cross-
+   library walk does not try to read their nonexistent [.d] files. *)
+(* [libs] should be the consumer's [direct_requires] — the libraries
+   whose entry modules can legitimately appear in the consumer's
+   ocamldep output (and, after the cross-library walk extends through
+   them, in cross-library ocamldep outputs that the BFS surfaces).
+   Hidden requires are intentionally excluded: the user has not
+   committed to them as direct dependencies, so the per-module
+   filter does not track them with per-module precision — they
+   fall to glob fallback in [lib_deps_for_module]'s post-walk
+   classification, which is the right behaviour for libs outside
+   the user's commitment surface. *)
+let build_lib_index ~super_context ~libs ~for_ =
+  let open Resolve.Memo.O in
+  let+ per_lib =
+    Resolve.Memo.List.map libs ~f:(fun lib ->
+      match Lib_info.entry_modules (Lib.info lib) ~for_ with
+      | External (Ok names) ->
+        Resolve.Memo.return (List.map names ~f:(fun n -> n, lib, None), None)
+      | External (Error e) -> Resolve.Memo.of_result (Error e)
+      | Local ->
+        Resolve.Memo.lift_memo
+          (Memo.map
+             (Dir_contents.modules_of_local_lib
+                super_context
+                (Lib.Local.of_lib_exn lib)
+                ~for_)
+             ~f:(fun mods ->
+               (* [Some m] iff the lib is tight-eligible (local + unwrapped):
+                  only then can downstream consumers issue per-module deps
+                  on its [.cmi] files. [None] therefore covers both wrapped
+                  locals and externals — in either case we don't have a
+                  [Module.t] the per-module path can use. *)
+               let unwrapped =
+                 match Lib_info.wrapped (Lib.info lib) with
+                 | Some (This w) -> not (Wrapped.to_bool w)
+                 | Some (From _) | None -> false
+               in
+               let entries =
+                 List.map (Modules.entry_modules mods) ~f:(fun m ->
+                   Module.name m, lib, if unwrapped then Some m else None)
+               in
+               let no_ocamldep_lib =
+                 match Modules.as_singleton mods with
+                 | Some _ when List.is_empty (Lib_info.requires (Lib.info lib) ~for_) ->
+                   Some lib
+                 | _ -> None
+               in
+               entries, no_ocamldep_lib)))
+  in
+  let entries = List.concat_map per_lib ~f:fst in
+  let no_ocamldep = List.filter_map per_lib ~f:snd |> Lib.Set.of_list in
+  Lib_file_deps.Lib_index.create ~no_ocamldep entries
+;;
+
 let create
       ~super_context
       ~scope
@@ -206,6 +250,23 @@ let create
     let profile = Context.profile context in
     eval_opaque ocaml profile opaque
   in
+  let* has_library_deps =
+    (* Determine whether any library dependencies are declared, so that
+       single-module stanzas still run ocamldep when its output could
+       inform the per-module inter-library dependency filter. *)
+    let open Resolve.Memo.O in
+    let+ direct = direct_requires
+    and+ hidden = hidden_requires in
+    match direct, hidden with
+    | [], [] -> false
+    | _ -> true
+  in
+  let has_library_deps =
+    (* Unresolved dependency errors propagate later through the normal
+       compilation rules; here we conservatively behave as if libraries
+       are present. *)
+    Resolve.peek has_library_deps |> Result.value ~default:true
+  in
   let+ dep_graphs =
     Dep_rules.rules
       ~dir:(Obj_dir.dir obj_dir)
@@ -215,6 +276,7 @@ let create
       ~impl:implements
       ~modules
       ~for_
+      ~has_library_deps
   and+ bin_annot =
     match bin_annot with
     | Some b -> Memo.return b
@@ -240,8 +302,18 @@ let create
   ; requires_link
   ; implements
   ; parameters
-  ; includes =
-      Includes.make ~project ~opaque ~direct_requires ~hidden_requires ocaml.lib_config
+  ; includes = Includes.make ~project ~direct_requires ~hidden_requires ocaml.lib_config
+  ; lib_index =
+      Memo.lazy_ (fun () ->
+        let open Resolve.Memo.O in
+        let* libs = direct_requires in
+        build_lib_index ~super_context ~libs ~for_)
+  ; has_virtual_impl =
+      Memo.lazy_ (fun () ->
+        let open Resolve.Memo.O in
+        let+ direct = direct_requires
+        and+ hidden = hidden_requires in
+        List.exists (direct @ hidden) ~f:(fun lib -> Option.is_some (Lib.implements lib)))
   ; preprocessing
   ; opaque
   ; js_of_ocaml
@@ -333,7 +405,6 @@ let for_module_generated_at_link_time cctx ~requires ~module_ =
     let direct_requires = requires in
     Includes.make
       ~project:(Scope.project cctx.scope)
-      ~opaque
       ~direct_requires
       ~hidden_requires
       cctx.ocaml.lib_config
@@ -344,6 +415,7 @@ let for_module_generated_at_link_time cctx ~requires ~module_ =
   ; requires_link = Memo.lazy_ (fun () -> requires)
   ; requires_compile = requires
   ; includes
+  ; lib_index = Memo.lazy_ (fun () -> Resolve.Memo.return Lib_file_deps.Lib_index.empty)
   ; modules
   }
 ;;

src/dune_rules/compilation_context.mli

@@ -62,6 +62,13 @@ val requires_hidden : t -> Lib.t list Resolve.Memo.t
 val requires_compile : t -> Lib.t list Resolve.Memo.t
 val parameters : t -> Module_name.t list Resolve.Memo.t
 val includes : t -> Command.Args.without_targets Command.Args.t Lib_mode.Cm_kind.Map.t
+val lib_index : t -> Lib_file_deps.Lib_index.t Resolve.Memo.t
+
+(** [true] iff any library in the compilation context's direct or
+    hidden requires implements a virtual library. Memoized per
+    cctx; callers avoid re-scanning the lib list on each module. *)
+val has_virtual_impl : t -> bool Resolve.Memo.t
+
 val preprocessing : t -> Pp_spec.t
 val opaque : t -> bool
 val js_of_ocaml : t -> Js_of_ocaml.In_context.t option Js_of_ocaml.Mode.Pair.t

src/dune_rules/dep_graph.ml

@@ -7,6 +7,8 @@ type t =
   }
 
 let make ~dir ~per_module = { dir; per_module }
+let dir t = t.dir
+let mem t (m : Module.t) = Module_name.Unique.Map.mem t.per_module (Module.obj_name m)
 
 let deps_of t (m : Module.t) =
   match Module_name.Unique.Map.find t.per_module (Module.obj_name m) with

src/dune_rules/dep_graph.mli

@@ -9,6 +9,8 @@ val make
   -> per_module:Module.t list Action_builder.t Module_name.Unique.Map.t
   -> t
 
+val dir : t -> Path.Build.t
+val mem : t -> Module.t -> bool
 val deps_of : t -> Module.t -> Module.t list Action_builder.t
 val top_closed_implementations : t -> Module.t list -> Module.t list Action_builder.t
 

src/dune_rules/dep_rules.ml

@@ -131,9 +131,30 @@ let deps_of_vlib_module ~obj_dir ~vimpl ~dir ~sctx ~ml_kind ~for_ sourced_module
     Ocamldep.read_deps_of ~obj_dir:vlib_obj_dir ~modules ~ml_kind ~for_ m
 ;;
 
-(** Tests whether a set of modules is a singleton *)
+(** Tests whether a set of modules is a singleton. *)
 let has_single_file modules = Option.is_some @@ Modules.With_vlib.as_singleton modules
 
+(** Tests whether ocamldep can be short-circuited for [modules]: true
+    for single-module stanzas that have no library dependencies.
+    The premise — "no consumer of ocamldep output can benefit" — was
+    valid before #4572; under that PR, cross-library consumers now
+    read a target library's ocamldep output as part of the
+    per-module inter-library dependency filter. Libraries identified
+    here must also be identified in
+    [Compilation_context.build_lib_index]'s [no_ocamldep_lib] check
+    so the cross-library walk knows their [.d] files will be
+    missing. Keeping the two in sync is fragile.
+
+    TODO: unify into a single predicate that both call sites
+    consult, so future changes to the condition can't drift. The
+    cleanest shape is probably to retire this short-circuit for
+    library stanzas entirely — libraries that could be consumed
+    cross-stanza should always run ocamldep — and keep the
+    optimisation only for executable/test stanzas. *)
+let skip_ocamldep ~has_library_deps modules =
+  has_single_file modules && not has_library_deps
+;;
+
 let rec deps_of
           ~obj_dir
           ~modules
@@ -143,6 +164,7 @@ let rec deps_of
           ~sctx
           ~ml_kind
           ~for_
+          ~has_library_deps
           (m : Modules.Sourced_module.t)
   =
   let is_alias_or_root =
@@ -153,7 +175,7 @@ let rec deps_of
        | Root | Alias _ -> true
        | _ -> false)
   in
-  if is_alias_or_root || has_single_file modules
+  if is_alias_or_root || skip_ocamldep ~has_library_deps modules
   then Memo.return (Action_builder.return [])
   else (
     let skip_if_source_absent f sourced_module =
@@ -173,14 +195,18 @@ let rec deps_of
         (deps_of_module ~modules ~sandbox ~sctx ~dir ~obj_dir ~ml_kind ~for_)
         m
     | Impl_of_virtual_module impl_or_vlib ->
-      deps_of ~obj_dir ~modules ~sandbox ~impl ~dir ~sctx ~ml_kind ~for_
+      deps_of ~obj_dir ~modules ~sandbox ~impl ~dir ~sctx ~ml_kind ~for_ ~has_library_deps
       @@
       let m = Ml_kind.Dict.get impl_or_vlib ml_kind in
       (match ml_kind with
        | Intf -> Imported_from_vlib m
        | Impl -> Normal m))
 ;;
 
+(* [read_deps_of_module] reports intra-stanza module dependencies. For
+   single-module stanzas that dependency graph is trivially empty
+   regardless of whether the stanza declares library dependencies, so we
+   keep the unconditional short-circuit here. *)
 let read_deps_of_module ~modules ~obj_dir dep ~for_ =
   let (Obj_dir.Module.Dep.Immediate (unit, _) | Transitive (unit, _)) = dep in
   match Module.kind unit with
@@ -219,18 +245,37 @@ let dict_of_func_concurrently f =
 
 let for_module ~obj_dir ~modules ~sandbox ~impl ~dir ~sctx ~for_ module_ =
   dict_of_func_concurrently
-    (deps_of ~obj_dir ~modules ~sandbox ~impl ~dir ~sctx ~for_ (Normal module_))
+    (deps_of
+       ~obj_dir
+       ~modules
+       ~sandbox
+       ~impl
+       ~dir
+       ~sctx
+       ~for_
+       ~has_library_deps:true
+       (Normal module_))
 ;;
 
-let rules ~obj_dir ~modules ~sandbox ~impl ~sctx ~dir ~for_ =
+let rules ~obj_dir ~modules ~sandbox ~impl ~sctx ~dir ~for_ ~has_library_deps =
   match Modules.With_vlib.as_singleton modules with
-  | Some m -> Memo.return (Dep_graph.Ml_kind.dummy m)
-  | None ->
+  | Some m when not has_library_deps -> Memo.return (Dep_graph.Ml_kind.dummy m)
+  | Some _ | None ->
     dict_of_func_concurrently (fun ~ml_kind ->
       let+ per_module =
         Modules.With_vlib.obj_map modules
         |> Parallel_map.parallel_map ~f:(fun _obj_name m ->
-          deps_of ~obj_dir ~modules ~sandbox ~impl ~sctx ~dir ~ml_kind ~for_ m)
+          deps_of
+            ~obj_dir
+            ~modules
+            ~sandbox
+            ~impl
+            ~sctx
+            ~dir
+            ~ml_kind
+            ~for_
+            ~has_library_deps
+            m)
       in
       Dep_graph.make ~dir ~per_module)
     |> Memo.map ~f:(Dep_graph.Ml_kind.for_module_compilation ~modules)

src/dune_rules/dep_rules.mli

@@ -13,6 +13,12 @@ val for_module
   -> Module.t
   -> Module.t list Action_builder.t Ml_kind.Dict.t Memo.t
 
+(** [has_library_deps] indicates whether the enclosing stanza declares any
+    library dependencies. When false, single-module stanzas short-circuit
+    ocamldep entirely (no build rule, no [.d]/[.all-deps] file). When
+    true, single-module stanzas run ocamldep so that the per-module
+    inter-library dependency filter can determine which libraries the
+    single module references. *)
 val rules
   :  obj_dir:Path.Build.t Obj_dir.t
   -> modules:Modules.With_vlib.t
@@ -21,6 +27,7 @@ val rules
   -> sctx:Super_context.t
   -> dir:Path.Build.t
   -> for_:Compilation_mode.t
+  -> has_library_deps:bool
   -> Dep_graph.Ml_kind.t Memo.t
 
 val read_immediate_deps_of