
Thank you for your comments!
On Wed, Apr 24, 2013 at 8:23 PM, Edward Z. Yang
Great, good to see.
Some questions:
- You seem to be stubbing out cfinalizers with stg_WEAK_IS_DEAD_closure when a weak pointer gets finalized; your locking is predicated on operations on this field being cas'es. Another design is to lock the closure itself, via the info table header (using lockClosure). I think both strategies should be sound, but I suspect lockClosure is bound to be safer, in case you end up mutating other fields.
You are right, I updated the patches to use lockClosure. I was not aware of the existence of lockClosure when I wrote the previous patches, which is why I used cas().
- Typo "Kill a weak poitner"
Now the comment is gone.
- There was a comment about LDV which was removed. Why is that comment irrelevant in the new world order?
In that version of the patches finalizeWeak# didn't modify the infoptr, so the comment didn't apply. It is back in the latest version of the patches, though.
Edward
Thank you very much for your help, I just updated the patches on the ticket.
On Sun, Apr 21, 2013 at 10:50 AM, Edward Z. Yang
In your ticket, you mention this patch introduces a race condition. One possible fix is to have addCFinalizerToWeak# check if the pointer is already dead, and just run the finalizer immediately if it is. I think this preserves the semantics, but this needs to be checked closely.
Edward
Excerpts from Akio Takano's message of Fri Apr 19 02:58:50 -0700 2013:
I removed the invariant by adding a new primop, addCFinalizerToWeak#. I opened a ticket for the issue.
http://hackage.haskell.org/trac/ghc/ticket/7847
- Akio
On Thu, Mar 21, 2013 at 2:40 PM, Simon Marlow
wrote: On 11/03/13 03:17, Akio Takano wrote:
Hi,
I'm working on implementing per-generation lists of weak pointers to speed up garbage collection in programs that allocate a lot of weak pointers. I have a patch [1] that validates and gives a 3x speed up on a benchmark. However I'd like to ask for some advise before finishing and submitting the patch.
[1] https://github.com/takano-**akio/ghc/commit/** c7345c68eaa1e7f9572e693b5e352e**386df7d680<
https://github.com/takano-akio/ghc/commit/c7345c68eaa1e7f9572e693b5e352e386d...
The problem is that since my patch splits the weak pointer list between generations, it no longer maintains the right order of
weak
pointers. This could cause finalizers added with addForeignPtrFinalizer to run in the wrong order.
I can think of one way to fix it; to make sure that when a WEAK object gets promoted, it is always added to the front of the new list. So my questions are:
- Would it be a correct fix? - If so, is it an acceptable fix? For example, is it too fragile a reasoning to rely on?
I don't like the way that we rely on the ordering of the weak
Excerpts from Akio Takano's message of Wed Apr 24 02:12:42 -0700 2013: pointer
right now. I think this invariant arose accidentally when the support for C finalizers was added. It was wrong for some time, see:
http://hackage.haskell.org/**trac/ghc/ticket/7160< http://hackage.haskell.org/trac/ghc/ticket/7160>
and as per my comments in that commit log, I think we should do it differently. I don't know how hard that would be though.
Incidentally, I implemented per-generation weak pointers in the local-gc branch, but didn't get around to porting it back over into the
list mainline (I
still have a ToDo for that). My version probably has the ordering bug, but you could always look at the branch to see how my approach compares to yours.
Cheers, Simon
participants (1)
-
Akio Takano