opam show: Reduce I/O when multiple versions of a package exist
#6818
+47
−23
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ed packages and their switch at once
'OpamGlobalState.fold_switches' and its derivatives are expensive functions in terms of I/O. In opam show, in particular, the switch selections were read for each version.
…installed packages and their switch at once
rjbou
commented
Nov 27, 2025
| ## List | ||
|
|
||
| ## Show | ||
| * Improve performance of `opam show` by reading switch selection only once instead of once per package-version [#6515 @dra27] |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| * Improve performance of `opam show` by reading switch selection only once instead of once per package-version [#6515 @dra27] | |
| * Improve performance of `opam show` by reading switch selection only once instead of once per package-version [#6818 @dra27] |
|
|
||
| ## opam-state | ||
| * `OpamRepositoryState.load_opams_from_diff` track added packages to avoid removing version-equivalent packages [#6774 @arozovyk fix #6754] | ||
| * `OpamGlobalState.all_installed_versions`: was added [#6515 @dra27] |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| * `OpamGlobalState.all_installed_versions`: was added [#6515 @dra27] | |
| * `OpamGlobalState.all_installed_versions`: was added [#6818 @dra27] |
| ## opam-core | ||
| * `OpamCmdliner` was added. It is accessible through a new `opam-core.cmdliner` sub-library [#6755 @kit-ty-kate] | ||
| * `OpamUrl`: rename and expose `local_path` as `looks_like_local_path` [#5979 @kit-ty-kate] | ||
| * `OpamCompat.Map.add_to_list`: was added [#6515 @dra27] |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| * `OpamCompat.Map.add_to_list`: was added [#6515 @dra27] | |
| * `OpamCompat.Map.add_to_list`: was added [#6818 @dra27] |
|
|
||
| (** Returns the map of installed instances of the package name towards the list | ||
| of switches they are installed in *) | ||
| val installed_versions: 'a global_state -> name -> switch list package_map |
Collaborator
Author
There was a problem hiding this comment.
Should we keep installed_versions? It is not used in the codebase, and sherlocode doesn't highlight any use
| gt OpamPackage.Set.empty | ||
|
|
||
| let installed_versions gt name = | ||
| let installed_versions_t gt ~cond = |
Collaborator
Author
There was a problem hiding this comment.
The last commit proposes a factorisation of the 2 functions
|
|
||
| let installed_versions gt name = | ||
| installed_versions_t gt | ||
| ~cond:(fun nv -> OpamPackage.Name.equal name (OpamPackage.name nv)) |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| ~cond:(fun nv -> OpamPackage.Name.equal name (OpamPackage.name nv)) | |
| ~cond:(fun nv -> OpamPackage.Name.equal name nv.name) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Extracted from #6515, authored by @dra27
I noticed with
opam show dunethat the switch selections of the switches are read in proportion to the number of versions being displayed. That creates both unnecessary I/O and GC pressure. [...] I'd countenance against hyping the performance improvement onopam showbecause it won't always show up that way!