From 9f19f154f3acd729c6f5830723509825876f0804 Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Tue, 29 Nov 2022 15:40:48 +0100 Subject: [PATCH 1/3] target hint: only load targets from subdir This is an optimization: instead of loading all targets and filtering, we start with only the right subdirectory. Signed-off-by: Etienne Millon --- bin/print_rules.ml | 2 +- bin/target.ml | 18 +++++++++++------- src/dune_engine/load_rules.ml | 7 +++++-- src/dune_engine/load_rules.mli | 8 ++++++-- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/bin/print_rules.ml b/bin/print_rules.ml index cee1e57e21f..d05f9f8c121 100644 --- a/bin/print_rules.ml +++ b/bin/print_rules.ml @@ -119,7 +119,7 @@ let term = let* request = match targets with | [] -> - Load_rules.all_direct_targets () + Load_rules.all_direct_targets None >>| Path.Build.Map.foldi ~init:[] ~f:(fun p _ acc -> Path.build p :: acc) >>| Action_builder.paths diff --git a/bin/target.ml b/bin/target.ml index ad57dbe3847..b2e451b7ad5 100644 --- a/bin/target.ml +++ b/bin/target.ml @@ -23,15 +23,19 @@ let target_hint (_setup : Dune_rules.Main.build_system) path = assert (Path.is_managed path); let open Memo.O in let sub_dir = Option.value ~default:path (Path.parent path) in - (* CR-someday amokhov: There are two issues with the code below. + (* CR-someday amokhov: - (1) We first get *all* targets but then filter out only those that are - defined in the [sub_dir]. It would be better to just get the targets for - the [sub_dir] directly (the API supports this). - - (2) We currently provide the same hint for all targets. It would be nice to + We currently provide the same hint for all targets. It would be nice to indicate whether a hint corresponds to a file or to a directory target. *) - let+ candidates = Load_rules.all_direct_targets () >>| Path.Build.Map.keys in + let root = + match sub_dir with + | External _ -> (* checked above *) assert false + | In_source_tree d -> d + | In_build_dir d -> Path.Build.drop_build_context_exn d + in + let+ candidates = + Load_rules.all_direct_targets (Some root) >>| Path.Build.Map.keys + in let candidates = if Path.is_in_build_dir path then List.map ~f:Path.build candidates else diff --git a/src/dune_engine/load_rules.ml b/src/dune_engine/load_rules.ml index 685eebe797c..10cdb7bc048 100644 --- a/src/dune_engine/load_rules.ml +++ b/src/dune_engine/load_rules.ml @@ -918,8 +918,11 @@ end module Source_tree_map_reduce = Source_tree.Dir.Make_map_reduce (Memo) (All_targets) -let all_direct_targets () = - let* root = Source_tree.root () +let all_direct_targets dir = + let* root = + match dir with + | None -> Source_tree.root () + | Some dir -> Source_tree.nearest_dir dir and* contexts = Memo.Lazy.force (Build_config.get ()).contexts in Memo.parallel_map (Context_name.Map.values contexts) ~f:(fun ctx -> Source_tree_map_reduce.map_reduce root ~traverse:Sub_dirs.Status.Set.all diff --git a/src/dune_engine/load_rules.mli b/src/dune_engine/load_rules.mli index 57deaa73606..91ac875758d 100644 --- a/src/dune_engine/load_rules.mli +++ b/src/dune_engine/load_rules.mli @@ -69,8 +69,12 @@ val is_target : Path.t -> is_target Memo.t val is_under_directory_target : Path.t -> bool Memo.t (** List of all buildable direct targets. This does not include files and - directory produced under a directory target. *) -val all_direct_targets : unit -> target_type Path.Build.Map.t Memo.t + directory produced under a directory target. + + If argument is [None], load the root, otherwise only load targets from the + nearest subdirectory. *) +val all_direct_targets : + Path.Source.t option -> target_type Path.Build.Map.t Memo.t type rule_or_source = | Source of Digest.t From e9222b8d6e50213b989b9249194fcb1b157b8845 Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Mon, 5 Dec 2022 14:27:54 +0100 Subject: [PATCH 2/3] Use a Code_error Signed-off-by: Etienne Millon --- bin/target.ml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bin/target.ml b/bin/target.ml index b2e451b7ad5..0b53b57b4d5 100644 --- a/bin/target.ml +++ b/bin/target.ml @@ -20,7 +20,6 @@ let request targets = | Alias a -> Alias.request a) let target_hint (_setup : Dune_rules.Main.build_system) path = - assert (Path.is_managed path); let open Memo.O in let sub_dir = Option.value ~default:path (Path.parent path) in (* CR-someday amokhov: @@ -29,7 +28,9 @@ let target_hint (_setup : Dune_rules.Main.build_system) path = indicate whether a hint corresponds to a file or to a directory target. *) let root = match sub_dir with - | External _ -> (* checked above *) assert false + | External e -> + Code_error.raise "target_hint: external path" + [ ("path", Path.External.to_dyn e) ] | In_source_tree d -> d | In_build_dir d -> Path.Build.drop_build_context_exn d in From 15a99815e7b6ed6d0b14c9528ff70fcf91ccbb23 Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Mon, 5 Dec 2022 14:28:06 +0100 Subject: [PATCH 3/3] fix typo Signed-off-by: Etienne Millon --- src/dune_engine/load_rules.mli | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dune_engine/load_rules.mli b/src/dune_engine/load_rules.mli index 91ac875758d..ff43af7e4b7 100644 --- a/src/dune_engine/load_rules.mli +++ b/src/dune_engine/load_rules.mli @@ -69,7 +69,7 @@ val is_target : Path.t -> is_target Memo.t val is_under_directory_target : Path.t -> bool Memo.t (** List of all buildable direct targets. This does not include files and - directory produced under a directory target. + directories produced under a directory target. If argument is [None], load the root, otherwise only load targets from the nearest subdirectory. *)