On 3/8/2010 4:45 PM, Igor Stasenko wrote:
On 9 March 2010 02:06, Andreas Raabandreas.raab@gmx.de wrote:
Thanks Igor - if you have been implementing what you've described in this email I'm all for it. Give me a bit of time to review the code since this stuff can be tricky.
One thing I do recall and that you may want to check is that testing class membership can be tricky during GC - in fact, the introduction of a weak format was due to issues with determining the class of some object correctly. Unfortunately, I do not recall the exact details but it's worthwhile going through this very carefully and make sure that one can actually rely on a) the class pointer being valid (and not substituted while traversing the object) and b) the pointer in splObjs be valid (and not substituted).
I'll give this special attention while looking through the code.
Yes, this is a biggest of my concern.
You know what, I've been looking at it and it looks safe to me. I also remembered why it was originally unsafe - it wasn't sweep phase that's the problem it is mark phase, where the headers get replaced you can't do a proper class membership test (but you need to avoid tracing weak references). During sweep, where you hook in, there is no such problem.
Also maybe i should use a #longAt:put: , or uncheckedXXX put: (forgot the selector) instead of self storePointer: 1 ofObject: oop withValue:
because storePointer using the possibleRootStoreIntoValue(), which could have unwanted effects :)
I think using storePointer:ofObject:withValue is the right thing to do. The roots table will be cleared if this happens during fullGC, but if this happens during IGC, and either list or finalizer are old, then they must be properly recorded as root.
Maybe it could help you with analyzis:
- the code touching a strong references , where all of them is
already marked as reachable (because oop is reachable, and hence the list ivar, and hence the list's 'first' ivar). So, its only rearranging them a bit, but doesn't making none of them unreachable, or even worse - making an already non-reachable object be reachable again.
Yes, that sounds right.
- the weak finalization check runs during sweep phase before
compaction. Which means, that objects is not changing their locations during that phase.
It sounds good to me. I *really* like how small the changes are - it makes it much easier to verify correctness. I think what we should be doing is build VMs to test it and run it for a while (which is easy enough between us).
BTW, one tiny thing we'll have to fix is to add a test for splObj array - the way the code is written right now it could run afoul of an old image that doesn't have anything registered as ClassWeakFinalizer.
But other than that, great job!
Cheers, - Andreas