Modifying the inscope set in the simplifier can be surprisingly costly.

The problem

In light of #19817 (closed) i looked a bit into the simplifier.

Nothing much came of it but what I noticed was that adding a call to modifyInScope in a new (conditional) branch in simplLamBndrs was surprisingly costly adding 5-10% allocations for T9233 (though it was added in a pretty common case)

I assume this is because:

• The inscope set can become large rather quickly (reaching >300 ids for T9233).
• We can end up with somewhat inbalanced trees internally for our Data.IntMap resulting in rewriting a large part of the map when we modify one node.

In particular in simplLamBndrs we aren't very efficient.

We execute:

  = do { (env1, bndr1) <- simplBinder env bndr
       ; unf'          <- simplStableUnfolding env1 NotTopLevel Nothing bndr
                                      (idType bndr1) (idArityType bndr1) old_unf
       ; let bndr2 = bndr1 `setIdUnfolding` unf'
       ; return (modifyInScope env1 bndr2, bndr2) }

But that's not great.

• simplBinder internally updates the inscope set with bndr1 (which has a zapped unfolding). This is done inside subst_id_bndr
• We compute a potentially new unfolding.
• We update the inscope set again with the new unfolding.

A similar pattern happens in simplAlts which I suspect is called decently often as well. Only there we always use evaldUnfolding as final unfolding so we skip the step to compute a new unfolding.

If we could simplify the unfolding before we simplify the binder we could just pass the unfolding into simplBinder (as a maybe perhaps) and we would only have to update the in scope set once. Not sure if this is possible however.

Impact estimate

I tried simply adding yet another modifyInScope into the two code paths above. This caused about 0.7% additional allocations when compiling nofib/spectral/simple with -O

If we eliminate the current one that happens after simplBinder we should be able to expect the same kind of reduction on ghc. Not a huge improvement but possibly getting rid of 0.7% of allocations still seems worthwhile.