Re: [GHC] #14998: Sort out the strictness mess for exceptions

#14998: Sort out the strictness mess for exceptions -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.3 Component: Compiler | Version: 8.2.2 Resolution: | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by simonpj): * keywords: => Exceptions Old description:

Over time we have evolved into a messy and bad place for the interaction of strictness analysis and exceptions.

Here's the amazing history of the strictness of `catch#`

* Dec 13: 0558911f91c: catch# gets a strictness signature with `lazyApply1Dmd` * Jul 15: 7c0fff41789: catch# goes from `lazyApply1Dmd` to `strictApply1Dmd` * Dec 15: 28638dfe79e: catch# goes from `strictApply1Dmd` to `lazyApply1Dmd`. Trac #10712 * Jan 16: 9915b656440: catch# goes from `lazyApply1Dmd` to `catchArgDmd`, and result goes from `botRes` to `exnRes` Trac #11222. * Mar 17: 701256df88c: catch# goes from 'catchArgDmd` to `lazyApply1Dmd`. This ticket #13330.

See also * `Note [Strictness for mask/unmask/catch]` in `primops.txt/pp`. * `exceptions_and_strictness` in `primops.txt/pp`. * `Note [Exceptions and strictness]` in `Demand.hs` * #11555, #13330, #10712, #11222

'''Item 1: ThrowsExn'''

* The strictness analyser has quite a bit of complexity around `ThrowsExn`. * `ThrowsExn` only differs from `Diverges` if the `ExnStr` flag is set to `ExnStr` * And the only place that happens is in `Demand.catchArgDmd` * The only place `catchArgDmd` is used is in `primops.txt.pp`. Originally, in the patch for #11222, `catchArgDmd` was used for the first arg of `catch#`, `catchSTM#` and `catchRetry#`. * But the patch in this ticket removed two of those three; now only `catchRetry#` uses the `catchArgDmd` thing.

It looks to me as if `catchRetry#` was left out by mistake; and indeed David says "I left it out on the grounds that I didn't understand it well enough."

And if so, that's the last use of `catchArgDmd` and I think that all the tricky `ThrowsExn` machinery can disappear.

'''Item 2: strictness of catchException'''

As a result of all this to-and-fro we have in GHC.IO {{{ catch (IO io) handler = IO $ catch# io handler' catchException !io handler = catch io handler }}} That is, `catchException` is strict in the IO action itself. But Neil argues in #11555, commment:6 that this is bad because it breaks the monad laws.

I believe that there is some claimed performance gain from the strictness of `catchException`, but I don't believe it exists. The perf gain was from when it had a `strictApply1Dmd`; that is, it promised to call its argument. See this note in `primops.txt.pp` {{{ -- Note [Strictness for mask/unmask/catch] -- Consider this example, which comes from GHC.IO.Handle.Internals: -- wantReadableHandle3 f ma b st -- = case ... of -- DEFAULT -> case ma of MVar a -> ... -- 0# -> maskAsynchExceptions# (\st -> case ma of MVar a -> ...) -- The outer case just decides whether to mask exceptions, but we don't want -- thereby to hide the strictness in 'ma'! Hence the use of strictApply1Dmd. }}} I think the above saga was all in pursuit of exposing the strictness of `ma` if we used `catch#` instead of `maskAsynchExceptions#` in this example. But the saga ultimate appears to have failed, and the strictenss annotation in `catchException` is a vestige that carries no benefit, but does requires comments etc.

'''Item 3: performance'''

