Perf: Speed up solver by parsonsmatt · Pull Request #11566 · haskell/cabal

parsonsmatt · 2026-03-03T06:08:06Z

This PR speeds up the solver part of cabal. On our codebase, this reduces solver time from 5.7s to 4.1s.

Please read Github PR Conventions and then fill in one of these two templates.

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

Patches conform to the coding conventions.
Any changes that could be relevant to users have been recorded in the changelog.
- Is the change significant? If so, remember to add significance: significant in the changelog file.
The documentation has been updated, if necessary.
Manual QA notes have been included.
Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

Patches conform to the coding conventions.
Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

parsonsmatt

Running locally, I'm seeing solver times between 3.75 and 4s with this patch, and 5-6.24s without. Pretty solid improvement, and nothing terribly complex for it.

parsonsmatt · 2026-03-03T06:11:25Z

cabal-install-solver/src/Distribution/Solver/Modular/ConflictSet.hs

+-- | Version range with an 'Ord' instance. The show string is cached to avoid
+-- recomputing it on every comparison.
+data OrderedVersionRange = OVR !String !VR


This change is likely to reduce allocations at the cost of greater resident memory - rather than generating and throwing away the strings for every compare, we're holding on to them.

Could we use Distribution.Utils.ShortText instead of String? This should reduce resident memory and make instance Ord OrderedVersionRange even faster.

🚀 heck yes 🚀

cabal-install-solver/src/Distribution/Solver/Modular/Assignment.hs

parsonsmatt · 2026-03-03T06:13:05Z

cabal-install-solver/src/Distribution/Solver/Modular/ConflictSet.hs

 instance Ord OrderedVersionRange where
-  compare = compare `on` show


funny - because we'd be doing "OrderedVersionRange ..." as the prefix of every string, a Set ORderedVersionRange would be a linked list for that prefix. This is delegating to show on the VR which may be similarly disadvantaged, but at least we're caching

It would be, but a drop 20 on the String would resolve that cheaply?

parsonsmatt · 2026-03-03T06:13:40Z

cabal-install-solver/src/Distribution/Solver/Modular/Explore.hs

+    qo = defaultQualifyOptions idx
+
    couldResolveConflicts :: QPN -> POption -> S.Set CS.Conflict -> Maybe ConflictSet
    couldResolveConflicts currentQPN@(Q _ pn) (POption i@(I v _) _) conflicts =
      let (PInfo deps _ _ _) = idx M.! pn M.! i
-          qdeps = qualifyDeps (defaultQualifyOptions idx) currentQPN deps
+          qdeps = qualifyDeps qo currentQPN deps


qo shared between invocations- possibly not required with let floating but 🤷🏻

parsonsmatt · 2026-03-03T06:14:30Z

cabal-install-solver/src/Distribution/Solver/Modular/Validate.hs

@@ -301,7 +301,7 @@ checkComponentsInNewPackage :: ComponentDependencyReasons
                            -> Map ExposedComponent ComponentInfo
                            -> Either Conflict ()
 checkComponentsInNewPackage required qpn providedComps =
-    case M.toList $ deleteKeys (M.keys providedComps) required of
+    case M.toList $ M.difference required providedComps of


should be algorithmically better here

Bodigrim

Exciting!

Bodigrim · 2026-03-03T22:12:25Z

cabal-install-solver/src/Distribution/Solver/Modular/ConflictSet.hs

+-- | Version range with an 'Ord' instance. The show string is cached to avoid
+-- recomputing it on every comparison.
+data OrderedVersionRange = OVR !String !VR


Could we use Distribution.Utils.ShortText instead of String? This should reduce resident memory and make instance Ord OrderedVersionRange even faster.

cabal-install-solver/src/Distribution/Solver/Modular/Explore.hs

geekosaur · 2026-03-04T19:46:56Z

cabal-install-solver/src/Distribution/Solver/Modular/ConflictSet.hs

 instance Ord OrderedVersionRange where
-  compare = compare `on` show


It would be, but a drop 20 on the String would resolve that cheaply?

sebright · 2026-03-05T00:30:24Z

Is there an example of a package on Hackage that shows the performance improvement?

sebright

I tried running the solver benchmarks with some packages from Hackage, but I wasn't able to find any that showed a significant performance improvement. What are the characteristics of the packages that showed the improvement? Was there any backtracking in the run that you timed?

EDIT: I'm also wondering if the solver had to skip many versions, with lines containing "has the same characteristics that caused the previous version to fail". The backtracking and conflicts are only shown with -v3.

sebright · 2026-03-05T08:40:02Z

cabal-install-solver/src/Distribution/Solver/Modular/ConflictSet.hs

-- | Version range with an 'Ord' instance.
-newtype OrderedVersionRange = OrderedVersionRange VR
-  deriving (Eq, Show)
+-- | Version range with an 'Ord' instance. The show string is cached to avoid
+-- recomputing it on every comparison.
+data OrderedVersionRange = OVR !ShortText !VR


It looks like #8269 added an Ord instance to VersionRange, so OrderedVersionRange could be removed, unless it leads to a significant regression.

