This test fails because, as far as I can tell, SystemChangeFileTest >>
#testClassRenamed breaks the integrity of an IdentityDictionary. This IdentityDictionary (call it dict) reports having a key in #keysAndValuesDo: such that dict at: key raises a KeyNotFound. Inspecting dict verifies this weirdness, even though you can see the key in the inspector's right hand pane. Ideas? The diff in the manifests between the last version where this test didn't fail and where it did looks like this: $ diff /home/frank/Downloads/TrunkImage.313.manifest /home/frank/Downloads/TrunkImage.314.manifest 22c22 < Monticello (bf.541) --- > Monticello (bf.542) 26c26 < MorphicTests (ar.18) --- > MorphicTests (fbs.20) 49c49 < Tests (fbs.207) --- > * Tests (fbs.212) 62,63d61 Not terribly enlightening... frank |
Usually this sort of problem is due to changing the hash key of an
object in the dictionary -- but you said an IdentityDictionary so.. unless there was a become: of some kind (with copyHash: false) then it may be something else.. On Sun, May 12, 2013 at 6:30 AM, Frank Shearar <[hidden email]> wrote: > This test fails because, as far as I can tell, SystemChangeFileTest >> > #testClassRenamed breaks the integrity of an IdentityDictionary. > > This IdentityDictionary (call it dict) reports having a key in > #keysAndValuesDo: such that dict at: key raises a KeyNotFound. > > Inspecting dict verifies this weirdness, even though you can see the > key in the inspector's right hand pane. > > Ideas? > > The diff in the manifests between the last version where this test > didn't fail and where it did looks like this: > > $ diff /home/frank/Downloads/TrunkImage.313.manifest > /home/frank/Downloads/TrunkImage.314.manifest > 22c22 > < Monticello (bf.541) > --- >> Monticello (bf.542) > 26c26 > < MorphicTests (ar.18) > --- >> MorphicTests (fbs.20) > 49c49 > < Tests (fbs.207) > --- >> * Tests (fbs.212) > 62,63d61 > > Not terribly enlightening... > > frank > |
On Mon, 13 May 2013, Chris Muller wrote:
> Usually this sort of problem is due to changing the hash key of an > object in the dictionary -- but you said an IdentityDictionary so.. > unless there was a become: of some kind (with copyHash: false) then it > may be something else.. It's enough to change the key of the association to a different object. Levente > > On Sun, May 12, 2013 at 6:30 AM, Frank Shearar <[hidden email]> wrote: >> This test fails because, as far as I can tell, SystemChangeFileTest >> >> #testClassRenamed breaks the integrity of an IdentityDictionary. >> >> This IdentityDictionary (call it dict) reports having a key in >> #keysAndValuesDo: such that dict at: key raises a KeyNotFound. >> >> Inspecting dict verifies this weirdness, even though you can see the >> key in the inspector's right hand pane. >> >> Ideas? >> >> The diff in the manifests between the last version where this test >> didn't fail and where it did looks like this: >> >> $ diff /home/frank/Downloads/TrunkImage.313.manifest >> /home/frank/Downloads/TrunkImage.314.manifest >> 22c22 >> < Monticello (bf.541) >> --- >>> Monticello (bf.542) >> 26c26 >> < MorphicTests (ar.18) >> --- >>> MorphicTests (fbs.20) >> 49c49 >> < Tests (fbs.207) >> --- >>> * Tests (fbs.212) >> 62,63d61 >> >> Not terribly enlightening... >> >> frank >> > > |
On 14 May 2013 12:28, Levente Uzonyi <[hidden email]> wrote:
> On Mon, 13 May 2013, Chris Muller wrote: > >> Usually this sort of problem is due to changing the hash key of an >> object in the dictionary -- but you said an IdentityDictionary so.. >> unless there was a become: of some kind (with copyHash: false) then it >> may be something else.. > > > It's enough to change the key of the association to a different object. Which explains why it's something called #testClassRenamed causing the problem. frank > Levente > > >> >> On Sun, May 12, 2013 at 6:30 AM, Frank Shearar <[hidden email]> >> wrote: >>> >>> This test fails because, as far as I can tell, SystemChangeFileTest >> >>> #testClassRenamed breaks the integrity of an IdentityDictionary. >>> >>> This IdentityDictionary (call it dict) reports having a key in >>> #keysAndValuesDo: such that dict at: key raises a KeyNotFound. >>> >>> Inspecting dict verifies this weirdness, even though you can see the >>> key in the inspector's right hand pane. >>> >>> Ideas? >>> >>> The diff in the manifests between the last version where this test >>> didn't fail and where it did looks like this: >>> >>> $ diff /home/frank/Downloads/TrunkImage.313.manifest >>> /home/frank/Downloads/TrunkImage.314.manifest >>> 22c22 >>> < Monticello (bf.541) >>> --- >>>> >>>> Monticello (bf.542) >>> >>> 26c26 >>> < MorphicTests (ar.18) >>> --- >>>> >>>> MorphicTests (fbs.20) >>> >>> 49c49 >>> < Tests (fbs.207) >>> --- >>>> >>>> * Tests (fbs.212) >>> >>> 62,63d61 >>> >>> Not terribly enlightening... >>> >>> frank >>> >> >> > |
In reply to this post by Levente Uzonyi-2
> It's enough to change the key of the association to a different object.
True but that would be circumventing the public API of the Dictionary would it not? Is there any path through the API where one obtains such access to the underlying Associations? |
It's not clear what you are asking.
Dictionaries have long had methods like #associationAt:. One would
want to be careful using such methods, but they do seem to be
*public*, however fuzzy that distinction might be.
Cheers, Bob On 5/14/13 11:42 AM, Chris Muller
wrote:
It's enough to change the key of the association to a different object.True but that would be circumventing the public API of the Dictionary would it not? Is there any path through the API where one obtains such access to the underlying Associations? |
My question is: Should users of Dictionary regard the internal
Associations as private, internal and, otherwise, not to be messed with? VisualAge had a class called a LookupTable which was identical to a Dictionary but by using... parallel keys/values Arrays I think, and so while it could answer an #associationAt:, it would just be a new object and not part of the Dictionary. To think one could set the key of this Association would not allow compatibility with regular Dictionary. So I personally have always regarded the internal Associations as private.. On Tue, May 14, 2013 at 11:01 AM, Bob Arning <[hidden email]> wrote: > It's not clear what you are asking. Dictionaries have long had methods like > #associationAt:. One would want to be careful using such methods, but they > do seem to be *public*, however fuzzy that distinction might be. > > Cheers, > Bob > > On 5/14/13 11:42 AM, Chris Muller wrote: > > It's enough to change the key of the association to a different object. > > True but that would be circumventing the public API of the Dictionary > would it not? Is there any path through the API where one obtains > such access to the underlying Associations? > > > > > > |
On Tue, May 14, 2013 at 10:39 AM, Chris Muller <[hidden email]> wrote: My question is: Should users of Dictionary regard the internal They clearly are not in important circumstances. One is global variables (system, pool and class), where the association is the variable. There are other circumstances (WeakDictionary) where they are private and carefully managed. MethodDictionary, IdentityDictionary etc are like VA's LookupTable; they have no associations.
best, Eliot
|
In reply to this post by Frank Shearar-3
On Tue, 14 May 2013, Frank Shearar wrote:
> On 14 May 2013 12:28, Levente Uzonyi <[hidden email]> wrote: >> On Mon, 13 May 2013, Chris Muller wrote: >> >>> Usually this sort of problem is due to changing the hash key of an >>> object in the dictionary -- but you said an IdentityDictionary so.. >>> unless there was a become: of some kind (with copyHash: false) then it >>> may be something else.. >> >> >> It's enough to change the key of the association to a different object. > > Which explains why it's something called #testClassRenamed causing the problem. Well, it caused no problems before Environments, because the association was removed from the SystemDictionary before renaming, and added again after it. With Environments it's a bit more complicated, because there are two dictionaries in an Environment: 'contents' and 'bindins'. I'm not sure what the role of these are, but #renameClass:from:to: only removes the association from 'contents'. If it's also present in 'bindings', then 'bindings' will get into an invalid state. It's also a bit suspicious, that #associationAt: and #associationAt:ifAbsent: both use 'contents' to get the association, but #associationOrUndeclaredAt: uses 'bindings'. Levente > > frank > >> Levente >> >> >>> >>> On Sun, May 12, 2013 at 6:30 AM, Frank Shearar <[hidden email]> >>> wrote: >>>> >>>> This test fails because, as far as I can tell, SystemChangeFileTest >> >>>> #testClassRenamed breaks the integrity of an IdentityDictionary. >>>> >>>> This IdentityDictionary (call it dict) reports having a key in >>>> #keysAndValuesDo: such that dict at: key raises a KeyNotFound. >>>> >>>> Inspecting dict verifies this weirdness, even though you can see the >>>> key in the inspector's right hand pane. >>>> >>>> Ideas? >>>> >>>> The diff in the manifests between the last version where this test >>>> didn't fail and where it did looks like this: >>>> >>>> $ diff /home/frank/Downloads/TrunkImage.313.manifest >>>> /home/frank/Downloads/TrunkImage.314.manifest >>>> 22c22 >>>> < Monticello (bf.541) >>>> --- >>>>> >>>>> Monticello (bf.542) >>>> >>>> 26c26 >>>> < MorphicTests (ar.18) >>>> --- >>>>> >>>>> MorphicTests (fbs.20) >>>> >>>> 49c49 >>>> < Tests (fbs.207) >>>> --- >>>>> >>>>> * Tests (fbs.212) >>>> >>>> 62,63d61 >>>> >>>> Not terribly enlightening... >>>> >>>> frank >>>> >>> >>> >> > > |
In reply to this post by Eliot Miranda-2
On Tue, 14 May 2013, Eliot Miranda wrote:
> > > On Tue, May 14, 2013 at 10:39 AM, Chris Muller <[hidden email]> wrote: > My question is: Should users of Dictionary regard the internal > Associations as private, internal and, otherwise, not to be messed > with? > > VisualAge had a class called a LookupTable which was identical to a > Dictionary but by using... parallel keys/values Arrays I think, and so > while it could answer an #associationAt:, it would just be a new > object and not part of the Dictionary. To think one could set the key > of this Association would not allow compatibility with regular > Dictionary. > > So I personally have always regarded the internal Associations as private.. > > > They clearly are not in important circumstances. One is global variables (system, pool and class), where the association is the variable. There are other circumstances (WeakDictionary) where they are > private and carefully managed. MethodDictionary, IdentityDictionary etc are like VA's LookupTable; they have no associations. only use one array, the keys are stored in the MethodDictionary object's variable slots. That's how they can have the same layout as other Dictionaries. Levente > > > > On Tue, May 14, 2013 at 11:01 AM, Bob Arning <[hidden email]> wrote: > > It's not clear what you are asking. Dictionaries have long had methods like > > #associationAt:. One would want to be careful using such methods, but they > > do seem to be *public*, however fuzzy that distinction might be. > > > > Cheers, > > Bob > > > > On 5/14/13 11:42 AM, Chris Muller wrote: > > > > It's enough to change the key of the association to a different object. > > > > True but that would be circumventing the public API of the Dictionary > > would it not? Is there any path through the API where one obtains > > such access to the underlying Associations? > > > > > > > > > > > > > > > > > -- > best,Eliot > > |
In reply to this post by Levente Uzonyi-2
On Tue, 14 May 2013, Levente Uzonyi wrote:
> On Tue, 14 May 2013, Frank Shearar wrote: > >> On 14 May 2013 12:28, Levente Uzonyi <[hidden email]> wrote: >>> On Mon, 13 May 2013, Chris Muller wrote: >>> >>>> Usually this sort of problem is due to changing the hash key of an >>>> object in the dictionary -- but you said an IdentityDictionary so.. >>>> unless there was a become: of some kind (with copyHash: false) then it >>>> may be something else.. >>> >>> >>> It's enough to change the key of the association to a different object. >> >> Which explains why it's something called #testClassRenamed causing the >> problem. > > Well, it caused no problems before Environments, because the association was > removed from the SystemDictionary before renaming, and added again after it. > > With Environments it's a bit more complicated, because there are two > dictionaries in an Environment: 'contents' and 'bindins'. I'm not sure what > the role of these are, but #renameClass:from:to: only removes the association > from 'contents'. If it's also present in 'bindings', then 'bindings' will get > into an invalid state. > > It's also a bit suspicious, that #associationAt: and #associationAt:ifAbsent: > both use 'contents' to get the association, but #associationOrUndeclaredAt: > uses 'bindings'. By chasing the pointers I found that some associations are stored in the Environment's exports variable's namespace variable. That is the IdentityDictionary which breaks during renaming, because it holds on the same association. Levente > > > Levente > >> >> frank >> >>> Levente >>> >>> >>>> >>>> On Sun, May 12, 2013 at 6:30 AM, Frank Shearar <[hidden email]> >>>> wrote: >>>>> >>>>> This test fails because, as far as I can tell, SystemChangeFileTest >> >>>>> #testClassRenamed breaks the integrity of an IdentityDictionary. >>>>> >>>>> This IdentityDictionary (call it dict) reports having a key in >>>>> #keysAndValuesDo: such that dict at: key raises a KeyNotFound. >>>>> >>>>> Inspecting dict verifies this weirdness, even though you can see the >>>>> key in the inspector's right hand pane. >>>>> >>>>> Ideas? >>>>> >>>>> The diff in the manifests between the last version where this test >>>>> didn't fail and where it did looks like this: >>>>> >>>>> $ diff /home/frank/Downloads/TrunkImage.313.manifest >>>>> /home/frank/Downloads/TrunkImage.314.manifest >>>>> 22c22 >>>>> < Monticello (bf.541) >>>>> --- >>>>>> >>>>>> Monticello (bf.542) >>>>> >>>>> 26c26 >>>>> < MorphicTests (ar.18) >>>>> --- >>>>>> >>>>>> MorphicTests (fbs.20) >>>>> >>>>> 49c49 >>>>> < Tests (fbs.207) >>>>> --- >>>>>> >>>>>> * Tests (fbs.212) >>>>> >>>>> 62,63d61 >>>>> >>>>> Not terribly enlightening... >>>>> >>>>> frank >>>>> >>>> >>>> >>> >> >> > > |
In reply to this post by Levente Uzonyi-2
> | a |
> a := dictionary associationAt: x. > a value: a value + 1 > > instead of: > > | v | > v := dictionary at: x. > dictionary at: x put: v + 1. Oh, nice.. Now I feel like I need to go through my code and see if I'm doing it the double-lookup way.. :) Thanks. |
On 2013-05-15, at 00:03, Chris Muller <[hidden email]> wrote:
>> | a | >> a := dictionary associationAt: x. >> a value: a value + 1 >> >> instead of: >> >> | v | >> v := dictionary at: x. >> dictionary at: x put: v + 1. > > Oh, nice.. Now I feel like I need to go through my code and see if > I'm doing it the double-lookup way.. :) Thanks. How about this? dictionary at: x ifPresentPut: [:v | v + 1]. - Bert - |
Yea, that would be better than dealing with the Association. In fact
that would seem to bring a nice symmetry to the API since we have #at:ifAbsent: + #at:ifPresent: pair, it stands to reason to have a counterpart for at:ifAbsentPut:. We should add it.. On Tue, May 14, 2013 at 6:57 PM, Bert Freudenberg <[hidden email]> wrote: > On 2013-05-15, at 00:03, Chris Muller <[hidden email]> wrote: > >>> | a | >>> a := dictionary associationAt: x. >>> a value: a value + 1 >>> >>> instead of: >>> >>> | v | >>> v := dictionary at: x. >>> dictionary at: x put: v + 1. >> >> Oh, nice.. Now I feel like I need to go through my code and see if >> I'm doing it the double-lookup way.. :) Thanks. > > How about this? > > dictionary at: x ifPresentPut: [:v | v + 1]. > > - Bert - > > |
In reply to this post by Bert Freudenberg
> How about this?
> > dictionary at: x ifPresentPut: [:v | v + 1]. > > - Bert - Nice. Stef |
In reply to this post by Levente Uzonyi-2
On 14 May 2013 21:41, Levente Uzonyi <[hidden email]> wrote:
> On Tue, 14 May 2013, Levente Uzonyi wrote: > >> On Tue, 14 May 2013, Frank Shearar wrote: >> >>> On 14 May 2013 12:28, Levente Uzonyi <[hidden email]> wrote: >>>> >>>> On Mon, 13 May 2013, Chris Muller wrote: >>>> >>>>> Usually this sort of problem is due to changing the hash key of an >>>>> object in the dictionary -- but you said an IdentityDictionary so.. >>>>> unless there was a become: of some kind (with copyHash: false) then it >>>>> may be something else.. >>>> >>>> >>>> >>>> It's enough to change the key of the association to a different object. >>> >>> >>> Which explains why it's something called #testClassRenamed causing the >>> problem. >> >> >> Well, it caused no problems before Environments, because the association >> was removed from the SystemDictionary before renaming, and added again after >> it. >> >> With Environments it's a bit more complicated, because there are two >> dictionaries in an Environment: 'contents' and 'bindins'. I'm not sure what >> the role of these are, but #renameClass:from:to: only removes the >> association from 'contents'. If it's also present in 'bindings', then >> 'bindings' will get into an invalid state. >> >> It's also a bit suspicious, that #associationAt: and >> #associationAt:ifAbsent: both use 'contents' to get the association, but >> #associationOrUndeclaredAt: uses 'bindings'. > > > By chasing the pointers I found that some associations are stored in the > Environment's exports variable's namespace variable. That is the > IdentityDictionary which breaks during renaming, because it holds on the > same association. The test failure is also intermittent. Does that observation fit the hypothesis? frank > Levente > > >> >> >> Levente >> >>> >>> frank >>> >>>> Levente >>>> >>>> >>>>> >>>>> On Sun, May 12, 2013 at 6:30 AM, Frank Shearar >>>>> <[hidden email]> >>>>> wrote: >>>>>> >>>>>> >>>>>> This test fails because, as far as I can tell, SystemChangeFileTest >> >>>>>> #testClassRenamed breaks the integrity of an IdentityDictionary. >>>>>> >>>>>> This IdentityDictionary (call it dict) reports having a key in >>>>>> #keysAndValuesDo: such that dict at: key raises a KeyNotFound. >>>>>> >>>>>> Inspecting dict verifies this weirdness, even though you can see the >>>>>> key in the inspector's right hand pane. >>>>>> >>>>>> Ideas? >>>>>> >>>>>> The diff in the manifests between the last version where this test >>>>>> didn't fail and where it did looks like this: >>>>>> >>>>>> $ diff /home/frank/Downloads/TrunkImage.313.manifest >>>>>> /home/frank/Downloads/TrunkImage.314.manifest >>>>>> 22c22 >>>>>> < Monticello (bf.541) >>>>>> --- >>>>>>> >>>>>>> >>>>>>> Monticello (bf.542) >>>>>> >>>>>> >>>>>> 26c26 >>>>>> < MorphicTests (ar.18) >>>>>> --- >>>>>>> >>>>>>> >>>>>>> MorphicTests (fbs.20) >>>>>> >>>>>> >>>>>> 49c49 >>>>>> < Tests (fbs.207) >>>>>> --- >>>>>>> >>>>>>> >>>>>>> * Tests (fbs.212) >>>>>> >>>>>> >>>>>> 62,63d61 >>>>>> >>>>>> Not terribly enlightening... >>>>>> >>>>>> frank >>>>>> >>>>> >>>>> >>>> >>> >>> >> >> > |
On Thu, 23 May 2013, Frank Shearar wrote:
> On 14 May 2013 21:41, Levente Uzonyi <[hidden email]> wrote: >> >> By chasing the pointers I found that some associations are stored in the >> Environment's exports variable's namespace variable. That is the >> IdentityDictionary which breaks during renaming, because it holds on the >> same association. > > The test failure is also intermittent. Does that observation fit the hypothesis? This is a bit more than a hypothesis, but yes, HashedCollections "heal" themselves when they grow, shrink, get rehashed, or when the elements change rapidly. I think the best solution to the problem is to change the way bindings are handled. Currently bindings are used as associations in various dictionaries, so changing their key has effect on other dictionaries. If the bindings were the values of the dictionary and the bindings' keys were the keys, then changing the binding would not break the dictionary itself. This alone doesn't fix the problems in Environments, where the same bindings are stored in multiple dictionaries. Here's a snippet to find a binding which is present in all dictionaries in an environment: | environment exportsNamespace importsNamespace contents bindings | environment := Smalltalk globals. exportsNamespace := (environment instVarNamed: #exports) instVarNamed: #namespace. importsNamespace := (environment instVarNamed: #imports) instVarNamed: #namespace. contents := environment instVarNamed: #contents. bindings := environment instVarNamed: #bindings. exportsNamespace associations detect: [ :each | (importsNamespace associationAt: each key ifAbsent: [ nil ]) == each and: [ (contents associationAt: each key ifAbsent: [ nil ]) == each and: [ (bindings associationAt: each key ifAbsent: [ nil ]) == each ] ] ]. In my image it returns: #MCReorganizationPreloader=>MCReorganizationPreloader Levente > > frank > >> Levente >> >> >>> >>> >>> Levente >>> >>>> >>>> frank >>>> >>>>> Levente >>>>> >>>>> >>>>>> >>>>>> On Sun, May 12, 2013 at 6:30 AM, Frank Shearar >>>>>> <[hidden email]> >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> This test fails because, as far as I can tell, SystemChangeFileTest >> >>>>>>> #testClassRenamed breaks the integrity of an IdentityDictionary. >>>>>>> >>>>>>> This IdentityDictionary (call it dict) reports having a key in >>>>>>> #keysAndValuesDo: such that dict at: key raises a KeyNotFound. >>>>>>> >>>>>>> Inspecting dict verifies this weirdness, even though you can see the >>>>>>> key in the inspector's right hand pane. >>>>>>> >>>>>>> Ideas? >>>>>>> >>>>>>> The diff in the manifests between the last version where this test >>>>>>> didn't fail and where it did looks like this: >>>>>>> >>>>>>> $ diff /home/frank/Downloads/TrunkImage.313.manifest >>>>>>> /home/frank/Downloads/TrunkImage.314.manifest >>>>>>> 22c22 >>>>>>> < Monticello (bf.541) >>>>>>> --- >>>>>>>> >>>>>>>> >>>>>>>> Monticello (bf.542) >>>>>>> >>>>>>> >>>>>>> 26c26 >>>>>>> < MorphicTests (ar.18) >>>>>>> --- >>>>>>>> >>>>>>>> >>>>>>>> MorphicTests (fbs.20) >>>>>>> >>>>>>> >>>>>>> 49c49 >>>>>>> < Tests (fbs.207) >>>>>>> --- >>>>>>>> >>>>>>>> >>>>>>>> * Tests (fbs.212) >>>>>>> >>>>>>> >>>>>>> 62,63d61 >>>>>>> >>>>>>> Not terribly enlightening... >>>>>>> >>>>>>> frank >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>>> >>> >>> >> > > |
On 24 May 2013 15:20, Levente Uzonyi <[hidden email]> wrote:
> On Thu, 23 May 2013, Frank Shearar wrote: > >> On 14 May 2013 21:41, Levente Uzonyi <[hidden email]> wrote: >>> >>> >>> By chasing the pointers I found that some associations are stored in the >>> Environment's exports variable's namespace variable. That is the >>> IdentityDictionary which breaks during renaming, because it holds on the >>> same association. >> >> >> The test failure is also intermittent. Does that observation fit the >> hypothesis? > > > This is a bit more than a hypothesis, but yes, HashedCollections "heal" > themselves when they grow, shrink, get rehashed, or when the elements change > rapidly. > > I think the best solution to the problem is to change the way bindings are > handled. Currently bindings are used as associations in various > dictionaries, so changing their key has effect on other dictionaries. If the > bindings were the values of the dictionary and the bindings' keys were the > keys, then changing the binding would not break the dictionary itself. > > This alone doesn't fix the problems in Environments, where the same bindings > are stored in multiple dictionaries. Here's a snippet to find a binding > which is present in all dictionaries in an environment: > > | environment exportsNamespace importsNamespace contents bindings | > environment := Smalltalk globals. > exportsNamespace := (environment instVarNamed: #exports) instVarNamed: > #namespace. > importsNamespace := (environment instVarNamed: #imports) instVarNamed: > #namespace. > contents := environment instVarNamed: #contents. > bindings := environment instVarNamed: #bindings. > exportsNamespace associations detect: [ :each | > (importsNamespace associationAt: each key ifAbsent: [ nil ]) == each > and: [ (contents associationAt: each key ifAbsent: [ nil ]) > == each > and: [ (bindings associationAt: each key ifAbsent: [ nil ]) > == each ] ] ]. > > In my image it returns: > #MCReorganizationPreloader=>MCReorganizationPreloader Right. So in Environment >> #renameClass:from:to: we see this: <snip> oldref := self associationAt: oldName. contents removeKey: oldName. oldRef key: newName contents add: oldref <snip> and this might well be the cause of our grief? It should instead be something like cls := contents at: oldName. contents at:newName put: cls. contents removeKey: oldName. then? frank > Levente > > >> >> frank >> >>> Levente >>> >>> >>>> >>>> >>>> Levente >>>> >>>>> >>>>> frank >>>>> >>>>>> Levente >>>>>> >>>>>> >>>>>>> >>>>>>> On Sun, May 12, 2013 at 6:30 AM, Frank Shearar >>>>>>> <[hidden email]> >>>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> This test fails because, as far as I can tell, SystemChangeFileTest >>>>>>>> >> >>>>>>>> #testClassRenamed breaks the integrity of an IdentityDictionary. >>>>>>>> >>>>>>>> This IdentityDictionary (call it dict) reports having a key in >>>>>>>> #keysAndValuesDo: such that dict at: key raises a KeyNotFound. >>>>>>>> >>>>>>>> Inspecting dict verifies this weirdness, even though you can see the >>>>>>>> key in the inspector's right hand pane. >>>>>>>> >>>>>>>> Ideas? >>>>>>>> >>>>>>>> The diff in the manifests between the last version where this test >>>>>>>> didn't fail and where it did looks like this: >>>>>>>> >>>>>>>> $ diff /home/frank/Downloads/TrunkImage.313.manifest >>>>>>>> /home/frank/Downloads/TrunkImage.314.manifest >>>>>>>> 22c22 >>>>>>>> < Monticello (bf.541) >>>>>>>> --- >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Monticello (bf.542) >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> 26c26 >>>>>>>> < MorphicTests (ar.18) >>>>>>>> --- >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> MorphicTests (fbs.20) >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> 49c49 >>>>>>>> < Tests (fbs.207) >>>>>>>> --- >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> * Tests (fbs.212) >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> 62,63d61 >>>>>>>> >>>>>>>> Not terribly enlightening... >>>>>>>> >>>>>>>> frank >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>> >>>> >>> >> >> > |
On Fri, 24 May 2013, Frank Shearar wrote:
> On 24 May 2013 15:20, Levente Uzonyi <[hidden email]> wrote: >> On Thu, 23 May 2013, Frank Shearar wrote: >> >>> On 14 May 2013 21:41, Levente Uzonyi <[hidden email]> wrote: >>>> >>>> >>>> By chasing the pointers I found that some associations are stored in the >>>> Environment's exports variable's namespace variable. That is the >>>> IdentityDictionary which breaks during renaming, because it holds on the >>>> same association. >>> >>> >>> The test failure is also intermittent. Does that observation fit the >>> hypothesis? >> >> >> This is a bit more than a hypothesis, but yes, HashedCollections "heal" >> themselves when they grow, shrink, get rehashed, or when the elements change >> rapidly. >> >> I think the best solution to the problem is to change the way bindings are >> handled. Currently bindings are used as associations in various >> dictionaries, so changing their key has effect on other dictionaries. If the >> bindings were the values of the dictionary and the bindings' keys were the >> keys, then changing the binding would not break the dictionary itself. >> >> This alone doesn't fix the problems in Environments, where the same bindings >> are stored in multiple dictionaries. Here's a snippet to find a binding >> which is present in all dictionaries in an environment: >> >> | environment exportsNamespace importsNamespace contents bindings | >> environment := Smalltalk globals. >> exportsNamespace := (environment instVarNamed: #exports) instVarNamed: >> #namespace. >> importsNamespace := (environment instVarNamed: #imports) instVarNamed: >> #namespace. >> contents := environment instVarNamed: #contents. >> bindings := environment instVarNamed: #bindings. >> exportsNamespace associations detect: [ :each | >> (importsNamespace associationAt: each key ifAbsent: [ nil ]) == each >> and: [ (contents associationAt: each key ifAbsent: [ nil ]) >> == each >> and: [ (bindings associationAt: each key ifAbsent: [ nil ]) >> == each ] ] ]. >> >> In my image it returns: >> #MCReorganizationPreloader=>MCReorganizationPreloader > > Right. So in Environment >> #renameClass:from:to: we see this: > > <snip> > oldref := self associationAt: oldName. > contents removeKey: oldName. > oldRef key: newName > contents add: oldref > <snip> > > and this might well be the cause of our grief? It should instead be > something like > > cls := contents at: oldName. > contents at:newName put: cls. > contents removeKey: oldName. > > then? It's not clear for me that in which case would you use this code, but it doesn't seem right in any of them. If we keep the binding handling as it is now (the bindings are associations used by the dictionaries), then this is wrong, because it creates new bindings and doesn't update the old ones. If we change the binding handling, so that the dictionaries contain name->binding associations, where binding is a name->class association, then this code is incomplete, because it doesn't change the binding's key. In both cases the other 3 dictionaries have to be checked and updated if necessary: bindings, namespace in imports, and namespace in exports. Btw, it's still not clear to me why is there a bindings and a contents dictionary, and how can a class be in all 4 dictionaries at the same time. Levente > > frank > >> Levente >> >> >>> >>> frank >>> >>>> Levente >>>> >>>> >>>>> >>>>> >>>>> Levente >>>>> >>>>>> >>>>>> frank >>>>>> >>>>>>> Levente >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> On Sun, May 12, 2013 at 6:30 AM, Frank Shearar >>>>>>>> <[hidden email]> >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> This test fails because, as far as I can tell, SystemChangeFileTest >>>>>>>>>>> >>>>>>>>> #testClassRenamed breaks the integrity of an IdentityDictionary. >>>>>>>>> >>>>>>>>> This IdentityDictionary (call it dict) reports having a key in >>>>>>>>> #keysAndValuesDo: such that dict at: key raises a KeyNotFound. >>>>>>>>> >>>>>>>>> Inspecting dict verifies this weirdness, even though you can see the >>>>>>>>> key in the inspector's right hand pane. >>>>>>>>> >>>>>>>>> Ideas? >>>>>>>>> >>>>>>>>> The diff in the manifests between the last version where this test >>>>>>>>> didn't fail and where it did looks like this: >>>>>>>>> >>>>>>>>> $ diff /home/frank/Downloads/TrunkImage.313.manifest >>>>>>>>> /home/frank/Downloads/TrunkImage.314.manifest >>>>>>>>> 22c22 >>>>>>>>> < Monticello (bf.541) >>>>>>>>> --- >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Monticello (bf.542) >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> 26c26 >>>>>>>>> < MorphicTests (ar.18) >>>>>>>>> --- >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> MorphicTests (fbs.20) >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> 49c49 >>>>>>>>> < Tests (fbs.207) >>>>>>>>> --- >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> * Tests (fbs.212) >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> 62,63d61 >>>>>>>>> >>>>>>>>> Not terribly enlightening... >>>>>>>>> >>>>>>>>> frank >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>> >>> >>> >> > > |
Free forum by Nabble | Edit this page |