Draft: Split up Distribution.Simple.Setup

[TeofilC]

---

Please 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.
• [ ] The documentation has been updated, if necessary.

Please also shortly describe how you tested your change. Bonus points for added tests!

---

This PR is inspired by #8074 , which shows that Distribution.Simple.Setup is the heaviest module in lib:Cabal. This splits this module up into some smaller modules. This is quite natural as the file is already divided into sections for different Cabal commands.

The patch is quite rough right now and I need to clean it up quite a bit and update documentation.

Despite that we can already see some positive results on build times.

Before this change the critical path length for lib:Cabal was 44s after this change it is 36.3s.

Critical path before the change

(0.9958699999999999,"Distribution.Simple")
(2.03779,"Distribution.Simple.Haddock")
(1.05732,"Distribution.Simple.Build")
(3.2878300000000005,"Distribution.Simple.Configure")
(6.20755,"Distribution.Simple.GHC")
(0.17466000000000004,"Distribution.Simple.GHC.EnvironmentParser")
(1.3056100000000002,"Distribution.Simple.GHC.Internal")
(0.18498,"Distribution.Simple.BuildPaths")
(0.30138,"Distribution.Simple.LocalBuildInfo")
(2.2817099999999995,"Distribution.Types.LocalBuildInfo")
(15.639910000000004,"Distribution.Simple.Setup")
(2.9450000000000004e-2,"Distribution.Simple.Program")
(0.5085899999999999,"Distribution.Simple.Program.Db")
(0.19075999999999999,"Distribution.Simple.Program.Builtin")
(4.01405,"Distribution.Simple.Program.GHC")
(6.072999999999999e-2,"Distribution.Simple.GHC.ImplInfo")
(2.5100799999999994,"Distribution.Simple.Compiler")
(1.3293799999999998,"Distribution.Simple.Utils")
(1.0457600000000002,"Distribution.Verbosity")
(0.8630700000000001,"Distribution.Verbosity.Internal")
TOTAL: 44.026480000000014

Critical path after the change

(1.02669,"Distribution.Simple")
(2.05885,"Distribution.Simple.Haddock")
(1.07191,"Distribution.Simple.Build")
(3.3815399999999993,"Distribution.Simple.Configure")
(6.216269999999999,"Distribution.Simple.GHC")
(0.18744999999999998,"Distribution.Simple.GHC.EnvironmentParser")
(0.959,"Distribution.Simple.GHC.Internal")
(0.15687,"Distribution.Simple.BuildPaths")
(0.28278,"Distribution.Simple.LocalBuildInfo")
(2.189,"Distribution.Types.LocalBuildInfo")
(8.246440000000002,"Distribution.Simple.Setup.Config")
(0.22772999999999996,"Distribution.Simple.Setup.Common")
(2.8900000000000002e-2,"Distribution.Simple.Program")
(0.4929499999999999,"Distribution.Simple.Program.Db")
(0.18611000000000003,"Distribution.Simple.Program.Builtin")
(3.9847799999999998,"Distribution.Simple.Program.GHC")
(6.095999999999999e-2,"Distribution.Simple.GHC.ImplInfo")
(2.4298100000000002,"Distribution.Simple.Compiler")
(1.3067800000000003,"Distribution.Simple.Utils")
(1.02223,"Distribution.Verbosity")
(0.85893,"Distribution.Verbosity.Internal")
TOTAL: 36.375980000000006    

Before the Distribution.Simple.Setup file took 15.6s to compile.

Compile time for the split up modules sums to 13.4s

("Distribution.Simple.Setup",5.4199999999999995e-3)
("Distribution.Simple.Setup.Benchmark",0.14765999999999999)
("Distribution.Simple.Setup.Build",0.37532)
("Distribution.Simple.Setup.Clean",0.14677999999999997)
("Distribution.Simple.Setup.Common",0.22772999999999996)
("Distribution.Simple.Setup.Config",8.246440000000002)
("Distribution.Simple.Setup.Copy",0.21620999999999999)
("Distribution.Simple.Setup.Dist",0.18608000000000005)
("Distribution.Simple.Setup.Global",0.11736)
("Distribution.Simple.Setup.Haddock",1.18319)
("Distribution.Simple.Setup.HsColour",0.32167999999999997)
("Distribution.Simple.Setup.Install",0.29552999999999996)
("Distribution.Simple.Setup.REPL",0.5111499999999999)
("Distribution.Simple.Setup.Register",0.4165)
("Distribution.Simple.Setup.Test",1.0426199999999999)
TOTAL: 13.439670000000001

