haskell/core-libraries-committee

Add System.Mem.performBlockingMajorGC

TeofilC opened this issue · 12 comments

GHC's C RTS interface already exports a performBlockingMajorGC function. This differs from the standard performMajorGC in that it will force nonmoving collections to be blocking. Currently the only way to access this is through the C API.

This proposal is to expose this as a Haskell function: System.Mem.performBlockingMajorGC.

See the GHC MR here: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11693

I tried searching Hackage and there's no mention of performBlockingMajorGC so it's unlikely that exporting this could break any code.

This sounds like a great candidate to go into ghc-internal to me.

Could expand on what you mean @tomjaguarpaw?

Much like the currently exposed functions, the intention would be that end users would make use of this function.
As GHC specific things go, I don't think this is all too tightly coupled to internals. It just assumes whatever runtime you've got has got a GC.

In any case it would be a shame I think to split up these functions. I think they should all be in the same package whatever that is.

I suppose that, as we already have a lot of similar functions in System.Mem, there's a good argument to say that your suggested function belongs there. On the other hand, I personally am inclined to set a very high bar for what we expose from base, and I don't like exposing something from base that's never been exposed, nor used, anywhere else. (Other CLC members may take different views, but that's mine.)

I'm quite sympathetic to that! It's a bit of a shame that a bunch of the stable API of the rts is exposed through base.

FWIW while this variant itself isn't used the performMajorGC one (and performGC which is an alias for the former) is used quite extensively on Hackage. https://hackage-search.serokell.io/?q=perform%28Major%29%3FGC. Looks to be about 160 packages.

I think the majority of those should really be using performBlockingMajorGC instead if they wanted to be compatible with the nonmoving GC, since it looks like a bunch of the following code is depending on the invariant "a GC has just run" rather than "a GC has been started and is maybe running in the background"

I think the majority of those should really be using performBlockingMajorGC instead if they wanted to be compatible with the nonmoving GC, since it looks like a bunch of the following code is depending on the invariant "a GC has just run" rather than "a GC has been started and is maybe running in the background"

That's a very interesting point. A common use of performMajorGC is for benchmarking, when you want to measure timings from a clean slate. A GC running in the background would be disaster here, and that's exactly what would happen with non-moving GC running in another thread.

I strongly agree that performBlockingMajorGC should be exported from the same module as performMajorGC / performGC. The latter are commonly used by end users and I do not perceive it being hidden from them into the depth of ghc-internal any time soon.

Unless there is any further discussion, I'll open the vote soon.

Dear CLC members, let's vote on the proposal to add System.Mem.performBlockingMajorGC to accompany existing performMajorGC, performMinorGC, etc. The MR is available at https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11693.

The motivation is that in the presence of non-moving GC triggering garbage collection in multithreaded program may proceed to the next statement before garbage collection finishes. This is a good thing in general because of decreased latency, but users often trigger performMajorGC to get a clean state of RTS not obstructed by GC (e. g., for benchmarking or before performing heavy allocations which might get close to the heap limit). The proposed performBlockingMajorGC fixes this gap.

@tomjaguarpaw @parsonsmatt @angerman @mixphix @velveteer @hasufell


+1 from me, cannot wait to use it in tasty-bench.

+1


In an ideal world this would be made available in package maintained by GHC HQ but in the meantime it seems sensible to expose this from the same place that its family members are exposed.

+1

Thanks all, that's enough votes to approve.

Thanks folks