deepseq
deepseq copied to clipboard
Add more instances.
Adds instances for:
- UArray
- ForeignPtr
- TVar
Fixes #44 and fixes #15. Note that the ForeignPtr instance is strict in the contained Addr# but not the ForeignPtrContents. I think this is fine, since the only pure computation that can be done on a ForeignPtr is address arithmetic.
Great, thanks @TravisWhitaker.
@hvr, does this look good to you?
Pinging @hvr
What is required to merge this?
I haven't merged this yet since I was reminded of an ongoing discussion about whether deepseq
should provide NFData
for "reference-like" types (such IORef
and (->)
) at all. @hvr (the principal maintainer of deepseq
) has expressed skepticism about this, so I'm hesitant to proceed further without his input.
FWIW, the only instances in this patch that would fall into the "reference-like" category are the TVar
and ForeignPtr
instances. The UArray
instance seems uncontroversial, so perhaps that should be in its own patch.
I don't see how the instances for TVar
and ForeignPtr
(and IORef
) are anything like an instance for (->) r
. All of the former types are explicit references; there's no good reason IMO to expect that a normal form reference can only refer to normal form data.
Precisely what it means to have a normal form value of type (->) r
seems murky to me. What it means to have a normal form value of type TVar
, Ptr
, ForeignPtr
, STRef
, or IORef
seems pretty straight forward in contrast: there's no thunk evaluation required to determine where the reference is pointing.
I think the point is that for most other structures the definitions of the provided instances pass through to the child types.
If we had some sort of non recursive marker new type say NonRec we could perhaps have instances like NonRec (Ioref a)? On Thu, Oct 31, 2019 at 6:43 PM Travis Whitaker [email protected] wrote:
I don't see how the instances for TVar and ForeignPtr (and IORef) are anything like an instance for (->) r. All of the former types are explicit references; there's no good reason IMO to expect that a normal form reference can only refer to normal form data.
Precisely what it means to have a normal form value of type (->) r seems murky to me. What it means to have a normal form value of type TVar, Ptr, ForeignPtr, STRef, or IORef seems pretty straight forward in contrast: there's no thunk evaluation required to determine where the reference is pointing.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/haskell/deepseq/pull/50?email_source=notifications&email_token=AAABBQT6BMJQL2IXFKWFJDTQRNNQ3A5CNFSM4IY44WGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECZPA3Q#issuecomment-548597870, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABBQR3SAPLQEFM6ZVHWILQRNNQ3ANCNFSM4IY44WGA .
I think the point is that for most other structures the definitions of the provided instances pass through to the child types.
What's a "child type"? If it's just a type constructor's argument(s), would you consider the instances for Ptr
, FunPtr
, Fixed
, StableName
, MVar
, Proxy
, :~:
, and :~~:
problematic too?
@RyanGlScott @hvr what are the next steps for this PR?
Pinging @RyanGlScott @hvr how can this be moved forward?
I think the point is that for most other structures the definitions of the provided instances pass through to the child types. If we had some sort of non recursive marker new type say NonRec we could perhaps have instances like NonRec (Ioref a)?
I like this approach. One could define NFRec
and NonNFRec
wrappers and the relevant instances in a completely separate package, as the maintainers of this library seem potentially uninterested in this PR moving forward at the moment.
We do need to widen the contributor agency and I’m definitely guilty of it in my own maintainer duties.
I’m glad you like it! (The Rec/no Rec idea) I’m not quite sure how I’d structure it as a tool within the current api mind you. But it seems like a useful vocab that’s implicit in the underlying discussion.
I think it’s mostly cognitive fatigue of worrying about the Cartesian product of interactions and breakages if a bad semantics slips through. I could be wrong.
I find that when cognitive fatigue is the blocker, the simplest way to help progress things it to do some mix of the following
-
help write down / systematize the concerns or information, if it can be tabulated for comparison/ consistency checking all the Better
-
for code where I’m concerned about breakage, I actually have done a Grep of allll of hackage to look at / characterize how the impacted api is used and what might break so I can plan patches or remediation / assess impact. (Easiest when there’s a distinct module import or unique operation name )
-
establish feeling confident about the semantics being correct. This one I don’t have a good guidance on.
The point being, I think it’s best to view the slow velocity here not as adversarial, but rather as a cognitive load issue where evidential decision support will help progress the discussion.
Also fun collab makes everything more fun and effective!
On Thu, Dec 26, 2019 at 4:37 PM Samuel Schlesinger [email protected] wrote:
I think the point is that for most other structures the definitions of the provided instances pass through to the child types. If we had some sort of non recursive marker new type say NonRec we could perhaps have instances like NonRec (Ioref a)?
I like this approach. One could define NFRec and NonNFRec wrappers and the relevant instances in a completely separate package, as the maintainers of this library seem potentially uninterested in this PR moving forward at the moment.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/haskell/deepseq/pull/50?email_source=notifications&email_token=AAABBQVISWYNBLSPP76RPL3Q2UPY7A5CNFSM4IY44WGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHWFSJY#issuecomment-569137447, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABBQXC2QDUFVECE7XSVDLQ2UPY7ANCNFSM4IY44WGA .
@andrewthad @chessai thoughts?
I didn't see this, sorry. I like the wrapper idea. I think the addition of those, plus expanding documentation would work well and minimise breakage. I am specifically talking about reference types, not (->)
.
ok, so would we do eg
newtype NoRecWHNF a = NRWHNF a
instance NFData (NoRecWHNF a) where
rnf (NRWHNF a) = rwhnf a
and say "use this"?
Yep, that's what i was thinking of. The added NFData instances (different from what's in this PR) would go to NF (including the inner type), but the NoRec would always just go to WHNF. Are we on the same page?
Ok. So you’re proposing that nfdata instances should only be provided for things we can fully normalize (which perhaps includes flat structures like storable or boxed vectors.) and anything else should go through a marker that says “sorry, only whnf, no recursion”?
Or could you unpack the intent and execution and present vs future state I think you’re proposing?
I'm confused about what the Rec
/NoRec
wrappers have to do with the instances proposed in this PR. UArray
, ForeignPtr
, and TVar
are not recursive types, and although their type constructors take arguments, whether or not values of type UArray i a
/ForeignPtr a
/TVar a
are in normal form have nothing to do with what the argument type a
is, or indeed any values of type a
. These types have no a
in their structures at all; for example, I can easily construct a value of type ForeignPtr Void
that is in normal form.
I've been working with this definition of "normal form:" values in normal form can not be reduced any further, i.e.
- The constructor is determined.
- The constructor's arguments are not thunks.
- The constructor's arguments are in normal form.
This is subtle in the case of some reference types, since the reference's constructors don't contain the referenced type. Consider ForeignPtr
for example. Values of type ForeignPtr a
may reference values of type a
, even though the ForeignPtr
constructor and its arguments don't hold a value of type a
. Because a ForeignPtr a
is really just an Addr#
with some finalizers, we can use the exact same normalization strategy no matter what a
is. The normalized values yielded by the instances in this PR are not in WHNF even though rwhnf
is a sufficient implementation (except for ForeignPtr, which is strict in the Addr#
but not the finalizers). A ForeignPtr
with a known Addr#
is a real normal form value, unlike, for example, the values yielded by the NFData (a -> b)
instance (although I might be wrong about this, since I don't know what a function's normal form is) . Since this is a strategy that yields correct normal forms, I don't think it warrants a special wrapper.
It would be a mistake to dereference the a
that a ForeignPtr
might point to (e.g. nullPtr
can be made into a ForeignPtr
), since the referenced value has nothing to do with whether or not a ForeignPtr
value is in normal form. Indeed, I have a library that would segfault if the NFData (ForeignPtr a)
instance attempted to dereference and normalize the referenced a
values.
I suspect that Herbert simply hasn't had the time to review this PR or participate in this conversation. I'll shoot him an email, since that seems to be his preference these days.
The added NFData instances (different from what's in this PR) would go to NF (including the inner type)
@chessai, wouldn't the "recursive" NFData a => NFData (IORef a)
instance require unsafePerformIO
or similar? What rnf
definition would you use for this instance?
I agree, unsafe perform io instances are probably a bad idea
On Sun, Dec 29, 2019 at 10:12 PM Travis Whitaker [email protected] wrote:
The added NFData instances (different from what's in this PR) would go to NF (including the inner type)
@chessai https://github.com/chessai, wouldn't the "recursive" NFData a => NFData (IORef a) instance require unsafePerformIO or similar? What rnf definition would you use for this instance?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/haskell/deepseq/pull/50?email_source=notifications&email_token=AAABBQRCM633MZXRJIKKORTQ3FRJDA5CNFSM4IY44WGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHZPIAQ#issuecomment-569570306, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABBQXSJHP5UEOHIXYO5SLQ3FRJDANCNFSM4IY44WGA .
@TravisWhitaker yeah, my mistake. Didn't think too deeply on what I said about the added instances. And for most reference types, what i suggested doesn't really make any sense, as you pointed out. Perhaps the PR is just best as it stands, with sufficient documentation. To my understanding, we aren't really going to move forward without Herbert though.
Perhaps your well-worded note about reference types should be in the documentation, if that is how we will treat them?
values in normal form can not be reduced any further, i.e.
• The constructor is determined.
• The constructor's arguments are not thunks.
• The constructor's arguments are in normal form.
This is subtle in the case of some reference types, since the reference's constructors don't contain the referenced type. Consider ForeignPtr for example. Values of type ForeignPtr a may reference values of type a , even though the ForeignPtr constructor and its arguments don't hold a value of type a . Because a ForeignPtr a is really just an Addr# with some finalizers, we can use the exact same normalization strategy no matter what a is. The normalized values yielded by the instances in this PR are not in WHNF even though rwhnf is a sufficient implementation (except for ForeignPtr, which is strict in the Addr# but not the finalizers). A ForeignPtr with a known Addr# is a real normal form value, unlike, for example, the values yielded by the NFData (a -> b) instance.
EDIT: the note needs an edit, of course, if it were to be introduced into the actual documentation
Pinging @hvr .
Hvr: would you Like some other clc folks to take point on this ?
On Thu, Feb 27, 2020 at 5:07 PM Travis Whitaker [email protected] wrote:
Pinging @hvr https://github.com/hvr .
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/haskell/deepseq/pull/50?email_source=notifications&email_token=AAABBQUB35P5GSNA4ALWIHLRFA2SVA5CNFSM4IY44WGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENGENTA#issuecomment-592201420, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABBQRRJJSTNIXRJ7SCQITRFA2SVANCNFSM4IY44WGA .
Pinging @hvr.
@RyanGlScott is there anything that can be done to move this forward? I'm really curious to hear Herbert's take on this, but it's rather unfortunate for something like this to be bottlenecked for months on one person.
I'm no longer on the CLC, so I can't really say one way or the other.
@sjakobi @chessai what more can be done to either:
- Move this forward?
- Clearly explain and document why this should not be done?
For what it's worth, I expanded on my line of reasoning for the existence of these instances here.
@TravisWhitaker I'm not a maintainer of this package.
HVR is unavailable. Pinging @bgamari and @ekmett and @emilypi and @gwils for thoughts.
@TravisWhitaker I've just dropped support for GHC 7 to simplify the CPP in Control.DeepSeq
. Would you please rebase so that these instances can finally be added?