I would be surprised if it is a significant difference either way - compare on VersionRange may involve more pointer chasing, particularly on Union or Intersect cases, but compare @Version I'd expect to be very fast in the PV0 case, which may overcome the textual prefix comparison. Removing OrderedVersionRange feels more invasive than optimizing it transparently.

sebright · 2026-03-05T08:41:55Z

cabal-install-solver/src/Distribution/Solver/Modular/Explore.hs

@@ -267,24 +267,31 @@ exploreLog mbj enableBj fineGrainedConflicts (CountConflicts countConflicts) idx
    -- is true, because it is always safe to explore a package instance.
    -- Skipping it is an optimization. If false, it returns a new conflict set
    -- to be merged with the previous one.
+    qo = defaultQualifyOptions idx
+
    couldResolveConflicts :: QPN -> POption -> S.Set CS.Conflict -> Maybe ConflictSet


The comment should be above couldResolveConflicts.

sebright · 2026-03-05T09:15:35Z

cabal-install-solver/src/Distribution/Solver/Modular/Explore.hs

+          -- Pre-index: map from QPN to intersected version range (Constrained only)
+          depVRs :: M.Map QPN VR
+          depVRs = M.fromListWith (.&&.)
+            [ (qpn, case ci of Constrained vr -> vr; _ -> anyVersion)
+            | Simple (LDep _ (Dep (PkgComponent qpn _) ci)) _ <- qdeps ]


It's a little confusing that the alternative to Constrained, Fixed, is treated as anyVersion. Could this cache be split into two variables, one specialized for each of its two use cases, to improve readability?

Sure - since we only use the M.member function, I can do M.keysSet here to differentitae the use sites.

sebright · 2026-03-05T09:18:27Z

cabal-install-solver/src/Distribution/Solver/Modular/Validate.hs

@@ -301,7 +301,7 @@ checkComponentsInNewPackage :: ComponentDependencyReasons
                            -> Map ExposedComponent ComponentInfo
                            -> Either Conflict ()
 checkComponentsInNewPackage required qpn providedComps =
-    case M.toList $ deleteKeys (M.keys providedComps) required of
+    case M.toList $ M.difference required providedComps of


sebright

The way that I was thinking about this PR is that each change could be justified as either a code cleanup or a performance improvement. Since I can't reproduce the performance improvement, I was reviewing each of the changes as a cleanup. That's why I suggested simplifying the code by removing the OrderedVersionRange type and splitting the depVRs cache.

I'm also interested in looking into why the solver ran for so long with the packages that you mentioned. It might help to open a separate issue if there is a publicly available reproducer. I hadn't realized that the code for skipping conflicting packages could be so important for performance. It's possible that the use case could be optimized in a different way, such as adding a new case for skipping packages (constructor for CS.Conflict) or avoiding the conflicts entirely. If there's no good way to avoid skipping so many packages, and it's a common use case, then maybe we could find a way to add it to the test suite.

sebright · 2026-03-10T23:22:22Z

cabal-install-solver/src/Distribution/Solver/Modular/Explore.hs

+          depsWithVRs :: S.Set QPN
+          depsWithVRs = M.keysSet depVRs


This variable can just be called deps now, because it doesn't matter whether the dependencies are constrained for resolving CS.GoalConflict.

sebright · 2026-03-10T23:25:50Z

cabal-install-solver/src/Distribution/Solver/Modular/Explore.hs

+          -- Pre-index: map from QPN to intersected version range (Constrained only)
+          depVRs :: M.Map QPN VR
+          depVRs = M.fromListWith (.&&.)
+            [ (qpn, case ci of Constrained vr -> vr; _ -> anyVersion)
+            | Simple (LDep _ (Dep (PkgComponent qpn _) ci)) _ <- qdeps
+            ]


This index should only contain dependencies that are Constrained now, since CS.VersionConstraintConflict doesn't handle the others.

Speed up solver

86721c1

parsonsmatt commented Mar 3, 2026

View reviewed changes

parsonsmatt marked this pull request as ready for review March 3, 2026 06:15

sebright added the cabal-install: solver label Mar 3, 2026

Bodigrim reviewed Mar 3, 2026

View reviewed changes

Address review feedback

47e5847

Bodigrim approved these changes Mar 4, 2026

View reviewed changes

Bodigrim added the attention: needs-review label Mar 4, 2026

geekosaur approved these changes Mar 4, 2026

View reviewed changes

sebright requested changes Mar 5, 2026

View reviewed changes

Add a Set of QPN

85fe65e

parsonsmatt requested a review from sebright March 10, 2026 16:18

sebright reviewed Mar 11, 2026

View reviewed changes

sebright removed the attention: needs-review label Mar 11, 2026

		instance Ord OrderedVersionRange where
		compare = compare `on` show

Conversation

parsonsmatt commented Mar 3, 2026

Uh oh!

parsonsmatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bodigrim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebright commented Mar 5, 2026

Uh oh!

sebright left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebright left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sebright left a comment •

edited

Loading