diff --git a/master_changes.md b/master_changes.md index 68d5e27b4c5..7ef1490b99c 100644 --- a/master_changes.md +++ b/master_changes.md @@ -26,6 +26,7 @@ users) ## Actions ## Install + * Remove the build directory as soon as possible when installing a package [#6906 @kit-ty-kate - fix #5884] ## Build (package) @@ -148,6 +149,7 @@ users) ## Benchmarks * Add an even larger real-world diff to benchmark `opam update` [#6567 @kit-ty-kate] + * Add a benchmark for `opam install` [#6912 @kit-ty-kate] ## Reftests ### Tests diff --git a/src/client/opamSolution.ml b/src/client/opamSolution.ml index 9696b657594..2ff232eb526 100644 --- a/src/client/opamSolution.ml +++ b/src/client/opamSolution.ml @@ -683,6 +683,14 @@ let parallel_apply t | Left conf -> add_to_install nv conf; store_time (); + if not OpamClientConfig.(!r.keep_build_dir) then begin + (* NOTE: build_dir as defined above is the + "build dir in case of --inplace" so != OpamPath.Switch.build *) + let build_dir = + OpamPath.Switch.build t.switch_global.root t.switch nv + in + OpamFilename.rmdir build_dir; + end; Done (`Successful (OpamPackage.Set.add nv installed, removed)) | Right exn -> store_time (); @@ -864,17 +872,13 @@ let parallel_apply t end; let cleanup_artefacts graph = + (* NOTE: build dir removal is done directly after install + instead of here, for performance reasons. *) PackageActionGraph.iter_vertex (function | `Remove nv -> OpamAction.cleanup_package_artefacts t nv (* if reinstalled, only removes build dir *) - | `Install nv -> - if not OpamClientConfig.(!r.keep_build_dir) then - let build_dir = - OpamPath.Switch.build t.switch_global.root t.switch nv - in - OpamFilename.rmdir build_dir - | `Build _ | `Fetch _ -> () + | `Build _ | `Fetch _ | `Install _ -> () | `Change _ | `Reinstall _ -> assert false) graph in diff --git a/tests/bench/bench.ml b/tests/bench/bench.ml index c7b155144a9..d8ddd72ac2c 100644 --- a/tests/bench/bench.ml +++ b/tests/bench/bench.ml @@ -136,6 +136,37 @@ let () = in List.fold_left (+.) 0.0 l /. float_of_int n in + let time_install_core = + Gc.compact (); + launch (fmt "%s switch create -y eight ocaml-base-compiler.4.14.0" bin); + launch (fmt "%s install --download-only -y core.v0.15.1" bin); + launch "test -d ~/.opam/eight/.opam-switch/sources && rm -rf ~/.opam/eight/.opam-switch/sources"; + launch "test -d ~/.opam/eight/.opam-switch/build && rm -rf ~/.opam/eight/.opam-switch/build"; + time_cmd ~exit:0 (fmt "%s install -y core.v0.15.1" bin) + in + let time_install_big = + Gc.compact (); + launch (fmt "%s switch create nine --empty" bin); + OpamSystem.write "/tmp/opam-bench-bigpkg/repo" {|opam-version: "2.0" +|}; + OpamSystem.write "/tmp/opam-bench-bigpkg/packages/a.1/opam" {|opam-version: "2.0" +url { + src: "https://github.com/ocaml/opam-repository/archive/refs/tags/2025-01-before-archiving-phase1.tar.gz" + checksum: "md5=215a5261ad8358e34eb3222b83c4c8be" +} +|}; + OpamSystem.write "/tmp/opam-bench-bigpkg/packages/b.1/opam" {|opam-version: "2.0" +build: ["rm" "-r" "packages"] +url { + src: "https://github.com/ocaml/opam-repository/archive/refs/tags/2025-01-before-archiving-phase1.tar.gz" + checksum: "md5=215a5261ad8358e34eb3222b83c4c8be" +} +|}; + launch (fmt "%s repo add tmp /tmp/opam-bench-bigpkg" bin); + let res = time_cmd ~exit:0 (fmt "%s install a.1 b.1" bin) in + launch (fmt "%s repo remove tmp -a" bin); + res + in let init_root tmp_root_dir repo = launch (fmt "rm -rf %s" tmp_root_dir); launch (fmt "mkdir -p %s" tmp_root_dir); @@ -279,6 +310,16 @@ let () = "name": "OpamStd.String.split amortised over 10 runs", "value": %f, "units": "secs" + }, + { + "name": "opam install core.v0.15.1", + "value": %f, + "units": "secs" + }, + { + "name": "opam install large-pkg large-pkg2", + "value": %f, + "units": "secs" } ] }, @@ -363,6 +404,8 @@ let () = time_show_raw time_show_precise time_OpamStd_String_split_10 + time_install_core + time_install_big time_update_no_diff_local time_update_no_diff_git time_update_small_diff_local