Skip to content

Make GHCi commands compatible with multiple home units

Hannes Siebenhandl requested to merge wip/fendor/ghci-multiple-home-units into master

This MR addresses the issue #20889 (closed).

Given the design idea in #20889 (comment 621435), the implementation is relatively straight forward. The core insight is that a 'ModuleName' is not sufficient to identify a 'Module' in the 'HomeUnitGraph'. Thus, large parts of the PR is simply about refactoring usages of 'ModuleName' to prefer 'Module', which has a 'Unit' attached and is unique over the 'HomeUnitGraph'.

Consequentially, most usages of lookupHPT are likely to be incorrect and have been replaced by lookupHugByModule which is keyed by a 'Module'.

In GHCi/UI.hs, we make sure there is only one location where we are actually translating ModuleName to a Module:

  • lookupQualifiedModuleName

If a ModuleName is ambiguous, we detect this and report it to the user.

To avoid repeated lookups of ModuleNames, we store the Module in the InteractiveImport, which additionally simplifies the interface loading.

A subtle detail is that the DynFlags of the InteractiveContext are now stored both in the HomeUnitGraph and in the InteractiveContext. In UI.hs, there are multiple code paths where we are careful to update the DynFlags in both locations. Most importantly in addToProgramDynFlags.

There is one metric increase in this commit: T4029

It is an increase from 14.4 MB to 16.1 MB (+11.8%) which sounds like a pretty big regression at first. However, we argue this increase is solely caused by using more data structures for managing multiple home units in the GHCi session. In particular, due to the design decision of using three home units, the base memory usage increases... but by how much?

A big contributor is the UnitState, of which we have three now, which on its own 260 KB per instance. That makes an additional memory usage of 520 KB, already explaining a third of the overall memory usage increase. Then we store more elements in the HomeUnitGraph, we have more HomeUnitEnv entries, etc...

While we didn't chase down each byte, we looked at the memory usage over time for both '-hi' and '-hT' profiles and can say with confidence while the memory usage increased slightly, we did not introduce any space leak, as the graph looks almost identical as the memory usage graph of GHC HEAD.

Edited by Hannes Siebenhandl

Merge request reports