A little unboxing and strictifying #351
Conversation
|
|
||
| -- | Basic type for a socket. | ||
| data Socket = Socket (IORef CInt) CInt {- for Show -} | ||
| data Socket = Socket !(IORef CInt) !CInt {- for Show -} |
There was a problem hiding this comment.
Ah... actually, it should probably just be an IORef Validity! That removes the redundancy and also the CInt box in the IORef.
|
I'm traveling. I will look your issues in the next week. |
|
@treeowl Thanks for the PR. Could you please provide a description of this work, as well as a reason for its inclusion? |
|
@eborden, it gets rid of some indirections and unnecessary boxes. What in particular do you find unclear? |
|
@treeowl This isn't necessarily about being unclear. This PR introduces a fair amount of complexity by pulling in prim-ops. Increase complexity brings with it a maintenance burden. I'm curious if the performance benefit here pays for the complexity cost. |
|
I can't say for sure. Using a custom deal for the |
|
@treeowl I would easily approve the |
| -- value. This is similar to 'Data.IORef.mkWeakIORef', but we use it | ||
| -- to avoid keeping the useless 'IORef' wrapper alive. | ||
| mkWeakFromIORef :: IORef a -> b -> IO () -> IO (Weak b) | ||
| mkWeakFromIORef (IORef (STRef r##)) b (IO finalizer) = IO $ \s -> |
There was a problem hiding this comment.
I'd also love to see this function move to its own internal module.
There was a problem hiding this comment.
Agreed. I'll move it.
|
I suppose. The thing is that |
I think I agree here... However I also think it won't matter much here.. But I have no objections. |
|
Is |
|
I don't think it's big enough to require CLC approval, just a patch to base would be enough imho. That said, even if it makes it into base, it won't be usable here for a while.. unless you check for base versions. |
|
To make a step forward, may I copy |
|
👍 That sounds like a good step forward. |
|
Done. I would close this issue. @treeowl If you really want to bring prim-ops stuff to |
4 participants
No description provided.