[Vm-dev] [ENH] A better finalization support (VM & language side)

[Andreas Raab]

On 3/8/2010 4:45 PM, Igor Stasenko wrote:
>
> On 9 March 2010 02:06, Andreas Raab<andreas.raab at 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