Skip to content
GitLab
Closed
Issue created by Simon Peyton Jones@simonpjDeveloper

Sort out the strictness mess for exceptions

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: 0558911f: catch# gets a strictness signature with lazyApply1Dmd
  • Jul 15: 7c0fff41: catch# goes from lazyApply1Dmd to strictApply1Dmd
  • Dec 15: 28638dfe: catch# goes from strictApply1Dmd to lazyApply1Dmd. Trac #10712 (closed)
  • Jan 16: 9915b656: catch# goes from lazyApply1Dmd to catchArgDmd, and result goes from botRes to exnRes Trac #11222 (closed).
  • Mar 17: 701256df: catch# goes from catchArgDmd to lazyApply1Dmd. This ticket #13330 (closed).

See also

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 (closed), 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.

Edit (Mar 20): The resolution was that we simply could get rid of this machinery, see Note [Exceptions and strictness]. But with #17676 (closed) and !2525 (closed), we re-introduced a variant of ThrowsExn to preserve precise exceptions thrown by raiseIO# (concerning item 4 and item 5).

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 (closed), 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 7c0fff41, which added strictApply1Dmd:

commit 7c0fff41789669450b02dc1db7f5d7babba5dee6
Author: Simon Peyton Jones <simonpj@microsoft.com>
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.

We had !1829 (closed) to have a principled way to decide when to apply the IO hack, but that turned out to be too imprecise (#17653 (closed)). We reverted and track the progress on this item in #17676 (closed), with a resolution in !2525 (closed). This overlaps with Item 5, because the IO hack is all about precise exceptions, which are thrown by raiseIO#.

Item 5: raiseIO#

There is an unresolved question about whether raiseIO# should return an exception result in its strictness signature. See Trac #13380 (closed), which applied a patch, then reverted it (on perf grounds) and then nothing.

Edit (Mar 20): With #17676 (closed) and !2525 (closed), we re-introduced a variant of ThrowsExn to preserve precise exceptions thrown by raiseIO#.

Edited by Sebastian Graf
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information