Skip to content

Keep a running InstalledPackageIndex to skip expensive per-package ghc-pkg calls#11767

Merged
mergify[bot] merged 1 commit into
haskell:masterfrom
sheaf:cached-ipi
May 30, 2026
Merged

Keep a running InstalledPackageIndex to skip expensive per-package ghc-pkg calls#11767
mergify[bot] merged 1 commit into
haskell:masterfrom
sheaf:cached-ipi

Conversation

@sheaf

@sheaf sheaf commented Apr 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Building on the in-library refactor that landed in 6867dd5, this patch extracts computePackageInfoFromIndex from the Cabal computePackageInfo function, which allows cabal-install to use a running InstalledPackageIndex instead of having to repeatedly query ghc-pkg, once per package.

Several cabal-install functions now pass around a TVar InstalledPackageIndex which keeps track of the InstalledPackageIndex used for building a project. Each time a dependency gets registered, we update this TVar, and read from it to obtain an InstalledPackageIndex to pass to the Cabal configure function, skipping slow calls to ghc-pkg.

Template Α: This PR modifies behaviour or interface

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • No manual QA notes necessary (no change to the command line interface).
  • Testing approach: the existing tests continue to work even though we query ghc-pkg far less.

@sheaf sheaf mentioned this pull request Apr 27, 2026
Merged
5 tasks
@sheaf sheaf force-pushed the cached-ipi branch 6 times, most recently from f27441c to 06795ae Compare April 29, 2026 11:57
@sheaf sheaf marked this pull request as ready for review April 29, 2026 11:59
@sheaf

sheaf commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator Author

This was a bit subtle to write, and getting it wrong means that we pass an incomplete or incorrect InstalledPackageIndex when configuring, which can lead to "Package not found" errors.

@ulysses4ever

ulysses4ever commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

Since the title of this PR mentions replacing "expensive" ghc-pkg calls, I am curious if there is any fast, back-of-envelope timings of what sorts of gains one can expect from this change.

@sheaf

sheaf commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator Author

Since the title of this PR mentions replacing "expensive" ghc-pkg calls, I am curious if there is any fast, back-of-envelope timings of what sorts of gains one can expect from this change.

I will get back to you with more precise measurements but for typical packages this can save over half of the time spent in configure. Taken with #11768 I am seeing typical configure times go from ~3s to ~0.5s.

@sheaf

sheaf commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator Author

I have put up a spreadsheet here, compiling pandoc from scratch and using the --build-timings feature from #11769.

Base average time spent in configure: 2.9 seconds.
With this patch: it goes down to 1.7 seconds.
With #11768: it goes down to 1.2 seconds.
With both: it goes down to 0.15 seconds.

@sheaf sheaf mentioned this pull request Apr 30, 2026
Merged
5 tasks
Mikolaj
Mikolaj approved these changes Apr 30, 2026
View reviewed changes

@Mikolaj Mikolaj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great job! Looks plausible to me.

ulysses4ever
ulysses4ever approved these changes May 1, 2026
View reviewed changes

@ulysses4ever ulysses4ever 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.

