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.