Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the performance of opam update/init on Windows #5966

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

kit-ty-kate
Copy link
Member

@kit-ty-kate kit-ty-kate commented May 21, 2024

WIP attempt at (temporary) fixing #5741 by throwing more silicon^Wdomains at the problem while #5648 is being worked on.

This currently requires:

Current result:

  • On Linux: opam update default goes from 6 seconds to 4 seconds
  • On Windows: opam init --bare goes from 6 and a half minutes to 3 minutes exactly. Of that 3 minutes, 1 minute is unchanged and taken by the extraction of the tar.gz archive.

Partly inspired by NTFS really isn't that bad - Robert Collins (LCA 2020)

Related to #5591
Fixes #3171
Fixes #4455
Fixes #2442

@kit-ty-kate kit-ty-kate added PR: WIP Not for merge at this stage AREA: PERFORMANCE labels May 21, 2024
@kit-ty-kate kit-ty-kate removed the PR: WIP Not for merge at this stage label May 22, 2024
@kit-ty-kate kit-ty-kate marked this pull request as ready for review May 22, 2024 16:21
@kit-ty-kate
Copy link
Member Author

The PR is now fully working as far as i tested.
However given the untested nature of this change, its presence in such a central piece of code and its reliance on big changes in underlying software (opam-file-format) I don't think it should be in 2.2 but i think it a good place to start to think about:

I'll split this PR into several smaller ones once 2.2.0 is out.

Comment on lines +46 to 47
let split_url u =
let re =
Copy link
Member Author

Choose a reason for hiding this comment

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

actually it's much easier to do this using domain local storage

Suggested change
let split_url u =
let re =
let split_url =
let re =
Domain.DLS.new_key @@ fun () ->

@kit-ty-kate kit-ty-kate added this to the 2.4.0~alpha1 milestone Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AREA: PERFORMANCE PR: WIP Not for merge at this stage
Projects
None yet
2 participants