The caching strategy is straightforward and looks good. The relation to some ghc-pkg calls is indirect (you can't see it in the diff), so it's unclear to me. I'd appreciate if it was clarified, to which end I leave a comment for the changelog below. Thanks!

Comment thread changelog.d/cached-ipi.md Outdated
philderbeast
philderbeast reviewed May 2, 2026
View reviewed changes
Comment thread cabal-install/src/Distribution/Client/ProjectBuilding/PackageFileMonitor.hs Outdated
philderbeast
philderbeast reviewed May 2, 2026
View reviewed changes
Comment thread cabal-install/src/Distribution/Client/SetupWrapper.hs
@sheaf sheaf marked this pull request as draft May 4, 2026 10:20
@sheaf

sheaf commented May 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Re-marking as draft, as I have been using a version of cabal-install with these changes and have been running into some issues, with errors like:

Error: [Cabal-5000]
The following package dependencies were requested
    --dependency='th-abstraction=th-abstraction-0.7.2.0-f7bd9111da30b5c4189a982e4515bcca407b3baf'
however the given installed package instance does not exist.

@philderbeast philderbeast mentioned this pull request May 4, 2026
Open
@sheaf

sheaf commented May 5, 2026

Copy link
Copy Markdown
Collaborator Author

I have attempted to fix the issue I was seeing with the latest push to the branch. I will do a bit more testing locally to make sure things are working as expected.

@sheaf sheaf force-pushed the cached-ipi branch 2 times, most recently from 79f2edd to b9ac63a Compare May 7, 2026 13:08
@sheaf

sheaf commented May 15, 2026

Copy link
Copy Markdown
Collaborator Author

I ran more tests and it revealed some issues when resuming interrupted builds. I changed the approach: now cabal-install will perform a single ghc-pkg dump query at the start and then go from there. I've been testing this and haven't managed to run into any issues.

@sheaf sheaf marked this pull request as ready for review May 15, 2026 11:19
@sheaf

sheaf commented May 15, 2026

Copy link
Copy Markdown
Collaborator Author

Asking for re-review for the latest round of changes, see e.g. this diff.

@sheaf sheaf marked this pull request as draft May 15, 2026 13:00
@sheaf sheaf marked this pull request as ready for review May 18, 2026 12:24
@sheaf

sheaf commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Kindly asking whether @Mikolaj @ulysses4ever @philderbeast might take another look, this diff shows the changes since last time.

@ulysses4ever

ulysses4ever commented May 21, 2026

Copy link
Copy Markdown
Collaborator

I can take a look on the weekend

Mikolaj
Mikolaj approved these changes May 22, 2026
View reviewed changes

@Mikolaj Mikolaj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks just as plausible to me as last time. Good catch with the fixes.

@sheaf

sheaf commented May 27, 2026

Copy link
Copy Markdown
Collaborator Author

I would like to land this for the next cabal-install release. As I know we are drawing close to the deadline for the fork I would appreciate if someone could take the time to take another look. Sorry to keep pestering you all.

ulysses4ever
ulysses4ever reviewed May 27, 2026
View reviewed changes
Comment thread changelog.d/cached-ipi.md Outdated
ulysses4ever
ulysses4ever approved these changes May 27, 2026
View reviewed changes

@ulysses4ever ulysses4ever 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.

Looks very clean. Let's try it.

@ulysses4ever ulysses4ever added squash+merge me Tell Mergify Bot to squash-merge and removed attention: needs-review labels May 27, 2026
@mergify mergify Bot added ready and waiting Mergify is waiting out the cooldown period merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days labels May 27, 2026
@sheaf
Keep a running InstalledPackageIndex
38401e9 
We extract 'computePackageInfoFromIndex' from the Cabal 'computePackageInfo'
function, which allows cabal-install to use a running 'InstalledPackageIndex'
instead of having to repeatedly query ghc-pkg (once per package).

Several cabal-install functions now pass around a 'TVar InstalledPackageIndex'
which keeps track of the 'InstalledPackageIndex' used for building a
project. Each time a dependency gets registered, we update this 'TVar',
and read from it to obtain an 'InstalledPackageIndex' to pass to the
Cabal configure function, skipping slow calls to 'ghc-pkg'.

For more details, please see Note [Per-project InstalledPackageIndex]
in Distribution.Client.ProjectBuilding.
@mergify mergify Bot added the queued label May 30, 2026
@mergify

mergify Bot commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • ✅ Entered queue — 2026-05-30 16:14 UTC · Rule: squash-merge
  • ✅ Checks skipped · PR is already up-to-date
  • ✅ Merged — 2026-05-30 16:25 UTC · at 38401e9bcfeec4ccea771238ae25eccba89761e8 · squash

This pull request spent 11 minutes 21 seconds in the queue, including 2 seconds running CI.

Required conditions to merge
  • #approved-reviews-by >= 2 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Doctest Cabal
    • check-neutral = Doctest Cabal
    • check-skipped = Doctest Cabal
  • any of [🛡 GitHub branch protection]:
    • check-success = Meta checks
    • check-neutral = Meta checks
    • check-skipped = Meta checks
  • any of [🛡 GitHub branch protection]:
    • check-success = docs/readthedocs.org:cabal
    • check-neutral = docs/readthedocs.org:cabal
    • check-skipped = docs/readthedocs.org:cabal
  • any of [🛡 GitHub branch protection]:
    • check-success = Validate post job
    • check-neutral = Validate post job
    • check-skipped = Validate post job
  • any of [🛡 GitHub branch protection]:
    • check-success = fourmolu
    • check-neutral = fourmolu
    • check-skipped = fourmolu
  • any of [🛡 GitHub branch protection]:
    • check-success = hlint
    • check-neutral = hlint
    • check-skipped = hlint
  • any of [🛡 GitHub branch protection]:
    • check-success = Bootstrap post job
    • check-neutral = Bootstrap post job
    • check-skipped = Bootstrap post job
  • any of [🛡 GitHub branch protection]:
    • check-success = whitespace
    • check-neutral = whitespace
    • check-skipped = whitespace
  • any of [🛡 GitHub branch protection]:
    • check-success = Check sdist post job
    • check-neutral = Check sdist post job
    • check-skipped = Check sdist post job
  • any of [🛡 GitHub branch protection]:
    • check-success = Changelogs
    • check-neutral = Changelogs
    • check-skipped = Changelogs

@mergify mergify Bot merged commit b87ed0b into haskell:master May 30, 2026
63 checks passed
@mergify mergify Bot removed the queued label May 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mikolaj Mikolaj Mikolaj approved these changes
@ulysses4ever ulysses4ever ulysses4ever approved these changes
@philderbeast philderbeast Awaiting requested review from philderbeast

Assignees

No one assigned

Labels

merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days ready and waiting Mergify is waiting out the cooldown period squash+merge me Tell Mergify Bot to squash-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sheaf @ulysses4ever @Mikolaj @philderbeast