On Thu, Sep 20, 2012 at 1:45 PM, Levente Uzonyi <leves at elte.hu> wrote:

> On Thu, 20 Sep 2012, Colin Putney wrote:
>
>  On Thu, Sep 20, 2012 at 11:28 AM, Levente Uzonyi <leves at elte.hu> wrote:
>>
>>  In order to fix this bug, I came up with two ideas:
>>> 1) Recompile all methods which point to the old class when the class is
>>> removed. This helps, because the class will be undeclared at that point,
>>> so
>>> a new binding will be stored in Undeclared and the same binding will be
>>> used
>>> when the class is added again. The drawback of this solution is that it
>>> only
>>> fixes references in methods and it's slow, since it has to scan all
>>> methods
>>> in the system.
>>>
>>
>> Couldn't we just move the existing binding to Undeclared? That might
>> be unnecessary if there are no references, but I don't think it would
>> hurt.
>>
>
> I tried that, but ran into another issue related to Undeclared. Class >>
> #rename: checks if the new class name is included in Undeclared. And if it
> is, it'll #inform: about it. The problem is that Undeclared holds a strong
> reference to its bindings, so if we remove a class which has no references
> and we move it to Undeclared, then it will linger there, until someone
> calls #cleanOutUndeclared. This is the weakness I described in solution #2.
>
> To work around this we can remove the unreferenced keys from Undeclared in
> Class >> #rename:, but it means, that all methods in the system will be
> scanned for each renamed class.


That's a price well worth paying. Traversing the class hierarchy to fid
methods is quite quick (much faster than allInstances) and class rename is
rare (i.e. a devlopment time activity, not a typical runtime activity).  So
IMO this is the way to go.


>
>
>>  2) Create a new object (a datastructure) which points to the binding
>>> weakly
>>> and provides fast lookup by class name. Store the binding in this object
>>> when the class is removed and when a class is added, then check this
>>> object
>>> for existing bindings (besides Undeclared). This solution is fast, but
>>> more
>>> complicated than the previous one. But the same kind of
>>> datastructure can be used to replace Undeclared, so it will not hold onto
>>> bindings unnecessarily and it won't have to be cleared manually.
>>>
>>> I uploaded a few days ago System-ul.497.mcz to the Inbox which provides
>>> solution #1. There are ways to improve the performance of
>>> SystemNavigation
>>>
>>>>
>>>>> #allCallsOn: by ~20%, so it will take only 20-40ms more to remove a
>>>>>
>>>>
>>> class from the system, which might be good enough for now.
>>>
>>> In 4.5 I'd like to implement solution #2 and replace Undeclared to work
>>> this
>>> way too, but I think for 4.4 we can live with solution #1. What do you
>>> think?
>>>
>>
>> I'm not sure I follow solution #2, but we'll be addressing this stuff
>>
>
> The idea is to store className -> binding pairs in a modified
> WeakValueDictionary which cleans itself automatically as its values get
> garbage collected or in a new kind of weak dictionary which has weak
> references to both its keys and values.
>
>
>  anyway in 4.5 with Environments. We can certainly make sure this
>> doesn't crop up again.
>>
>
> I guess there will be a per environment "Undeclared", but it shouldn't
> work like the current one (see above why).
>
> I also saw, that Environment uses an IdentityDictionary to hold the
> classes instead of a SystemDictionary. It means that it doesn't use
> Undeclared at all and it doesn't use the hash advantage implemented in
> SystemDictionary, which uses #== and #hash (instead of #identityHash).
> Using #hash works, because Symbols are unique. Since calculating the hash
> is supported by a primitive, therefore it's pretty fast and the use of
> #hash itself reduces the chance for collisions.
>
>
> Levente
>
>
>> Colin
>>
>>
>>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20120920/69c70010/attachment.htm