DictionaryIntegrityTest >> #testDictionaries failure

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

DictionaryIntegrityTest >> #testDictionaries failure

Frank Shearar-3
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

Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Chris Muller-3
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
>

Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Levente Uzonyi-2
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
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Frank Shearar-3
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
>>>
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Fwd: [squeak-dev] DictionaryIntegrityTest >> #testDictionaries failure

Chris Muller-4
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?

Reply | Threaded
Open this post in threaded view
|

Re: Fwd: [squeak-dev] DictionaryIntegrityTest >> #testDictionaries failure

Bob Arning-2
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?





Reply | Threaded
Open this post in threaded view
|

Re: Fwd: [squeak-dev] DictionaryIntegrityTest >> #testDictionaries failure

Chris Muller-3
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?
>
>
>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Fwd: [squeak-dev] DictionaryIntegrityTest >> #testDictionaries failure

Eliot Miranda-2


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.



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


Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Levente Uzonyi-2
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
>>>>
>>>
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Fwd: [squeak-dev] DictionaryIntegrityTest >> #testDictionaries failure

Levente Uzonyi-2
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.
In Squeak only MethodDictionaries use two arrays internally. Actually they
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
>
>

Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Levente Uzonyi-2
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
>>>>>
>>>>
>>>>
>>>
>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Chris Muller-4
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.

Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Bert Freudenberg
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 -


Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Chris Muller-3
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 -
>
>

Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Stéphane Rollandin
In reply to this post by Bert Freudenberg
> How about this?
>
> dictionary at: x ifPresentPut: [:v | v + 1].
>
> - Bert -

Nice.

Stef


Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Frank Shearar-3
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
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Levente Uzonyi-2
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
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Frank Shearar-3
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
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: DictionaryIntegrityTest >> #testDictionaries failure

Levente Uzonyi-2
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
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>