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

>
> Please, review the
> http://bugs.squeak.org/view.php?id=7473
>
> There are two sets of changesets - one for vmmaker and other is for
> language-side.
>
> A VMMaker changeset is based on VMMaker-dtl.159.
>
> Here the little benchmark, between old and new weak registries:
>
> { WeakRegistry. WeakFinalizationRegistry } collect: [:class |
>     | registry weaklings time1 time2 |
>     registry := class new.
>     WeakArray removeWeakDependent: registry.
>
>     weaklings := (1 to: 100000) collect: [:i | Object new ].
>     time1 := [ weaklings do: [:each | registry add: each ] ] timeToRun.
>     weaklings at: 100 put: nil.
>     Smalltalk garbageCollect; garbageCollect.
>     time2 := [ registry finalizeValues ] timeToRun.
>     time1 @ time2
> ]
>   {7816@41 . 4114@0}
>
> While its not much better at first benchmark (since using the same
> approach to store objects in one dictionary),
> while other is significant, since there is no longer need to scan a
> whole collection to detect an items which become a garbage.
>
> Btw, i wonder, why current WeakRegistry using a WeakKeyDictionary
> instead of WeakIdentityKeyDictionary?
> Isn't a weak refs is identity-based?
>
> I am also a bit wonder, why WeakRegistry has to support a copy protocol?
> It may lead to unpredictable behavior once you try to copy such kind
> of container, no matter how well you protect it.
>

>
> Please, review the
> http://bugs.squeak.org/view.php?id=7473
>
> There are two sets of changesets - one for vmmaker and other is for
> language-side.
>
> A VMMaker changeset is based on VMMaker-dtl.159.
>
> Here the little benchmark, between old and new weak registries:
>
> { WeakRegistry. WeakFinalizationRegistry } collect: [:class |
>   | registry weaklings time1 time2 |
>   registry := class new.
>   WeakArray removeWeakDependent: registry.
>
>   weaklings := (1 to: 100000) collect: [:i | Object new ].
>   time1 := [ weaklings do: [:each | registry add: each ] ] timeToRun.
>   weaklings at: 100 put: nil.
>   Smalltalk garbageCollect; garbageCollect.
>   time2 := [ registry finalizeValues ] timeToRun.
>   time1 @ time2
> ]
> {7816@41 . 4114@0}
>
> While its not much better at first benchmark (since using the same
> approach to store objects in one dictionary),

> Cheers,
>  - Andreas
>
> On 3/8/2010 1:31 PM, Igor Stasenko wrote:
>>
>> Please, review the
>> http://bugs.squeak.org/view.php?id=7473
>>
>> There are two sets of changesets - one for vmmaker and other is for
>> language-side.
>>
>> A VMMaker changeset is based on VMMaker-dtl.159.
>>
>> Here the little benchmark, between old and new weak registries:
>>
>> { WeakRegistry. WeakFinalizationRegistry } collect: [:class |
>>    | registry weaklings time1 time2 |
>>    registry := class new.
>>    WeakArray removeWeakDependent: registry.
>>
>>    weaklings := (1 to: 100000) collect: [:i | Object new ].
>>    time1 := [ weaklings do: [:each | registry add: each ] ] timeToRun.
>>    weaklings at: 100 put: nil.
>>    Smalltalk garbageCollect; garbageCollect.
>>    time2 := [ registry finalizeValues ] timeToRun.
>>    time1 @ time2
>> ]
>>  {7816@41 . 4114@0}
>>
>> While its not much better at first benchmark (since using the same
>> approach to store objects in one dictionary),
>> while other is significant, since there is no longer need to scan a
>> whole collection to detect an items which become a garbage.
>>
>> Btw, i wonder, why current WeakRegistry using a WeakKeyDictionary
>> instead of WeakIdentityKeyDictionary?
>> Isn't a weak refs is identity-based?
>>
>> I am also a bit wonder, why WeakRegistry has to support a copy protocol?
>> It may lead to unpredictable behavior once you try to copy such kind
>> of container, no matter how well you protect it.
>>
>

>
> On Mon, 8 Mar 2010, Igor Stasenko wrote:
>
>>
>> Please, review the
>> http://bugs.squeak.org/view.php?id=7473
>>
>> There are two sets of changesets - one for vmmaker and other is for
>> language-side.
>>
>> A VMMaker changeset is based on VMMaker-dtl.159.
>>
>> Here the little benchmark, between old and new weak registries:
>>
>> { WeakRegistry. WeakFinalizationRegistry } collect: [:class |
>>  | registry weaklings time1 time2 |
>>  registry := class new.
>>  WeakArray removeWeakDependent: registry.
>>
>>  weaklings := (1 to: 100000) collect: [:i | Object new ].
>>  time1 := [ weaklings do: [:each | registry add: each ] ] timeToRun.
>>  weaklings at: 100 put: nil.
>>  Smalltalk garbageCollect; garbageCollect.
>>  time2 := [ registry finalizeValues ] timeToRun.
>>  time1 @ time2
>> ]
>> {7816@41 . 4114@0}
>>
>> While its not much better at first benchmark (since using the same
>> approach to store objects in one dictionary),
>
> I took a quick look and found that you omitted the semaphore from
> WeakFinalizationRegistry. I think it won't work this way.
>

>> while other is significant, since there is no longer need to scan a
>> whole collection to detect an items which become a garbage.
>
> That's great.
>
>>
>> Btw, i wonder, why current WeakRegistry using a WeakKeyDictionary
>> instead of WeakIdentityKeyDictionary?
>> Isn't a weak refs is identity-based?
>>
>
> I found it strange too, but I think the users of WeakRegistry don't
> implement their own #hash and #=, though I didn't check that. Using a
> WeakIdentityKeyDictionary could also mean better performance in most cases.
>
>> I am also a bit wonder, why WeakRegistry has to support a copy protocol?
>> It may lead to unpredictable behavior once you try to copy such kind
>> of container, no matter how well you protect it.
>
> I think copy is supported because WeakRegistry is a collection. I wonder how
> could you get the unpredictable behavior with the current implementation.
>

>
> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>
>>
>> On 9 March 2010 00:44, Levente Uzonyi <[hidden email]> wrote:
>>>
>>> On Mon, 8 Mar 2010, Igor Stasenko wrote:
>>>
>>>>
>>>> Please, review the
>>>> http://bugs.squeak.org/view.php?id=7473
>>>>
>>>> There are two sets of changesets - one for vmmaker and other is for
>>>> language-side.
>>>>
>>>> A VMMaker changeset is based on VMMaker-dtl.159.
>>>>
>>>> Here the little benchmark, between old and new weak registries:
>>>>
>>>> { WeakRegistry. WeakFinalizationRegistry } collect: [:class |
>>>>  | registry weaklings time1 time2 |
>>>>  registry := class new.
>>>>  WeakArray removeWeakDependent: registry.
>>>>
>>>>  weaklings := (1 to: 100000) collect: [:i | Object new ].
>>>>  time1 := [ weaklings do: [:each | registry add: each ] ] timeToRun.
>>>>  weaklings at: 100 put: nil.
>>>>  Smalltalk garbageCollect; garbageCollect.
>>>>  time2 := [ registry finalizeValues ] timeToRun.
>>>>  time1 @ time2
>>>> ]
>>>> {7816@41 . 4114@0}
>>>>
>>>> While its not much better at first benchmark (since using the same
>>>> approach to store objects in one dictionary),
>>>
>>> I took a quick look and found that you omitted the semaphore from
>>> WeakFinalizationRegistry. I think it won't work this way.
>>>
>>
>> The point is, that new implementation don't touching the
>> 'valueDictionary' during finalization,
>> and therefore from this side, there is no concurrency issues.
>>
>> Adding/accessing items in the list is not protected. But i think the
>> main reason why we having a semaphore here is to protect from
>> interrupts caused by garbage collector and finalization process.
>> If you think we should also protect it from concurrent access , when
>> populating it - i could add the semaphores.
>
> I think it's a must. For example you can always open two sockets or two files concurrently. Or close one while opening another, etc.
>

>>
>>>> while other is significant, since there is no longer need to scan a
>>>> whole collection to detect an items which become a garbage.
>>>
>>> That's great.
>>>
>>>>
>>>> Btw, i wonder, why current WeakRegistry using a WeakKeyDictionary
>>>> instead of WeakIdentityKeyDictionary?
>>>> Isn't a weak refs is identity-based?
>>>>
>>>
>>> I found it strange too, but I think the users of WeakRegistry don't
>>> implement their own #hash and #=, though I didn't check that. Using a
>>> WeakIdentityKeyDictionary could also mean better performance in most cases.
>>>
>>>> I am also a bit wonder, why WeakRegistry has to support a copy protocol?
>>>> It may lead to unpredictable behavior once you try to copy such kind
>>>> of container, no matter how well you protect it.
>>>
>>> I think copy is supported because WeakRegistry is a collection. I wonder how
>>> could you get the unpredictable behavior with the current implementation.
>>>
>>
>> well, potentially, it could lead to making finalization twice (by each
>> registry object), since when you copying registry you doubling the
>> number of references to all executor objects which is held strongly by
>> registry.
>
> I think it's the responsibility of the user of #copy to avoid that. And the user doesn't have to do much, because the copy is not registered in WeakArray for finalization, so sending #copy to WeakRegistry won't cause any harm IMHO.
>

>
> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>
>>
>> 2010/3/9 Levente Uzonyi <[hidden email]>:
>>>
>>> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>>>
>>>>
>>>> On 9 March 2010 00:44, Levente Uzonyi <[hidden email]> wrote:
>>>>>
>>>>> On Mon, 8 Mar 2010, Igor Stasenko wrote:
>>>>>
>>>>>>
>>>>>> Please, review the
>>>>>> http://bugs.squeak.org/view.php?id=7473
>>>>>>
>>>>>> There are two sets of changesets - one for vmmaker and other is for
>>>>>> language-side.
>>>>>>
>>>>>> A VMMaker changeset is based on VMMaker-dtl.159.
>>>>>>
>>>>>> Here the little benchmark, between old and new weak registries:
>>>>>>
>>>>>> { WeakRegistry. WeakFinalizationRegistry } collect: [:class |
>>>>>>  | registry weaklings time1 time2 |
>>>>>>  registry := class new.
>>>>>>  WeakArray removeWeakDependent: registry.
>>>>>>
>>>>>>  weaklings := (1 to: 100000) collect: [:i | Object new ].
>>>>>>  time1 := [ weaklings do: [:each | registry add: each ] ] timeToRun.
>>>>>>  weaklings at: 100 put: nil.
>>>>>>  Smalltalk garbageCollect; garbageCollect.
>>>>>>  time2 := [ registry finalizeValues ] timeToRun.
>>>>>>  time1 @ time2
>>>>>> ]
>>>>>> {7816@41 . 4114@0}
>>>>>>
>>>>>> While its not much better at first benchmark (since using the same
>>>>>> approach to store objects in one dictionary),
>>>>>
>>>>> I took a quick look and found that you omitted the semaphore from
>>>>> WeakFinalizationRegistry. I think it won't work this way.
>>>>>
>>>>
>>>> The point is, that new implementation don't touching the
>>>> 'valueDictionary' during finalization,
>>>> and therefore from this side, there is no concurrency issues.
>>>>
>>>> Adding/accessing items in the list is not protected. But i think the
>>>> main reason why we having a semaphore here is to protect from
>>>> interrupts caused by garbage collector and finalization process.
>>>> If you think we should also protect it from concurrent access , when
>>>> populating it - i could add the semaphores.
>>>
>>> I think it's a must. For example you can always open two sockets or two files concurrently. Or close one while opening another, etc.
>>>
>> No problem, it could be added quite easily.
>>
>>>>
>>>>>> while other is significant, since there is no longer need to scan a
>>>>>> whole collection to detect an items which become a garbage.
>>>>>
>>>>> That's great.
>>>>>
>>>>>>
>>>>>> Btw, i wonder, why current WeakRegistry using a WeakKeyDictionary
>>>>>> instead of WeakIdentityKeyDictionary?
>>>>>> Isn't a weak refs is identity-based?
>>>>>>
>>>>>
>>>>> I found it strange too, but I think the users of WeakRegistry don't
>>>>> implement their own #hash and #=, though I didn't check that. Using a
>>>>> WeakIdentityKeyDictionary could also mean better performance in most cases.
>>>>>
>>>>>> I am also a bit wonder, why WeakRegistry has to support a copy protocol?
>>>>>> It may lead to unpredictable behavior once you try to copy such kind
>>>>>> of container, no matter how well you protect it.
>>>>>
>>>>> I think copy is supported because WeakRegistry is a collection. I wonder how
>>>>> could you get the unpredictable behavior with the current implementation.
>>>>>
>>>>
>>>> well, potentially, it could lead to making finalization twice (by each
>>>> registry object), since when you copying registry you doubling the
>>>> number of references to all executor objects which is held strongly by
>>>> registry.
>>>
>>> I think it's the responsibility of the user of #copy to avoid that. And the user doesn't have to do much, because the copy is not registered in WeakArray for finalization, so sending #copy to WeakRegistry won't cause any harm IMHO.
>>>
>>
>> why allowing user to go into absurdly direction? Why just
>> #shouldNotImplement it?
>> Do you see any good and practical uses of weakregistry copies?
>
> I don't see any use of it, but someone else may. We allow everyone to evaluate Object superclass: Object, even though we know that it will crash the system.
>

>
> Levente
>
>>
>>>
>>> Levente
>>>
>>
>> --
>> Best regards,
>> Igor Stasenko AKA sig.
>
>

>
> 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.
>

> Cheers,
>  - Andreas
>
> On 3/8/2010 3:14 PM, Igor Stasenko wrote:
>>
>> Oh, and this actually an implementation of idea, we discussed earlier
>>
>> http://lists.squeakfoundation.org/pipermail/vm-dev/2009-April/002572.html
>>
>> While its not an ephemerons, its much more easier to adopt because
>> there is no need in making heavy changes to GC.
>>
>

> On 9 March 2010 02:06, Andreas Raab <[hidden email]> 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.
>
> 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 :)
>
> 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.
>
> - the weak finalization check runs during sweep phase before
> compaction. Which means, that objects is not changing their locations
> during that phase.
>
>

>> Cheers,
>>  - Andreas
>>
>> On 3/8/2010 3:14 PM, Igor Stasenko wrote:
>>>
>>> Oh, and this actually an implementation of idea, we discussed earlier
>>>
>>> http://lists.squeakfoundation.org/pipermail/vm-dev/2009-April/002572.html
>>>
>>> While its not an ephemerons, its much more easier to adopt because
>>> there is no need in making heavy changes to GC.
>>>
>>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>

>
> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>
>>
>> 2010/3/9 Levente Uzonyi <[hidden email]>:
>>>
>>> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>>>
>>>>
>>>> 2010/3/9 Levente Uzonyi <[hidden email]>:
>>>>>
>>>>> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>>>>>
>>>>>>
>>>>>> On 9 March 2010 00:44, Levente Uzonyi <[hidden email]> wrote:
>>>>>>>
>>>>>>> On Mon, 8 Mar 2010, Igor Stasenko wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> Please, review the
>>>>>>>> http://bugs.squeak.org/view.php?id=7473
>>>>>>>>
>>>>>>>> There are two sets of changesets - one for vmmaker and other is for
>>>>>>>> language-side.
>>>>>>>>
>>>>>>>> A VMMaker changeset is based on VMMaker-dtl.159.
>>>>>>>>
>>>>>>>> Here the little benchmark, between old and new weak registries:
>>>>>>>>
>>>>>>>> { WeakRegistry. WeakFinalizationRegistry } collect: [:class |
>>>>>>>>  | registry weaklings time1 time2 |
>>>>>>>>  registry := class new.
>>>>>>>>  WeakArray removeWeakDependent: registry.
>>>>>>>>
>>>>>>>>  weaklings := (1 to: 100000) collect: [:i | Object new ].
>>>>>>>>  time1 := [ weaklings do: [:each | registry add: each ] ] timeToRun.
>>>>>>>>  weaklings at: 100 put: nil.
>>>>>>>>  Smalltalk garbageCollect; garbageCollect.
>>>>>>>>  time2 := [ registry finalizeValues ] timeToRun.
>>>>>>>>  time1 @ time2
>>>>>>>> ]
>>>>>>>> {7816@41 . 4114@0}
>>>>>>>>
>>>>>>>> While its not much better at first benchmark (since using the same
>>>>>>>> approach to store objects in one dictionary),
>>>>>>>
>>>>>>> I took a quick look and found that you omitted the semaphore from
>>>>>>> WeakFinalizationRegistry. I think it won't work this way.
>>>>>>>
>>>>>>
>>>>>> The point is, that new implementation don't touching the
>>>>>> 'valueDictionary' during finalization,
>>>>>> and therefore from this side, there is no concurrency issues.
>>>>>>
>>>>>> Adding/accessing items in the list is not protected. But i think the
>>>>>> main reason why we having a semaphore here is to protect from
>>>>>> interrupts caused by garbage collector and finalization process.
>>>>>> If you think we should also protect it from concurrent access , when
>>>>>> populating it - i could add the semaphores.
>>>>>
>>>>> I think it's a must. For example you can always open two sockets or two files concurrently. Or close one while opening another, etc.
>>>>>
>>>> No problem, it could be added quite easily.
>>>>
>>>>>>
>>>>>>>> while other is significant, since there is no longer need to scan a
>>>>>>>> whole collection to detect an items which become a garbage.
>>>>>>>
>>>>>>> That's great.
>>>>>>>
>>>>>>>>
>>>>>>>> Btw, i wonder, why current WeakRegistry using a WeakKeyDictionary
>>>>>>>> instead of WeakIdentityKeyDictionary?
>>>>>>>> Isn't a weak refs is identity-based?
>>>>>>>>
>>>>>>>
>>>>>>> I found it strange too, but I think the users of WeakRegistry don't
>>>>>>> implement their own #hash and #=, though I didn't check that. Using a
>>>>>>> WeakIdentityKeyDictionary could also mean better performance in most cases.
>>>>>>>
>>>>>>>> I am also a bit wonder, why WeakRegistry has to support a copy protocol?
>>>>>>>> It may lead to unpredictable behavior once you try to copy such kind
>>>>>>>> of container, no matter how well you protect it.
>>>>>>>
>>>>>>> I think copy is supported because WeakRegistry is a collection. I wonder how
>>>>>>> could you get the unpredictable behavior with the current implementation.
>>>>>>>
>>>>>>
>>>>>> well, potentially, it could lead to making finalization twice (by each
>>>>>> registry object), since when you copying registry you doubling the
>>>>>> number of references to all executor objects which is held strongly by
>>>>>> registry.
>>>>>
>>>>> I think it's the responsibility of the user of #copy to avoid that. And the user doesn't have to do much, because the copy is not registered in WeakArray for finalization, so sending #copy to WeakRegistry won't cause any harm IMHO.
>>>>>
>>>>
>>>> why allowing user to go into absurdly direction? Why just
>>>> #shouldNotImplement it?
>>>> Do you see any good and practical uses of weakregistry copies?
>>>
>>> I don't see any use of it, but someone else may. We allow everyone to evaluate Object superclass: Object, even though we know that it will crash the system.
>>>
>>
>> Well, if someone cares, then he actually can make own registry class,
>> which allows copying. But why we should care, by leaving a potential
>> security hole open?
>> I don't think that this is normal to rely on good manners of users and
>> expect them to not attempt to do something wrong. In contrast, a
>> protocols and interfaces should discourage user from abuse. At least,
>> an author could state where it is safe to use and where is not.
>> A copy protocol for weak registry is far from being safe.
>> In that situation is better to generate an error, indicating that such
>> use is not foreseen by author, rather than trying to implement
>> something which 'possibly could work' :)
>
> I still don't see how is it unsafe.
>

>
> On 9 March 2010 02:06, Andreas Raab<[hidden email]>  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.

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

>
> 2010/3/9 Levente Uzonyi <[hidden email]>:
>>
>> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>>
>>> Well, if someone cares, then he actually can make own registry class,
>>> which allows copying. But why we should care, by leaving a potential
>>> security hole open?
>>> I don't think that this is normal to rely on good manners of users and
>>> expect them to not attempt to do something wrong. In contrast, a
>>> protocols and interfaces should discourage user from abuse. At least,
>>> an author could state where it is safe to use and where is not.
>>> A copy protocol for weak registry is far from being safe.
>>> In that situation is better to generate an error, indicating that such
>>> use is not foreseen by author, rather than trying to implement
>>> something which 'possibly could work' :)
>>
>> I still don't see how is it unsafe.
>>
>
> A simple copy is fine. A copy, which then registered in weak
> dependents creating a problem.
>
> I even think that weak registry should not behave as collection at all,
> and having only #add: method, with no ability to remove items.
>
> The only use of #remove: i see is in Socket and StandardFileStream,
> which implement #unregister: in own class side.
>
> Now , if you look at my implementation, you could see that there is a
> way to completely avoid the need in removing items from a
> valueDictionary which is pairs of  <weakref> -> <executor>.
>
> A solution:
> - add a 'finalizer' ivar to Socket/StandardFileStream
> - by registering a socket in registry, retrieve an instance of
> WeakFinalizerItem as a result of registration and store it into
> 'finalizer' ivar.
>
> - on #destroy, simply nil-out all of the finalizer's ivars, so there
> is no chances that once socket become garbage, it will trigger an
> executor's #finalize method, which were registered previously in
> registry.
>
> - forget about removing the finalizer manually from registry, because
> an instance of WeakFinalizerItem which is held by 'valueDictionary' in
> registry will eventually be reclaimed, once dictionary will discover
> that corresponding key is nil.
>
> What you think?

>
>>
>> Levente
>>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>

>
> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>
>>
>> 2010/3/9 Levente Uzonyi <[hidden email]>:
>>>
>>> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>>>
>>>> Well, if someone cares, then he actually can make own registry class,
>>>> which allows copying. But why we should care, by leaving a potential
>>>> security hole open?
>>>> I don't think that this is normal to rely on good manners of users and
>>>> expect them to not attempt to do something wrong. In contrast, a
>>>> protocols and interfaces should discourage user from abuse. At least,
>>>> an author could state where it is safe to use and where is not.
>>>> A copy protocol for weak registry is far from being safe.
>>>> In that situation is better to generate an error, indicating that such
>>>> use is not foreseen by author, rather than trying to implement
>>>> something which 'possibly could work' :)
>>>
>>> I still don't see how is it unsafe.
>>>
>>
>> A simple copy is fine. A copy, which then registered in weak
>> dependents creating a problem.
>>
>> I even think that weak registry should not behave as collection at all,
>> and having only #add: method, with no ability to remove items.
>>
>> The only use of #remove: i see is in Socket and StandardFileStream,
>> which implement #unregister: in own class side.
>>
>> Now , if you look at my implementation, you could see that there is a
>> way to completely avoid the need in removing items from a
>> valueDictionary which is pairs of  <weakref> -> <executor>.
>>
>> A solution:
>> - add a 'finalizer' ivar to Socket/StandardFileStream
>> - by registering a socket in registry, retrieve an instance of
>> WeakFinalizerItem as a result of registration and store it into
>> 'finalizer' ivar.
>>
>> - on #destroy, simply nil-out all of the finalizer's ivars, so there
>> is no chances that once socket become garbage, it will trigger an
>> executor's #finalize method, which were registered previously in
>> registry.
>>
>> - forget about removing the finalizer manually from registry, because
>> an instance of WeakFinalizerItem which is held by 'valueDictionary' in
>> registry will eventually be reclaimed, once dictionary will discover
>> that corresponding key is nil.
>>
>> What you think?
>
> I took a deeper look and found that WeakFinalizationRegistry doesn't support
> multiple finalizers per object. It does what WeakRegistry did before: throw
> away the previous finalizer and replace it with a new one.
>
> I like the current features of WeakRegistry (removing, adding, multiple
> finalizers per object) and I think it's easy to modify it to use your vm
> support.
>

> I don't really like your suggestion for files and sockets, but it's doable.
>
> I was thinking about a simpler registry which would use an OrderedCollection
> instead of a WeakKeyDictionary. It'd need a new instance variable in
> WeakFinalizerItem (though it can be another class/subclass too) which would
> store the index of the finalizer in this OrderedCollection. It wouldn't have
> #do:, #keys or #remove:ifAbsent: (though all of them could be implemented)
> and #add: wouldn't replace existing finalizers, but just add them to the
> registry.
> This would have a bit better performance, simpler implementation and less
> features. But if you don't need #remove:, i'm sure it'd fit your needs.
> (It's a bit tricky though. Your socket/file ideas wouldn't work out of the
> box, because all of the WeakFinalizerItems would have to be removed from the
> OrderedCollecion somehow to avoid leaking memory. One solution would be to
> add it to the list on "remove" (when replacing the ivars with nil), another
> would be to remove it from the OrderedCollection by index, but the items
> don't know their collection so it would need another ivar).
>
> All in all, I'd keep WeakRegistry with the current features and optionally
> add a new simpler and faster registry if needed.
>

> On 10 March 2010 04:34, Levente Uzonyi <[hidden email]> wrote:
>>
>> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>>
>>>
>>> 2010/3/9 Levente Uzonyi <[hidden email]>:
>>>>
>>>> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>>>>
>>>>> Well, if someone cares, then he actually can make own registry class,
>>>>> which allows copying. But why we should care, by leaving a potential
>>>>> security hole open?
>>>>> I don't think that this is normal to rely on good manners of users and
>>>>> expect them to not attempt to do something wrong. In contrast, a
>>>>> protocols and interfaces should discourage user from abuse. At least,
>>>>> an author could state where it is safe to use and where is not.
>>>>> A copy protocol for weak registry is far from being safe.
>>>>> In that situation is better to generate an error, indicating that such
>>>>> use is not foreseen by author, rather than trying to implement
>>>>> something which 'possibly could work' :)
>>>>
>>>> I still don't see how is it unsafe.
>>>>
>>>
>>> A simple copy is fine. A copy, which then registered in weak
>>> dependents creating a problem.
>>>
>>> I even think that weak registry should not behave as collection at all,
>>> and having only #add: method, with no ability to remove items.
>>>
>>> The only use of #remove: i see is in Socket and StandardFileStream,
>>> which implement #unregister: in own class side.
>>>
>>> Now , if you look at my implementation, you could see that there is a
>>> way to completely avoid the need in removing items from a
>>> valueDictionary which is pairs of  <weakref> -> <executor>.
>>>
>>> A solution:
>>> - add a 'finalizer' ivar to Socket/StandardFileStream
>>> - by registering a socket in registry, retrieve an instance of
>>> WeakFinalizerItem as a result of registration and store it into
>>> 'finalizer' ivar.
>>>
>>> - on #destroy, simply nil-out all of the finalizer's ivars, so there
>>> is no chances that once socket become garbage, it will trigger an
>>> executor's #finalize method, which were registered previously in
>>> registry.
>>>
>>> - forget about removing the finalizer manually from registry, because
>>> an instance of WeakFinalizerItem which is held by 'valueDictionary' in
>>> registry will eventually be reclaimed, once dictionary will discover
>>> that corresponding key is nil.
>>>
>>> What you think?
>>
>> I took a deeper look and found that WeakFinalizationRegistry doesn't support
>> multiple finalizers per object. It does what WeakRegistry did before: throw
>> away the previous finalizer and replace it with a new one.
>>
>> I like the current features of WeakRegistry (removing, adding, multiple
>> finalizers per object) and I think it's easy to modify it to use your vm
>> support.
>>
>
> Sure, support for multiple finalizers per object could be added. But
> as to me, this looks like an over-engineering.
>
> Btw, the current implementation of WeakRegistry is buggy, because of
> use non-identity based dictionary.
> Try explore contents of it, after evaluating:
>
> 10 timesRepeat: [ registry add: (Array with:10) ].
> it adds only a single key/value pair into valueDictionary,
> while i'd expect to have 10 entries, because i adding 10 distinct array objects.
> The flaw in logic is easy to illustrate:
>
> array1 := Array with: 10.
> array2 := Array with: 10.
>
> registry add: array1; array2.

> array2 at: 1 put: 12.
> registry add: array2.
>
> array1 := nil "remove strong reference to array1, while keep it for array2"
>
> The point is, that if two disctinct objects answering true on #= ,
> when we adding them to registry
> it doesn't means that they will keep to be 'equal' after we added
> them, because these objects can mutate.
> So, if one of them eventually die, other(s) may still be in use, which
> will lead to receiving a death notice to wrong recipient.
>
>> I don't really like your suggestion for files and sockets, but it's doable.
>>
>> I was thinking about a simpler registry which would use an OrderedCollection
>> instead of a WeakKeyDictionary. It'd need a new instance variable in
>> WeakFinalizerItem (though it can be another class/subclass too) which would
>> store the index of the finalizer in this OrderedCollection. It wouldn't have
>> #do:, #keys or #remove:ifAbsent: (though all of them could be implemented)
>> and #add: wouldn't replace existing finalizers, but just add them to the
>> registry.
>> This would have a bit better performance, simpler implementation and less
>> features. But if you don't need #remove:, i'm sure it'd fit your needs.
>> (It's a bit tricky though. Your socket/file ideas wouldn't work out of the
>> box, because all of the WeakFinalizerItems would have to be removed from the
>> OrderedCollecion somehow to avoid leaking memory. One solution would be to
>> add it to the list on "remove" (when replacing the ivars with nil), another
>> would be to remove it from the OrderedCollection by index, but the items
>> don't know their collection so it would need another ivar).
>>
>> All in all, I'd keep WeakRegistry with the current features and optionally
>> add a new simpler and faster registry if needed.
>>
>
> Yes, i'm also thought about different ways how to organize things in
> less memory & CPU hungry manner.
> The main role of weak registry is to receive a notification, when
> particular object become garbage.
> Obviously, for this, we need to store a reference to notification
> handler (also known as executor) somewhere to
> be able to handle it. That's why current implementation using WeakKeyDictionary.
> Once such notification is handled, we usually no longer need to keep
> this object as well (thus we have to remove/unlink it from registry).
>
> My implementation is not perfect, it was mainly a quick and dirty hack
> (you pointed on not using semaphores already).
> But my intent was mainly to show how to use new VM feature, i added.
> So, once it will be approved (i hope) and adopted, we could implement
> things properly.
>
>>
>> Levente
>>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>