A few performance improvements - #6515
Merged
Merged
Conversation
kit-ty-kate
reviewed
Jul 7, 2025
kit-ty-kate
force-pushed
the
easy-wins
branch
2 times, most recently
from
October 13, 2025 19:43
eab0cef to
9fdc865
Compare
kit-ty-kate
approved these changes
Oct 13, 2025
dra27
commented
Oct 14, 2025
Gc.ramp_up is available since OCaml 5.4.0
rjbou
approved these changes
Nov 27, 2025
rjbou
left a comment
Collaborator
There was a problem hiding this comment.
I've rebased the PR, extracted opam show improvement in its own PR (to keep this PR for core changes), and split commits that concerns functions additions.
LGTM!
kit-ty-kate
force-pushed
the
easy-wins
branch
from
November 28, 2025 14:25
3842734 to
dc82dd8
Compare
Member
|
Thanks a lot! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Our version comparison function is very hot. While looking around it in the context of #4245, I picked up on a few easy wins. These are some basic results from hyperfine on my system for
opam show dune, but they must be taken with an appropriate amount of salt:The interesting part is that on OCaml 5.4,
opam showspends almost its entire time unmarshalling the repository state cache. However, these changes shouldn't be assessed in terms of performance improvement because what's happening on all versions of OCaml is a reduction in GC pressure (i.e. if opam were doing a bit more work, the GC would almost certainly run again and the time reduction wouldn't be as stark). The three changes are:Gc.ramp_upfor the unmarshalling call inOpamCached. This is logically sensible, as the caches are never collected. The speedup comes from the fact that the major GC doesn't actually have to run any more in theopam showcommand, and the 5.3.0 and 4.14.2 results for that commit are well within the noise of the microbenchmark.OpamStd.String.compare_caseadded in Optimise package name comparison #4328 contained a small mistake - the closure allocation for theauxfunction shows up in flame graphs. The derivation of sets of packages from the map of opams in OpamRepositoryState (which is also done in OpamSwitchState) is mostly dominated byOpamVersionCompare, but eliminating the closure incompare_casehas a benefit (as it happens, I also got a small benefit from a Hacker's Delight-style optimisation noting that opam package names can't contain certain characters, but as the function is also used for system package names, I decided not to commit that one!). The diff looks noisy without space-change being ignored - all I've done is move theauxfunction out ofcompare_case.Finally, I noticed withextracted inopam 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.opam show: Reduce I/O when multiple versions of a package exist #6818I think all three of these changes are worth putting in, but I'd countenance against hyping the performance improvement on
opam showbecause it won't always show up that way!