If making `catch#` have strictness `strictApply1Dmd` improves perf, it would be good to know where. That would entail making the (tiny) change, recompiling all, and nofibbing. Here is the commit message in 7c0fff41789, which added `strictApply1Dmd`: {{{ commit 7c0fff41789669450b02dc1db7f5d7babba5dee6 Author: Simon Peyton Jones Date: Tue Jul 21 12:28:42 2015 +0100

Improve strictness analysis for exceptions

Two things here:

* For exceptions-catching primops like catch#, we know that the main argument function will be called, so we can use strictApply1Dmd, rather than lazy

Changes in primops.txt.pp

* When a 'case' scrutinises a I/O-performing primop, the Note [IO hack in the demand analyser] was throwing away all strictness from the code that followed.

I found that this was causing quite a bit of unnecessary reboxing in the (heavily used) function GHC.IO.Handle.Internals.wantReadableHandle

So this patch prevents the hack applying when the case scrutinises a primop. See the revised Note [IO hack in the demand analyser]

Thse two things buy us quite a lot in programs that do a lot of IO.

Program Size Allocs Runtime Elapsed TotalMem -------------------------------------------------------------------------------- hpg -0.4% -2.9% -0.9% -1.0% +0.0% reverse-complem -0.4% -10.9% +10.7% +10.9% +0.0% simple -0.3% -0.0% +26.2% +26.2% +3.7% sphere -0.3% -6.3% 0.09 0.09 +0.0% -------------------------------------------------------------------------------- Min -0.7% -10.9% -4.6% -4.7% -1.7% Max -0.2% +0.0% +26.2% +26.2% +6.5% Geometric Mean -0.4% -0.3% +2.1% +2.1% +0.1% }}} If these gains are still on the table (i.e. moving to `strictApply1Dmd` gives them), perhaps we can add some strictness annotations to the I/O to replicate the effect, even if we'd decided we can't actualy make `catch#` strict?

'''Item 4: IO hack in the demand analyser'''

