Skip to content

Remove the build directory as soon as possible when installing a package - #6906

Merged
kit-ty-kate merged 1 commit into
ocaml:masterfrom
kit-ty-kate:rm-build-dir-asap
Jul 13, 2026
Merged

Remove the build directory as soon as possible when installing a package#6906
kit-ty-kate merged 1 commit into
ocaml:masterfrom
kit-ty-kate:rm-build-dir-asap

Conversation

@kit-ty-kate

@kit-ty-kate kit-ty-kate commented Apr 16, 2026

Copy link
Copy Markdown
Member

Fixes #5884
Queued on #6912

@kit-ty-kate

Copy link
Copy Markdown
Member Author

Tested over 40_000 times with great success together with #6440. Max disk + RAM usage goes from 4-10GB down to < 2GB per install.

@rjbou rjbou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Génial! lgtm on the idea.
tiny comment + it worth adding a bench if possible.

@@ -868,13 +876,7 @@ let parallel_apply t
| `Remove nv ->
OpamAction.cleanup_package_artefacts t nv
(* if reinstalled, only removes build dir *)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is tied to which line ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the one above. See b6ed206

Comment thread src/client/opamSolution.ml
@kit-ty-kate kit-ty-kate added the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Apr 28, 2026
@kit-ty-kate

Copy link
Copy Markdown
Member Author

tiny comment + it worth adding a bench if possible.

done. I've added the bench in #6912 and rebased this PR on top of it to check the result.

@kit-ty-kate
kit-ty-kate force-pushed the rm-build-dir-asap branch 3 times, most recently from 6adb851 to db02709 Compare May 1, 2026 14:24
@kit-ty-kate

Copy link
Copy Markdown
Member Author

Compared to baseline (#6912), the benchmark shows a 0.3% performance degradation of regular installs and a 5% performance improvement on IO-constrained filesystems (simulated as much as i could)

@kit-ty-kate
kit-ty-kate requested a review from rjbou May 1, 2026 17:23

@rjbou rjbou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a final check on the bench to rerun once rebased, and if it confirms it should be ok (there is no reason that the gain changes)

@rjbou

rjbou commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Compared to baseline (#6912), the benchmark shows a 0.3% performance degradation of regular installs

What is the reason of this degradation ?

@rjbou
rjbou force-pushed the rm-build-dir-asap branch from db02709 to ac57f09 Compare July 13, 2026 13:22
@kit-ty-kate

Copy link
Copy Markdown
Member Author

What is the reason of this degradation ?

rm of the build dir are sequential and block the next install when installing fast packages to build so it's very slightly slower when the builds are too fast

@kit-ty-kate kit-ty-kate added STATE: READY TO MERGE and removed PR: QUEUED Pending pull request, waiting for other work to be merged or closed labels Jul 13, 2026
@kit-ty-kate

Copy link
Copy Markdown
Member Author

Ignoring the build failure on cygwin (#7030)

Compared to baseline (#6912), the benchmark shows a 0.3% performance degradation of regular installs and a 5% performance improvement on IO-constrained filesystems (simulated as much as i could)

Latest diff is -0.1% and -1.3%, well within the margin of fluctuation. The main improvement is RAM usage over time anyway, not the performance improvement.

@kit-ty-kate
kit-ty-kate merged commit 4f27207 into ocaml:master Jul 13, 2026
40 of 41 checks passed
@kit-ty-kate
kit-ty-kate deleted the rm-build-dir-asap branch July 13, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

opam install should cleanup the build directory right after a successful install instead of waiting until the end of the command

2 participants