Open mpickering opened 2 years ago
Great analysis - but my conclusion is not
Commit https://github.com/haskell/containers/commit/e6fbb98a71c5a72e0805285cbbb799c5f978aceb causes GHCs performance tests to fail.
but this instead:
GHC's (quite unpredictable) inlining mechanics causes GHCs performance tests to fail.
It's not clear to me that rolling back the commit here would bring a stable improvement for GHC. The discussion https://github.com/haskell/containers/issues/794 https://github.com/haskell/containers/pull/800 seemed to show that there were some good reasons, backed up with measurements.
@nomeata perhaps saw something coming: https://github.com/haskell/containers/pull/800#issuecomment-922498670 Now would "offer both" help?
Agreed; this commit can't be blamed for subtle inlining interactions, unless it does something wrong with inlining.
Commit e6fbb98a71c5a72e0805285cbbb799c5f978aceb causes GHCs performance tests to fail. (https://gitlab.haskell.org/ghc/ghc/-/jobs/958207)
The issue for us in
Data.IntMap.Internal.lookup
, the modification of this function makes it small enough that Worker/Wrapper doesn't trigger on it:https://gist.github.com/mpickering/74590e7ababb7a82abc4b04749456ccd
The consequence is that different things get inlined into
lookupVarEnv
(a function in GHC)https://gist.github.com/mpickering/cd5a3c7baf1ae678d1a830cf76b2c56e
Then
lookupVarEnv
doesn't get inlined (because it is now too big) and every time it's called a function is allocated.OTOH, if the function is like how it was before then the wrapper gets inlined into
lookupVarEnv
,lookupVarEnv
is the inlined and the worker subsequently inlined into the right place.