This also shows us what specific parts of that module are leading to slowness namely the bits to do with config flags.

[TeofilC]

Some stats about Distribution.Simple.Setup.Config: time to compile:

• all classes: 8.37s
• without Semigroup, Monoid: 7.59s
• without Semigroup, Monoid, Binary: 6.08s
• without Semigroup, Monoid, Binary, Structured: 5.69s
• without Semigroup, Monoid, Binary, Structured, Read: 4.82s
• without Semigroup, Monoid, Binary, Structured, Read, Generic: 4.60s
• without Semigroup, Monoid, Binary, Structured, Read, Generic, Eq: 4.25s
• without Semigroup, Monoid, Binary, Structured, Read, Generic, Eq, Show: 3.41s

[ulysses4ever]

@mergify rebase

[mergify[bot]]

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/2)
error: could not apply 45925a3cc... split up Distribution.Simple.Setup
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 45925a3cc... split up Distribution.Simple.Setup
Auto-merging Cabal/src/Distribution/Simple/Setup.hs
CONFLICT (content): Merge conflict in Cabal/src/Distribution/Simple/Setup.hs
Auto-merging Cabal/Cabal.cabal

err-code: A4750

[ulysses4ever]

@TeofilC great work here! I don't think you need to do much more here (e.g. removing instances can go into separate PR). Do you want to finish it?

[TeofilC]

Thanks! I've been meaning to finish off this PR. I'll see if I have some time this week.

[TeofilC]

I've had a chance to rebase these changes. I think it's ready for review.

The diff is quite large but really it's just splitting up this huge module into some bite sized ones

[ulysses4ever]

Great work! I wanna have a closer look some time this week, but I encourage anyone to go ahead and review: on the surface, this is a very nice refactoring!

[ulysses4ever]

Tremendous, thank you!

I think, this can be merged as is. We usually ask for a changelog file (see the link in the PR template), but I'm not sure it's strictly needed for a pure refactoring like this.

I have one question, though. The commit message says:

The main motivation is to improve parallelism of the module graph.

It won't be substantial as long as the rest of the codebase imports Distribution.Simple.Setup wholesale, will it? Do you have energy to go through those imports and see when they can be changed for finer-grained imports? Currently,

❯ rg 'import Distribution.Simple.Setup' -c | wc -l
85

I wonder if there are tools around that could assist in such task...

[TeofilC]

Good point @ulysses4ever . I'd be happy to do that. The refine imports HLS action should do exactly this so it shouldn't be too laborious

[TeofilC]

I've pushed a commit that should remove all but 3 of the imports in Cabal. Those 3 used most of the re-exported modules anyway so, we wouldn't be gaining anything but would pay the cost of extra verbosity.

There's some more imports in cabal-install and other packages but I haven't touched those, since the new modules are internal and because there's nothing to gain there (we have to wait for all of Cabal to build before starting to build any dependants anyway).

[ulysses4ever]

Makes sense, thanks a lot for the prompt action! Let's hope the second review will come soon.

[TeofilC]

@Mikolaj @Kleidukos I noticed you interacted with some of the comments in this PR.

Would you be interested in reviewing (the diff looks daunting but it's just moving things around)? Absolutely no worries if not!

[Kleidukos]

Hi @TeofilC. Changelog updates would be highly appreciated before we merge this. :)

[TeofilC]

Thanks for that speedy response @Kleidukos . I've added a changelog entry in b3e89588fca300407b9b0cce1a78b060169f9fee

[Kleidukos]

@TeofilC I'll let you set the "squash+merge me" label then :)

[TeofilC]

@Kleidukos I'm not sure if I can set labels. The little cog doesn't show up for me by Labels

[Kleidukos]

@TeofilC You have been invited as an outside collaborator with Triage access. Could you please reload the page?

[TeofilC]

Cheers @Kleidukos works now!