Opened 4 years ago

Closed 8 months ago

#11222 closed feature request (wontfix)

Teach strictness analysis about `catch`-like operations

Reported by: bgamari Owned by: dfeuer
Priority: normal Milestone: 8.10.1
Component: Compiler Version: 7.10.3
Keywords: Exceptions Cc: simonpj, simonmar
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: #10712, #14998 Differential Rev(s):
Wiki Page:

Description

Consider the catch# primop,

catch# :: (State# RealWorld -> (# State# RealWorld, a #) )      -- ^ thing to catch exceptions from
       -> (b -> State# RealWorld -> (# State# RealWorld, a #) ) -- ^ exception handler
       -> State# RealWorld
       -> (# State# RealWorld, a #)

Semantically, this operation will always evaluate its first argument. Ideally we would indicate this in the primop's strictness signature in primops.txt.pp. Sadly, we can't do this at the moment due to a subtle wrinkle (discovered in #10712):

Consider,

let r = \st -> raiseIO# blah st
in catch (\st -> ...(r st)..) handler st

If we give the first argument of catch a strict signature, we'll get a demand C(S) for r; that is, r is definitely called with one argument, which indeed it is. The trouble comes when we feed C(S) into r's RHS as the demand of the body as this will lead us to conclude that the whole let will diverge; clearly this isn't right.

As Simon noted in ticket:10712#comment:4,

There's something very special about catch: it turns divergence into non-divergence.

In order to apply a proper strictness signature to catch-like operations we would need to teach the strictness analyzer about this property.

Change History (15)

comment:1 Changed 4 years ago by Ben Gamari <ben@…>

In 28638df/ghc:

primops: Mark actions evaluated by `catch*` as lazy

There is something very peculiar about the `catch` family of operations
with respect to strictness analysis: they turn divergence into
non-divergence. For this reason, it isn't safe to mark them as strict in
the expression whose exceptions they are catching. The reason is this:

Consider,

    let r = \st -> raiseIO# blah st
    in catch (\st -> ...(r st)..) handler st

If we give the first argument of catch a strict signature, we'll get a
demand 'C(S)' for 'r'; that is, 'r' is definitely called with one
argument, which indeed it is. The trouble comes when we feed 'C(S)' into
'r's RHS as the demand of the body as this will lead us to conclude that
the whole 'let' will diverge; clearly this isn't right.

This is essentially the problem in #10712, which arose when
7c0fff41789669450b02dc1db7f5d7babba5dee6 marked the `catch*` primops as
being strict in the thing to be evaluated. Here I've partially reverted
this commit, again marking the first argument of these primops as lazy.

Fixes #10712.

Test Plan: Validate checking `exceptionsrun001`

Reviewers: simonpj, austin

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D1616

GHC Trac Issues: #10712, #11222

comment:2 Changed 4 years ago by simonmar

Cc: simonmar added

comment:3 Changed 4 years ago by Simon Peyton Jones <simonpj@…>

In 9915b656/ghc:

Make demand analysis understand catch

As Trac #11222, and #10712 note, the strictness analyser
needs to be rather careful about exceptions.  Previously
it treated them as identical to divergence, but that
won't quite do.

See Note [Exceptions and strictness] in Demand, which
explains the deal.

Getting more strictness in 'catch' and friends is a
very good thing.  Here is the nofib summary, keeping
only the big ones.

--------------------------------------------------------------------------------
        Program           Size    Allocs   Runtime   Elapsed  TotalMem
--------------------------------------------------------------------------------
          fasta          -0.1%     -6.9%     -3.0%     -3.0%     +0.0%
            hpg          -0.1%     -2.0%     -6.2%     -6.2%     +0.0%
       maillist          -0.1%     -0.3%      0.08      0.09     +1.2%
reverse-complem          -0.1%    -10.9%     -6.0%     -5.9%     +0.0%
         sphere          -0.1%     -4.3%      0.08      0.08     +0.0%
           x2n1          -0.1%     -0.0%      0.00      0.00     +0.0%
--------------------------------------------------------------------------------
            Min          -0.2%    -10.9%    -17.4%    -17.3%     +0.0%
            Max          -0.0%     +0.0%     +4.3%     +4.4%     +1.2%
 Geometric Mean          -0.1%     -0.3%     -2.9%     -3.0%     +0.0%

On the way I did quite a bit of refactoring in Demand.hs

comment:4 Changed 4 years ago by simonpj

Resolution: fixed
Status: newclosed

Done!

comment:5 Changed 3 years ago by simonpj

Keywords: Exceptions added

comment:6 Changed 2 years ago by dfeuer

Resolution: fixed
Status: closednew

This was semantically shady and has been partially reverted. We would like to find a way to recover some of the benefits without the downsides. One strong possibility is to add a catchIO# primop that only catches exceptions thrown by raiseIO#. Such a primop could be handled more aggressively. See https://ghc.haskell.org/trac/ghc/wiki/Exceptions/PreciseExceptions. Another direction would be to treat catch# forms specially in strictness analysis, somewhat like case expressions.

comment:7 Changed 2 years ago by dfeuer

Owner: set to dfeuer

comment:8 Changed 2 years ago by dfeuer

Milestone: 8.2.18.4.1

comment:9 Changed 20 months ago by bgamari

Milestone: 8.4.18.6.1

This ticket won't be resolved in 8.4; remilestoning for 8.6. Do holler if you are affected by this or would otherwise like to work on it.

comment:10 Changed 17 months ago by Ben Gamari <ben@…>

In 00b8ecb7/ghc:

Declare `catchRetry#` lazy in its first argument

As per the results on item 1 in T14998, declaring `catchRetry#` lazy in
its first argument opens the possibility to remove `ExnStr` complexity
from strictness demands at virtually no regressions in NoFib.

This brings `catchRetry#` in line with other primops from the `catch*`
family.

Reviewers: bgamari, simonpj, nomeata

Reviewed By: bgamari

Subscribers: thomie, carter

GHC Trac Issues: #14998, #11222

Differential Revision: https://phabricator.haskell.org/D4573

comment:11 Changed 15 months ago by bgamari

Milestone: 8.6.18.8.1

These won't be fixed for 8.6, bumping to 8.8.

comment:12 Changed 9 months ago by osa1

Milestone: 8.8.18.10.1

Bumping milestones of low-priority tickets.

comment:13 Changed 8 months ago by sgraf

comment:14 Changed 8 months ago by sgraf

#14998 suggests that we have the performance gains without actually making catch# and catchRetry# any more special than necessary. I think we can close this?

comment:15 Changed 8 months ago by simonpj

Resolution: wontfix
Status: newclosed

I think we can close this?

Yes I think so.

Note: See TracTickets for help on using tickets.