Fix space leak where EPS retained HPTs from old HscEnv

[mpickering]

The issue here is that the ModuleGraph value is not forced eagerly, so
the EPS ends up loading an interface file for an external package and
retains a stale reference to the old HPT for the rest of the session.

The heap should always satisfy the invariant that no HomeModInfo are
reachable via the EPS.

See GHC issue #20509 for something similar.

[pepeiborra]

@Anton-Latukha thanks for the quick approval, but I would have preferred to have a quick look before the change was merged.

Thanks for the fix @mpickering. How did you find this leak? I would love to learn more (and probably other people would too).

Is there any way to test for this? Perhaps using weak references in the style of https://simonmar.github.io/posts/2018-06-20-Finding-fixing-space-leaks.html

[mpickering]

I was in the process of adding a performance test which would trigger this case but didn't have chance to finish it yet.

How did I find this place? In a bit of a roundabout way:

1. Looked at a -hi profile of a very long (> 24hr) hls session.
2. Fired up ghc-debug, asked what was retaining some of the suspicious stuff.
3. Scrolled down the retainer stack and noticed that the EPS was retaining a HPT, and using my domain knowledge knew this shouldn't happen.

So basically I just messed around a bit with ghc-debug looking at some retainer stacks and making sure they made sense to me.

[Anton-Latukha]

Yes, I was on the edge to merge it or not & wanted to re-ask if it is done.

I agree that is probably a mistake on my part, and I was accustomed to other conditions, I probably should use a more careful strategy than the current evaluation.

Would use a more rigorous process & probably would stay away from merging the complex Haskell PRs however simple they may look.

[mpickering]

@pepeiborra @wz1000 I am finding the benchmark suit a bit difficult to navigate and have spent too much time now trying to work out how to write a benchmark.

I need to write a simple test which

• Loads a bunch of modules (but does not open them, so that the HomeModInfo for each of them is live)
• Performs a simple edit in all the modules (adding a newline is fine, so that the ModDetails changes for each of them)
• Checks that current memory usage at the end of the test is the same as at the start (to check the ModDetails from the first load is not retained)

I can't work out how to achieve this with the current benchmarking code. I started looking to see if there was a way to get a list of the "known files" but then I considered that lsp-test doesn't honour watch file requests (and hence notice if they changed).

[pepeiborra]

I need to write a simple test which...

Rather than a benchmark, did you consider a test that uses weak references to check the invariant?

[mpickering]

I need to write a simple test which...

Rather than a benchmark, did you consider a test that uses weak references to check the invariant?

I am a bit nervous using weak references on boxed values as you can end up in situations where you still have the leak but the value has been collected (if it's been reboxed).

I have nearly got the performance test working now.

[mpickering]

Turns out I spoke too soon, I can't conjure up a test which gets us into the bad situation unfortunately. I will try and add a test if I find anything else.

[mpickering]

I backported some more space leak issues to the 9.2 branch and these seem to fix the EPS retaining issues that I could see when loading GHC into HLS.

https://gitlab.haskell.org/ghc/ghc/-/merge_requests/7274