In getting up to speed with this, I noticed this comment in the demand analyser: {{{ {- Note [IO hack in the demand analyser] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ There's a hack here for I/O operations. Consider

case foo x s of { (# s', r #) -> y }

...omitted...

However, consider f x s = case getMaskingState# s of (# s, r #) -> case x of I# x2 -> ...

Here it is terribly sad to make 'f' lazy in 's'. After all, getMaskingState# is not going to diverge or throw an exception! This situation actually arises in GHC.IO.Handle.Internals.wantReadableHandle (on an MVar not an Int), and made a material difference.

So if the scrutinee is a primop call, we *don't* apply the state hack... }}} Idea: in the nested-CPR work, we have simple termination information. So we can use that, instead of "is a primop call" to deliver on this hack in a much more principled way.

New description: Over time we have evolved into a messy and bad place for the interaction of strictness analysis and exceptions. Here's the amazing history of the strictness of `catch#` * Dec 13: 0558911f91c: catch# gets a strictness signature with `lazyApply1Dmd` * Jul 15: 7c0fff41789: catch# goes from `lazyApply1Dmd` to `strictApply1Dmd` * Dec 15: 28638dfe79e: catch# goes from `strictApply1Dmd` to `lazyApply1Dmd`. Trac #10712 * Jan 16: 9915b656440: catch# goes from `lazyApply1Dmd` to `catchArgDmd`, and result goes from `botRes` to `exnRes` Trac #11222. * Mar 17: 701256df88c: catch# goes from 'catchArgDmd` to `lazyApply1Dmd`. This ticket #13330. See also * The [wiki:Exceptions] page * `Note [Strictness for mask/unmask/catch]` in `primops.txt/pp`. * `exceptions_and_strictness` in `primops.txt/pp`. * `Note [Exceptions and strictness]` in `Demand.hs` * #11555, #13330, #10712, #11222 '''Item 1: ThrowsExn''' * The strictness analyser has quite a bit of complexity around `ThrowsExn`. * `ThrowsExn` only differs from `Diverges` if the `ExnStr` flag is set to `ExnStr` * And the only place that happens is in `Demand.catchArgDmd` * The only place `catchArgDmd` is used is in `primops.txt.pp`. Originally, in the patch for #11222, `catchArgDmd` was used for the first arg of `catch#`, `catchSTM#` and `catchRetry#`. * But the patch in this ticket removed two of those three; now only `catchRetry#` uses the `catchArgDmd` thing. It looks to me as if `catchRetry#` was left out by mistake; and indeed David says "I left it out on the grounds that I didn't understand it well enough." And if so, that's the last use of `catchArgDmd` and I think that all the tricky `ThrowsExn` machinery can disappear. '''Item 2: strictness of catchException''' As a result of all this to-and-fro we have in GHC.IO {{{ catch (IO io) handler = IO $ catch# io handler' catchException !io handler = catch io handler }}} That is, `catchException` is strict in the IO action itself. But Neil argues in #11555, commment:6 that this is bad because it breaks the monad laws. I believe that there is some claimed performance gain from the strictness of `catchException`, but I don't believe it exists. The perf gain was from when it had a `strictApply1Dmd`; that is, it promised to call its argument. See this note in `primops.txt.pp` {{{ -- Note [Strictness for mask/unmask/catch] -- Consider this example, which comes from GHC.IO.Handle.Internals: -- wantReadableHandle3 f ma b st -- = case ... of -- DEFAULT -> case ma of MVar a -> ... -- 0# -> maskAsynchExceptions# (\st -> case ma of MVar a -> ...) -- The outer case just decides whether to mask exceptions, but we don't want -- thereby to hide the strictness in 'ma'! Hence the use of strictApply1Dmd. }}} I think the above saga was all in pursuit of exposing the strictness of `ma` if we used `catch#` instead of `maskAsynchExceptions#` in this example. But the saga ultimate appears to have failed, and the strictenss annotation in `catchException` is a vestige that carries no benefit, but does requires comments etc. '''Item 3: performance''' If making `catch#` have strictness `strictApply1Dmd` improves perf, it would be good to know where. That would entail making the (tiny) change, recompiling all, and nofibbing. Here is the commit message in 7c0fff41789, which added `strictApply1Dmd`: {{{ commit 7c0fff41789669450b02dc1db7f5d7babba5dee6 Author: Simon Peyton Jones Date: Tue Jul 21 12:28:42 2015 +0100 Improve strictness analysis for exceptions Two things here: * For exceptions-catching primops like catch#, we know that the main argument function will be called, so we can use strictApply1Dmd, rather than lazy Changes in primops.txt.pp * When a 'case' scrutinises a I/O-performing primop, the Note [IO hack in the demand analyser] was throwing away all strictness from the code that followed. I found that this was causing quite a bit of unnecessary reboxing in the (heavily used) function GHC.IO.Handle.Internals.wantReadableHandle So this patch prevents the hack applying when the case scrutinises a primop. See the revised Note [IO hack in the demand analyser] Thse two things buy us quite a lot in programs that do a lot of IO. Program Size Allocs Runtime Elapsed TotalMem -------------------------------------------------------------------------------- hpg -0.4% -2.9% -0.9% -1.0% +0.0% reverse-complem -0.4% -10.9% +10.7% +10.9% +0.0% simple -0.3% -0.0% +26.2% +26.2% +3.7% sphere -0.3% -6.3% 0.09 0.09 +0.0% -------------------------------------------------------------------------------- Min -0.7% -10.9% -4.6% -4.7% -1.7% Max -0.2% +0.0% +26.2% +26.2% +6.5% Geometric Mean -0.4% -0.3% +2.1% +2.1% +0.1% }}} If these gains are still on the table (i.e. moving to `strictApply1Dmd` gives them), perhaps we can add some strictness annotations to the I/O to replicate the effect, even if we'd decided we can't actualy make `catch#` strict? '''Item 4: IO hack in the demand analyser''' In getting up to speed with this, I noticed this comment in the demand analyser: {{{ {- Note [IO hack in the demand analyser] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ There's a hack here for I/O operations. Consider case foo x s of { (# s', r #) -> y } ...omitted... However, consider f x s = case getMaskingState# s of (# s, r #) -> case x of I# x2 -> ... Here it is terribly sad to make 'f' lazy in 's'. After all, getMaskingState# is not going to diverge or throw an exception! This situation actually arises in GHC.IO.Handle.Internals.wantReadableHandle (on an MVar not an Int), and made a material difference. So if the scrutinee is a primop call, we *don't* apply the state hack... }}} Idea: in the nested-CPR work, we have simple termination information. So we can use that, instead of "is a primop call" to deliver on this hack in a much more principled way. '''Item 5: raiseIO#''' There is an unresolved question about whether `raiseIO#` should return an exception result in its strictness signature. See Trac #13380, which applied a patch, then reverted it (on perf grounds) and then nothing. -- -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14998#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler