Another fixes to finalization (was Re: [update 1.2] #12161 - #12172)

> I uploaded new changesets to
> http://code.google.com/p/pharo/issues/detail?id=3002
>
>
> I tested applying them in Pharo-Core-#12159
> and then updating an image after that.
>
> After update:
> MCMethodDefinition cachedDefinitions size is 18668
>
> But image don't feels slow.
>
> As a side note, i recommend to review the MC caching mechanism towards
> avoiding putting so much load
> on finalization process (such as scanning 18k entries to find dead objects).
>
> This is actually the purpose of new finalization scheme:
> - it allows to avoid scanning whole weak dictionary in order to get
> rid of expired associations.

>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

> On Fri, 1 Oct 2010, Igor Stasenko wrote:
>
>> I uploaded new changesets to
>> http://code.google.com/p/pharo/issues/detail?id=3002
>>
>>
>> I tested applying them in Pharo-Core-#12159
>> and then updating an image after that.
>>
>> After update:
>> MCMethodDefinition cachedDefinitions size is 18668
>>
>> But image don't feels slow.
>>
>> As a side note, i recommend to review the MC caching mechanism towards
>> avoiding putting so much load
>> on finalization process (such as scanning 18k entries to find dead
>> objects).
>>
>> This is actually the purpose of new finalization scheme:
>> - it allows to avoid scanning whole weak dictionary in order to get
>> rid of expired associations.
>
> WeakKeyDictionaries shouldn't be added to the finalization process at all.
> In the current case there's no finalization action at all, so it's totally
> pointless IMO. And these dictionaries are not thread-safe, so bad things can
> happen if they are registered with the finalization process.
>

>>
>> I'm already implemented a variant of self-cleaning weak dictionary for
>> Magma,
>> which using new finalization scheme, and avoids scanning huge number
>> of entries to discover few of them,
>> which became garbage.
>
> Squeak's WeakKeyDictionaries are self cleaning, though the there's no
> guarantee that finalization action is performed when the key is GC'd.
>

>
> Levente
>
>>
>>
>> --
>> Best regards,
>> Igor Stasenko AKA sig.
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

> Here the simple fix for it.
>
>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>

> Here the simple fix for it.
>
>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
> <mc-cache-fix.1.cs>_______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

> Stef
>
> On Oct 1, 2010, at 7:34 PM, Igor Stasenko wrote:
>
>> Here the simple fix for it.
>>
>>
>>
>>
>>
>> --
>> Best regards,
>> Igor Stasenko AKA sig.
>> <mc-cache-fix.1.cs>_______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

> On 2 October 2010 19:47, Stéphane Ducasse <[hidden email]> wrote:
>> Hi igor
>>
>> do you understand why the previous definition was
>>
>> cachedDefinitions
>>        Definitions ifNil: [Definitions := WeakIdentityKeyDictionary new.  WeakArray addWeakDependent: Definitions].
>>        ^ Definitions
>>
> i don't. :)
>
> It just a way to free memory as fast as possible.
> But its really not worth spending so much CPU in order to save few
> bytes of memory.

> On 2 October 2010 19:47, Stéphane Ducasse<[hidden email]>  wrote:
>> Hi igor
>>
>> do you understand why the previous definition was
>>
>> cachedDefinitions
>>         Definitions ifNil: [Definitions := WeakIdentityKeyDictionary new.  WeakArray addWeakDependent: Definitions].
>>         ^ Definitions
>>
> i don't. :)
>
> It just a way to free memory as fast as possible.
> But its really not worth spending so much CPU in order to save few
> bytes of memory.

>  On 02.10.2010 19:01, Igor Stasenko wrote:
>>
>> On 2 October 2010 19:47, Stéphane Ducasse<[hidden email]>
>>  wrote:
>>>
>>> Hi igor
>>>
>>> do you understand why the previous definition was
>>>
>>> cachedDefinitions
>>>        Definitions ifNil: [Definitions := WeakIdentityKeyDictionary new.
>>>  WeakArray addWeakDependent: Definitions].
>>>        ^ Definitions
>>>
>> i don't. :)
>>
>> It just a way to free memory as fast as possible.
>> But its really not worth spending so much CPU in order to save few
>> bytes of memory.
>
> Without registering the dict for finalization, the values won't ever be
> removed with the current implementation.

> On 3 October 2010 03:13, Henrik Sperre Johansen
> <[hidden email]>  wrote:
>>   On 02.10.2010 19:01, Igor Stasenko wrote:
>>> On 2 October 2010 19:47, Stéphane Ducasse<[hidden email]>
>>>   wrote:
>>>> Hi igor
>>>>
>>>> do you understand why the previous definition was
>>>>
>>>> cachedDefinitions
>>>>         Definitions ifNil: [Definitions := WeakIdentityKeyDictionary new.
>>>>   WeakArray addWeakDependent: Definitions].
>>>>         ^ Definitions
>>>>
>>> i don't. :)
>>>
>>> It just a way to free memory as fast as possible.
>>> But its really not worth spending so much CPU in order to save few
>>> bytes of memory.
>> Without registering the dict for finalization, the values won't ever be
>> removed with the current implementation.
> They are removed. After each package load/unload operation.
> I don't see a reason to do this more often.

>  On 03.10.2010 05:47, Igor Stasenko wrote:
>>
>> On 3 October 2010 03:13, Henrik Sperre Johansen
>> <[hidden email]>  wrote:
>>>
>>>  On 02.10.2010 19:01, Igor Stasenko wrote:
>>>>
>>>> On 2 October 2010 19:47, Stéphane Ducasse<[hidden email]>
>>>>  wrote:
>>>>>
>>>>> Hi igor
>>>>>
>>>>> do you understand why the previous definition was
>>>>>
>>>>> cachedDefinitions
>>>>>        Definitions ifNil: [Definitions := WeakIdentityKeyDictionary
>>>>> new.
>>>>>  WeakArray addWeakDependent: Definitions].
>>>>>        ^ Definitions
>>>>>
>>>> i don't. :)
>>>>
>>>> It just a way to free memory as fast as possible.
>>>> But its really not worth spending so much CPU in order to save few
>>>> bytes of memory.
>>>
>>> Without registering the dict for finalization, the values won't ever be
>>> removed with the current implementation.
>>
>> They are removed. After each package load/unload operation.
>> I don't see a reason to do this more often.
>
> If they are removed explicitly, then yes. But then again, if that's the
> case, why use a weak dictionary in the first place?

>>>
>>> Which can be somewhat hampering to performance once you have lots of
>>> nil-keyed elements all stashed from index 1 and onwards after a rehash.
>>>
>> In Pharo, weak dict knows how to reuse expired associations:
>>
> And when do they expire? Only when they are either finalized, or removed
> explicitly.
> So if you do neither, they will stay indefinately without being
> removed/reused.

> On 3 October 2010 12:36, Henrik Sperre Johansen
> <[hidden email]>  wrote:
>>   On 03.10.2010 05:47, Igor Stasenko wrote:
>>> On 3 October 2010 03:13, Henrik Sperre Johansen
>>> <[hidden email]>    wrote:
>>>>   On 02.10.2010 19:01, Igor Stasenko wrote:
>>>>> On 2 October 2010 19:47, Stéphane Ducasse<[hidden email]>
>>>>>   wrote:
>>>>>> Hi igor
>>>>>>
>>>>>> do you understand why the previous definition was
>>>>>>
>>>>>> cachedDefinitions
>>>>>>         Definitions ifNil: [Definitions := WeakIdentityKeyDictionary
>>>>>> new.
>>>>>>   WeakArray addWeakDependent: Definitions].
>>>>>>         ^ Definitions
>>>>>>
>>>>> i don't. :)
>>>>>
>>>>> It just a way to free memory as fast as possible.
>>>>> But its really not worth spending so much CPU in order to save few
>>>>> bytes of memory.
>>>> Without registering the dict for finalization, the values won't ever be
>>>> removed with the current implementation.
>>> They are removed. After each package load/unload operation.
>>> I don't see a reason to do this more often.
>> If they are removed explicitly, then yes. But then again, if that's the
>> case, why use a weak dictionary in the first place?
> i don't know much about MC to tell for sure.
>
>>>> Which can be somewhat hampering to performance once you have lots of
>>>> nil-keyed elements all stashed from index 1 and onwards after a rehash.
>>>>
>>> In Pharo, weak dict knows how to reuse expired associations:
>>>
>> And when do they expire? Only when they are either finalized, or removed
>> explicitly.
>> So if you do neither, they will stay indefinately without being
>> removed/reused.
> no. they expiring when key become nil.
>

> On 2 October 2010 19:47, Stéphane Ducasse <[hidden email]> wrote:
>> Hi igor
>>
>> do you understand why the previous definition was
>>
>> cachedDefinitions
>>        Definitions ifNil: [Definitions := WeakIdentityKeyDictionary new.  WeakArray addWeakDependent: Definitions].
>>        ^ Definitions
>>
> i don't. :)
>
> It just a way to free memory as fast as possible.
> But its really not worth spending so much CPU in order to save few
> bytes of memory.
>
>> Stef
>>
>> On Oct 1, 2010, at 7:34 PM, Igor Stasenko wrote:
>>
>>> Here the simple fix for it.
>>>
>>>
>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Igor Stasenko AKA sig.
>>> <mc-cache-fix.1.cs>_______________________________________________
>>> Pharo-project mailing list
>>> [hidden email]
>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

>  On 03.10.2010 11:59, Igor Stasenko wrote:
>>
>> On 3 October 2010 12:36, Henrik Sperre Johansen
>> <[hidden email]>  wrote:
>>>
>>>  On 03.10.2010 05:47, Igor Stasenko wrote:
>>>>
>>>> On 3 October 2010 03:13, Henrik Sperre Johansen
>>>> <[hidden email]>    wrote:
>>>>>
>>>>>  On 02.10.2010 19:01, Igor Stasenko wrote:
>>>>>>
>>>>>> On 2 October 2010 19:47, Stéphane Ducasse<[hidden email]>
>>>>>>  wrote:
>>>>>>>
>>>>>>> Hi igor
>>>>>>>
>>>>>>> do you understand why the previous definition was
>>>>>>>
>>>>>>> cachedDefinitions
>>>>>>>        Definitions ifNil: [Definitions := WeakIdentityKeyDictionary
>>>>>>> new.
>>>>>>>  WeakArray addWeakDependent: Definitions].
>>>>>>>        ^ Definitions
>>>>>>>
>>>>>> i don't. :)
>>>>>>
>>>>>> It just a way to free memory as fast as possible.
>>>>>> But its really not worth spending so much CPU in order to save few
>>>>>> bytes of memory.
>>>>>
>>>>> Without registering the dict for finalization, the values won't ever be
>>>>> removed with the current implementation.
>>>>
>>>> They are removed. After each package load/unload operation.
>>>> I don't see a reason to do this more often.
>>>
>>> If they are removed explicitly, then yes. But then again, if that's the
>>> case, why use a weak dictionary in the first place?
>>
>> i don't know much about MC to tell for sure.
>>
>>>>> Which can be somewhat hampering to performance once you have lots of
>>>>> nil-keyed elements all stashed from index 1 and onwards after a rehash.
>>>>>
>>>> In Pharo, weak dict knows how to reuse expired associations:
>>>>
>>> And when do they expire? Only when they are either finalized, or removed
>>> explicitly.
>>> So if you do neither, they will stay indefinately without being
>>> removed/reused.
>>
>> no. they expiring when key become nil.
>>
> You'd think so, yes.
> However, see implementor of WeakKeyAssociation>>#expired, and senders of
> #expire.
>

> On 2 October 2010 19:47, Stéphane Ducasse<[hidden email]>
>>>>>>>  wrote:
>>>>>>>>
>>>>>>>> Hi igor
>>>>>>>>
>>>>>>>> do you understand why the previous definition was
>>>>>>>>
>>>>>>>> cachedDefinitions
>>>>>>>>        Definitions ifNil: [Definitions := WeakIdentityKeyDictionary
>>>>>>>> new.
>>>>>>>>  WeakArray addWeakDependent: Definitions].
>>>>>>>>        ^ Definitions
>>>>>>>>
>>>>>>> i don't. :)
>>>>>>>
>>>>>>> It just a way to free memory as fast as possible.
>>>>>>> But its really not worth spending so much CPU in order to save few
>>>>>>> bytes of memory.
>>>>>>
>>>>>> Without registering the dict for finalization, the values won't ever be
>>>>>> removed with the current implementation.
>>>>>
>>>>> They are removed. After each package load/unload operation.
>>>>> I don't see a reason to do this more often.
>>>>
>>>> If they are removed explicitly, then yes. But then again, if that's the
>>>> case, why use a weak dictionary in the first place?
>>>
>>> i don't know much about MC to tell for sure.
>>>
>>>>>> Which can be somewhat hampering to performance once you have lots of
>>>>>> nil-keyed elements all stashed from index 1 and onwards after a rehash.
>>>>>>
>>>>> In Pharo, weak dict knows how to reuse expired associations:
>>>>>
>>>> And when do they expire? Only when they are either finalized, or removed
>>>> explicitly.
>>>> So if you do neither, they will stay indefinately without being
>>>> removed/reused.
>>>
>>> no. they expiring when key become nil.
>>>
>> You'd think so, yes.
>> However, see implementor of WeakKeyAssociation>>#expired, and senders of
>> #expire.
>>
>
> Oh, you are right. They are not reused unless marked as expired
> (associatin's value == association).
> But concerning speed, then my benchmark should show an opposite results. :)
>
> cache holding a pairs of:
> compiledmethod -> MCMethodDefinitions.
>
> Compiled methods usually gone when you either recompiling class or
> removing it from system.
> This is why i put #finalizeValues after package loading/unloading.
> But if you think it worth adding it into some other places, then tell me where.
> Still, putting it into FinalizationDependents is bad idea.
>
>> Cheers,
>> Henry
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

>
>> On 2 October 2010 19:47, Stéphane Ducasse<[hidden email]>
>>>>>>>>  wrote:
>>>>>>>>>
>>>>>>>>> Hi igor
>>>>>>>>>
>>>>>>>>> do you understand why the previous definition was
>>>>>>>>>
>>>>>>>>> cachedDefinitions
>>>>>>>>>        Definitions ifNil: [Definitions := WeakIdentityKeyDictionary
>>>>>>>>> new.
>>>>>>>>>  WeakArray addWeakDependent: Definitions].
>>>>>>>>>        ^ Definitions
>>>>>>>>>
>>>>>>>> i don't. :)
>>>>>>>>
>>>>>>>> It just a way to free memory as fast as possible.
>>>>>>>> But its really not worth spending so much CPU in order to save few
>>>>>>>> bytes of memory.
>>>>>>>
>>>>>>> Without registering the dict for finalization, the values won't ever be
>>>>>>> removed with the current implementation.
>>>>>>
>>>>>> They are removed. After each package load/unload operation.
>>>>>> I don't see a reason to do this more often.
>>>>>
>>>>> If they are removed explicitly, then yes. But then again, if that's the
>>>>> case, why use a weak dictionary in the first place?
>>>>
>>>> i don't know much about MC to tell for sure.
>>>>
>>>>>>> Which can be somewhat hampering to performance once you have lots of
>>>>>>> nil-keyed elements all stashed from index 1 and onwards after a rehash.
>>>>>>>
>>>>>> In Pharo, weak dict knows how to reuse expired associations:
>>>>>>
>>>>> And when do they expire? Only when they are either finalized, or removed
>>>>> explicitly.
>>>>> So if you do neither, they will stay indefinately without being
>>>>> removed/reused.
>>>>
>>>> no. they expiring when key become nil.
>>>>
>>> You'd think so, yes.
>>> However, see implementor of WeakKeyAssociation>>#expired, and senders of
>>> #expire.
>>>
>>
>> Oh, you are right. They are not reused unless marked as expired
>> (associatin's value == association).
>> But concerning speed, then my benchmark should show an opposite results. :)
>>
>> cache holding a pairs of:
>> compiledmethod -> MCMethodDefinitions.
>>
>> Compiled methods usually gone when you either recompiling class or
>> removing it from system.
>> This is why i put #finalizeValues after package loading/unloading.
>> But if you think it worth adding it into some other places, then tell me where.
>> Still, putting it into FinalizationDependents is bad idea.
>>
>>> Cheers,
>>> Henry
>>>
>>> _______________________________________________
>>> Pharo-project mailing list
>>> [hidden email]
>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>>
>>
>> --
>> Best regards,
>> Igor Stasenko AKA sig.
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

> On 4 October 2010 22:09, Stéphane Ducasse<[hidden email]>  wrote:
>> so let us know in the bug entry what is the conclusion :)
>>
> I think someone should verify my benchmarks i.e.
>
> [ self loadsomething ] timeToRun
>
> before and after patch.
>
> And conclusion is better be written by Henrik, because he's having
> concerns about speed,
> while i don't. :)

>  On 04.10.2010 21:47, Igor Stasenko wrote:
>>
>> On 4 October 2010 22:09, Stéphane Ducasse<[hidden email]>
>>  wrote:
>>>
>>> so let us know in the bug entry what is the conclusion :)
>>>
>> I think someone should verify my benchmarks i.e.
>>
>> [ self loadsomething ] timeToRun
>>
>> before and after patch.
>>
>> And conclusion is better be written by Henrik, because he's having
>> concerns about speed,
>> while i don't. :)
>
> I already have:
>
> http://code.google.com/p/pharo/issues/detail?id=1628#c13
>

emm.. wait.