Introduce with# as preferred alternative to touch#
Needs RevisionPublic

Authored by bgamari on Oct 19 2017, 8:30 AM.

Details

Summary

This introduces with# as a safer alternative to touch#, which is quite
susceptible to the simplifier optimizing it away. with# is a new primop
of this form,

with# :: a -> (State# s -> (# State s, r #)) -> State# s -> (# State# s, r #)

which evaluates the r, ensuring that the a remains alive throughout
evaluation. This is equivalent to the common pattern,

with' :: a                                -- ^ value to keep alive
      -> (State# s -> (# State# s, r #))  -- ^ value to evaluate
      -> State# s -> (# State# s, r #)
with' x r s0 =
    case r s0 of { (# s1, r' #) ->
    case touch# x s1 of { s2 -> (# s2, r' #) } }

but much harder for the simplifier to mess up.

Consider, for instance the case

test :: IO ()
test = do
    arr <- newPinnedByteArray 42
    doSomething (byteArrayContents arr)
    touch arr

If the simplifier believes that`doSomething` will fail to return (e.g. because
it is of the form forever ...), then it will be tempted to drop the
continuation containing touch#. This would result in the garbage collector
inappropriately freeing arr, resulting in catastrophe. This was the cause of
Trac Trac #14346.

However, with with# we can write this as

test :: IO ()
test = do
    arr <- newPinnedByteArray 42
    with# arr (unIO (doSomething (byteArrayContents arr)))

This construction the compiler can't mangle as there is no continuation to
drop. Instead, with# keeps arr alive by pushing a stack frame.
Consequently, arr will be kept alive as long as we are in
unIO (doSomething ...). If and when doSomething returns, the frame will
be dropped and arr allowed to die.

Test Plan

Fix and validate

bgamari created this revision.Oct 19 2017, 8:30 AM
bgamari updated this revision to Diff 14401.Oct 19 2017, 8:35 AM

Fix type

bgamari edited the summary of this revision. (Show Details)Oct 19 2017, 8:39 AM
bgamari edited the summary of this revision. (Show Details)Oct 19 2017, 8:41 AM
bgamari updated this revision to Diff 14403.Oct 19 2017, 8:44 AM

Add note

bgamari added a subscriber: simonpj.

Adding @simonpj, having preempted the request for a Note :).

bgamari updated this revision to Diff 14404.Oct 19 2017, 8:54 AM

Update docs

bgamari updated this revision to Diff 14405.Oct 19 2017, 8:55 AM

More docs

Indeed this appears to fix Trac #14346.

bgamari retitled this revision from [WIP] Introduce with# to Introduce with# as preferred alternative to touch#.Oct 19 2017, 5:05 PM
dfeuer added a subscriber: dfeuer.Oct 19 2017, 5:20 PM

The type signature you give in the summary is rather different from the one in primops.txt.pp. I'm rather unclear on what the true type is. Can you document this more clearly? In

o -> (State# s -> (# State# s, p #)) -> State# s -> (# State# s, p #)

o must of course be a pointer, but I would hope p could be any TYPE.

By the way, I like this idea very much. The semantics of touch# always looked very strange to me.

One other question: do you really need that tuple, or will a result of any
shape do?

simonmar requested changes to this revision.Oct 20 2017, 2:43 AM

The one downside of this is that we have to build a function closure to pass to with#, which entails more allocation than we were doing previously. But there's an alternative approach that would avoid this: expand with# during CoreToStg (or CorePrep perhaps) into the original case expression + touch#. There should be no extra allocation, no new primops needed, all it does is prevent the simplifier from eliminating the continuation.

Sorry I didn't think of this before, but it seems like a better solution (unless I'm missing something, which is entirely possible!).

This revision now requires changes to proceed.Oct 20 2017, 2:43 AM

The one downside of this is that we have to build a function closure to pass to with#, which entails more allocation than we were doing previously. But there's an alternative approach that would avoid this: expand with# during CoreToStg (or CorePrep perhaps) into the original case expression + touch#. There should be no extra allocation, no new primops needed, all it does is prevent the simplifier from eliminating the continuation.

Indeed, the allocation is unfortunate.

Sorry I didn't think of this before, but it seems like a better solution (unless I'm missing something, which is entirely possible!).

It sounds to me like @simonpj is suggesting in Trac #14375 that we move ahead with this approach until we work out how the story for more clever codegen should look like. Does this sound reasonable to you, @simonmar?

There is also the matter of what should be done for 8.2.2. There are two options, as far as I can see,

  • mark allocaBytes and friends as NOINLINE, essentially working around the issue, or
  • backport this patch which is big but not enormous

The type signature you give in the summary is rather different from the one in primops.txt.pp. I'm rather unclear on what the true type is.

Initially I gave a simplified version of the type to avoid obfuscating the point. However, I can see that this may be confusing. I'll amend.

bgamari edited the summary of this revision. (Show Details)Oct 20 2017, 7:14 AM
bgamari updated the Trac tickets for this revision.

That new signature smells funny!

That new signature smells funny!

Sorry, I'll put it in the next load of laundry. ;)

But in all seriousness, can you elaborate? It looks perfectly reasonable to me.

with# :: a -> (State# s -> r) -> State# s -> (# State# s, r #)

What is (State# s -> r)? It looks like the State# s token is being consumed by the callback, which seems a bit odd.

with# :: a -> (State# s -> r) -> State# s -> (# State# s, r #)

What is (State# s -> r)? It looks like the State# s token is being consumed by the callback, which seems a bit odd.

Yes, the summary was wrong; that was supposed to read (State# s -> (# State# s, r #)).

bgamari edited the summary of this revision. (Show Details)Nov 9 2017, 5:06 PM
austin resigned from this revision.Nov 9 2017, 5:49 PM