<br><br><div class="gmail_quote">On Thu, Sep 20, 2012 at 1:45 PM, Levente Uzonyi <span dir="ltr"><<a href="mailto:leves@elte.hu" target="_blank">leves@elte.hu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Thu, 20 Sep 2012, Colin Putney wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Thu, Sep 20, 2012 at 11:28 AM, Levente Uzonyi <<a href="mailto:leves@elte.hu" target="_blank">leves@elte.hu</a>> wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
In order to fix this bug, I came up with two ideas:<br>
1) Recompile all methods which point to the old class when the class is<br>
removed. This helps, because the class will be undeclared at that point, so<br>
a new binding will be stored in Undeclared and the same binding will be used<br>
when the class is added again. The drawback of this solution is that it only<br>
fixes references in methods and it's slow, since it has to scan all methods<br>
in the system.<br>
</blockquote>
<br>
Couldn't we just move the existing binding to Undeclared? That might<br>
be unnecessary if there are no references, but I don't think it would<br>
hurt.<br>
</blockquote>
<br></div>
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.<br>
<br>
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.</blockquote><div><br></div><div>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.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2) Create a new object (a datastructure) which points to the binding weakly<br>
and provides fast lookup by class name. Store the binding in this object<br>
when the class is removed and when a class is added, then check this object<br>
for existing bindings (besides Undeclared). This solution is fast, but more<br>
complicated than the previous one. But the same kind of<br>
datastructure can be used to replace Undeclared, so it will not hold onto<br>
bindings unnecessarily and it won't have to be cleared manually.<br>
<br>
I uploaded a few days ago System-ul.497.mcz to the Inbox which provides<br>
solution #1. There are ways to improve the performance of SystemNavigation<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
#allCallsOn: by ~20%, so it will take only 20-40ms more to remove a<br>
</blockquote></blockquote>
<br>
class from the system, which might be good enough for now.<br>
<br>
In 4.5 I'd like to implement solution #2 and replace Undeclared to work this<br>
way too, but I think for 4.4 we can live with solution #1. What do you<br>
think?<br>
</blockquote>
<br>
I'm not sure I follow solution #2, but we'll be addressing this stuff<br>
</blockquote>
<br></div>
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.<div class="im">
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
anyway in 4.5 with Environments. We can certainly make sure this<br>
doesn't crop up again.<br>
</blockquote>
<br></div>
I guess there will be a per environment "Undeclared", but it shouldn't work like the current one (see above why).<br>
<br>
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.<br>
<br>
<br>
Levente<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Colin<br>
<br>
<br>
</blockquote>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br>best,<div>Eliot</div><br>