Smalltalk › Squeak › Squeak - Dev

The Trunk: Collections-bf.612.mcz

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

12 messages Options

commits-2

The Trunk: Collections-bf.612.mcz

Tobias Pape uploaded a new version of Collections to project The Trunk:
http://source.squeak.org/trunk/Collections-bf.612.mcz

==================== Summary ====================

Name: Collections-bf.612
Author: bf
Time: 7 April 2015, 2:04:06.99 pm
UUID: 1b9e2fc6-03fb-41c2-a3e8-197fa24c108a
Ancestors: Collections-ul.611

Ensure that copyWith/out retains the weak-ness of a weak array.

=============== Diff against Collections-ul.611 ===============

Item was added:
+ ----- Method: WeakArray>>copyWith: (in category 'copying') -----
+ copyWith: newElement
+ "Re-implemented to not return a strong copy."
+ ^ (super copyWith: newElement) as: self class!

Item was added:
+ ----- Method: WeakArray>>copyWithout: (in category 'copying') -----
+ copyWithout: oldElement
+ "Re-implemented to not return a strong copy."
+ ^ (super copyWithout: oldElement) as: self class!

Item was changed:
+ ----- Method: WeakArray>>species (in category 'private') -----
- ----- Method: WeakArray>>species (in category 'as yet unclassified') -----
species
"More useful to have strongly-referenced results of #select: and #collect:."
^ Array!

Chris Muller-3

Re: The Trunk: Collections-bf.612.mcz

The change to override #species may not have gotten enough scrutiny.
It was a mere mention by Eliot in a prior thread, which I captured
into Collections-cmm.603. It may be worth some additional peer
scrutiny. It feels right on the surface but could such a change cause
negative impacts elsewhere?

On Tue, Apr 7, 2015 at 7:04 AM, <[hidden email]> wrote:

> Tobias Pape uploaded a new version of Collections to project The Trunk:
> http://source.squeak.org/trunk/Collections-bf.612.mcz
>
> ==================== Summary ====================
>
> Name: Collections-bf.612
> Author: bf
> Time: 7 April 2015, 2:04:06.99 pm
> UUID: 1b9e2fc6-03fb-41c2-a3e8-197fa24c108a
> Ancestors: Collections-ul.611
>
> Ensure that copyWith/out retains the weak-ness of a weak array.
>
> =============== Diff against Collections-ul.611 ===============
>
> Item was added:
> + ----- Method: WeakArray>>copyWith: (in category 'copying') -----
> + copyWith: newElement
> + "Re-implemented to not return a strong copy."
> + ^ (super copyWith: newElement) as: self class!
>
> Item was added:
> + ----- Method: WeakArray>>copyWithout: (in category 'copying') -----
> + copyWithout: oldElement
> + "Re-implemented to not return a strong copy."
> + ^ (super copyWithout: oldElement) as: self class!
>
> Item was changed:
> + ----- Method: WeakArray>>species (in category 'private') -----
> - ----- Method: WeakArray>>species (in category 'as yet unclassified') -----
> species
> "More useful to have strongly-referenced results of #select: and #collect:."
> ^ Array!
>
>

Tobias Pape

Re: The Trunk: Collections-bf.612.mcz

On 08.04.2015, at 04:45, Chris Muller <[hidden email]> wrote:

> The change to override #species may not have gotten enough scrutiny.
> It was a mere mention by Eliot in a prior thread, which I captured
> into Collections-cmm.603. It may be worth some additional peer
> scrutiny. It feels right on the surface but could such a change cause
> negative impacts elsewhere?

Always.
I sat there with Bert and we tried to debug why obsoletes never got
cleaned up and it turned out to be due to copyWith/without relying on
species which now gives a strong class.
We understood that the collection protocol might find it useful to
have a strong copy of a weak collection.

We really need to find out which part of the protocol has to
stay weak or go strong :)

Best
-Tobais

>
>
> On Tue, Apr 7, 2015 at 7:04 AM, <[hidden email]> wrote:
>> Tobias Pape uploaded a new version of Collections to project The Trunk:
>> http://source.squeak.org/trunk/Collections-bf.612.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Collections-bf.612
>> Author: bf
>> Time: 7 April 2015, 2:04:06.99 pm
>> UUID: 1b9e2fc6-03fb-41c2-a3e8-197fa24c108a
>> Ancestors: Collections-ul.611
>>
>> Ensure that copyWith/out retains the weak-ness of a weak array.
>>
>> =============== Diff against Collections-ul.611 ===============
>>
>> Item was added:
>> + ----- Method: WeakArray>>copyWith: (in category 'copying') -----
>> + copyWith: newElement
>> + "Re-implemented to not return a strong copy."
>> + ^ (super copyWith: newElement) as: self class!
>>
>> Item was added:
>> + ----- Method: WeakArray>>copyWithout: (in category 'copying') -----
>> + copyWithout: oldElement
>> + "Re-implemented to not return a strong copy."
>> + ^ (super copyWithout: oldElement) as: self class!
>>
>> Item was changed:
>> + ----- Method: WeakArray>>species (in category 'private') -----
>> - ----- Method: WeakArray>>species (in category 'as yet unclassified') -----
>> species
>> "More useful to have strongly-referenced results of #select: and #collect:."
>> ^ Array!

Bert Freudenberg

WeakArray species (was The Trunk: Collections-bf.612.mcz)

In reply to this post by Chris Muller-3

On 08.04.2015, at 04:45, Chris Muller <[hidden email]> wrote:
>
> The change to override #species may not have gotten enough scrutiny.
> It was a mere mention by Eliot in a prior thread, which I captured
> into Collections-cmm.603. It may be worth some additional peer
> scrutiny. It feels right on the surface but could such a change cause
> negative impacts elsewhere?

Oh, that was a recent change? I wondered why we never noticed the problems before. That explains it.

- Bert -

smime.p7s (5K) Download Attachment

Chris Muller-3

Re: WeakArray species (was The Trunk: Collections-bf.612.mcz)

The impetus for the change was so that (WeakArray new = Array new)
would return true instead of false. But we could accomplish that by
overriding #= instead of #species, I think..

Do you think overriding #species in WeakArray at this point in the
release process is too aggressive?

On Wed, Apr 8, 2015 at 6:16 AM, Bert Freudenberg <[hidden email]> wrote:

> On 08.04.2015, at 04:45, Chris Muller <[hidden email]> wrote:
>>
>> The change to override #species may not have gotten enough scrutiny.
>> It was a mere mention by Eliot in a prior thread, which I captured
>> into Collections-cmm.603. It may be worth some additional peer
>> scrutiny. It feels right on the surface but could such a change cause
>> negative impacts elsewhere?
>
> Oh, that was a recent change? I wondered why we never noticed the problems before. That explains it.
>
> - Bert -
>
>>
>>
>> On Tue, Apr 7, 2015 at 7:04 AM, <[hidden email]> wrote:
>>> Tobias Pape uploaded a new version of Collections to project The Trunk:
>>> http://source.squeak.org/trunk/Collections-bf.612.mcz
>>>
>>> ==================== Summary ====================
>>>
>>> Name: Collections-bf.612
>>> Author: bf
>>> Time: 7 April 2015, 2:04:06.99 pm
>>> UUID: 1b9e2fc6-03fb-41c2-a3e8-197fa24c108a
>>> Ancestors: Collections-ul.611
>>>
>>> Ensure that copyWith/out retains the weak-ness of a weak array.
>>>
>>> =============== Diff against Collections-ul.611 ===============
>>>
>>> Item was added:
>>> + ----- Method: WeakArray>>copyWith: (in category 'copying') -----
>>> + copyWith: newElement
>>> + "Re-implemented to not return a strong copy."
>>> + ^ (super copyWith: newElement) as: self class!
>>>
>>> Item was added:
>>> + ----- Method: WeakArray>>copyWithout: (in category 'copying') -----
>>> + copyWithout: oldElement
>>> + "Re-implemented to not return a strong copy."
>>> + ^ (super copyWithout: oldElement) as: self class!
>>>
>>> Item was changed:
>>> + ----- Method: WeakArray>>species (in category 'private') -----
>>> - ----- Method: WeakArray>>species (in category 'as yet unclassified') -----
>>> species
>>> "More useful to have strongly-referenced results of #select: and #collect:."
>>> ^ Array!
>>>
>>>
>>
>
>
>
>
>

Bert Freudenberg

Re: WeakArray species (was The Trunk: Collections-bf.612.mcz)

On 08.04.2015, at 18:03, Chris Muller <[hidden email]> wrote:
>
> The impetus for the change was so that (WeakArray new = Array new)
> would return true instead of false. But we could accomplish that by
> overriding #= instead of #species, I think..
>
> Do you think overriding #species in WeakArray at this point in the
> release process is too aggressive?

Not sure. The #testNoObsoleteClasses test is still failing, even though there were fewer obsolete classes when we fixed WeakArray>>copyWith:. Would reverting #species help fix these for good?

- Bert -

> On Wed, Apr 8, 2015 at 6:16 AM, Bert Freudenberg <[hidden email]> wrote:
>> On 08.04.2015, at 04:45, Chris Muller <[hidden email]> wrote:
>>>
>>> The change to override #species may not have gotten enough scrutiny.
>>> It was a mere mention by Eliot in a prior thread, which I captured
>>> into Collections-cmm.603. It may be worth some additional peer
>>> scrutiny. It feels right on the surface but could such a change cause
>>> negative impacts elsewhere?
>>
>> Oh, that was a recent change? I wondered why we never noticed the problems before. That explains it.
>>
>> - Bert -
>>

smime.p7s (5K) Download Attachment

Tobias Pape

Re: WeakArray species (was The Trunk: Collections-bf.612.mcz)

Hi,

On 09.04.2015, at 12:03, Bert Freudenberg <[hidden email]> wrote:

> On 08.04.2015, at 18:03, Chris Muller <[hidden email]> wrote:
>>
>> The impetus for the change was so that (WeakArray new = Array new)
>> would return true instead of false. But we could accomplish that by
>> overriding #= instead of #species, I think..
>>
>> Do you think overriding #species in WeakArray at this point in the
>> release process is too aggressive?
>
> Not sure. The #testNoObsoleteClasses test is still failing, even though there were fewer obsolete classes when we fixed WeakArray>>copyWith:. Would reverting #species help fix these for good?

I am unsure. Is there a weak-ref-guru out there?

Best
-Tobias

signature.asc (1K) Download Attachment

Bert Freudenberg

Re: WeakArray species (was The Trunk: Collections-bf.612.mcz)

> On 09.04.2015, at 20:27, Tobias Pape <[hidden email]> wrote:
>
> Hi,
>
> On 09.04.2015, at 12:03, Bert Freudenberg <[hidden email]> wrote:
>
>> On 08.04.2015, at 18:03, Chris Muller <[hidden email]> wrote:
>>>
>>> The impetus for the change was so that (WeakArray new = Array new)
>>> would return true instead of false. But we could accomplish that by
>>> overriding #= instead of #species, I think..
>>>
>>> Do you think overriding #species in WeakArray at this point in the
>>> release process is too aggressive?
>>
>> Not sure. The #testNoObsoleteClasses test is still failing, even though there were fewer obsolete classes when we fixed WeakArray>>copyWith:. Would reverting #species help fix these for good?
>
> I am unsure. Is there a weak-ref-guru out there?

It would help to know what still holds onto the obsolete classes. Unfortunately, the test image does not get saved for a post-mortem.

Let's see if Collections-bf.614 makes a difference ...

- Bert -

smime.p7s (5K) Download Attachment

Bert Freudenberg

Re: WeakArray species (was The Trunk: Collections-bf.612.mcz)

> On 10.04.2015, at 16:38, Bert Freudenberg <[hidden email]> wrote:
>
>
>> On 09.04.2015, at 20:27, Tobias Pape <[hidden email]> wrote:
>>
>> Hi,
>>
>> On 09.04.2015, at 12:03, Bert Freudenberg <[hidden email]> wrote:
>>
>>> On 08.04.2015, at 18:03, Chris Muller <[hidden email]> wrote:
>>>>
>>>> The impetus for the change was so that (WeakArray new = Array new)
>>>> would return true instead of false. But we could accomplish that by
>>>> overriding #= instead of #species, I think..
>>>>
>>>> Do you think overriding #species in WeakArray at this point in the
>>>> release process is too aggressive?
>>>
>>> Not sure. The #testNoObsoleteClasses test is still failing, even though there were fewer obsolete classes when we fixed WeakArray>>copyWith:. Would reverting #species help fix these for good?
>>
>> I am unsure. Is there a weak-ref-guru out there?
>
> It would help to know what still holds onto the obsolete classes. Unfortunately, the test image does not get saved for a post-mortem.
>
> Let's see if Collections-bf.614 makes a difference ...

Nope, no difference:

====================================================================================
http://build.squeak.org/job/SqueakTrunk/1250/console
2015-04-10T16:54:29.378+01:00: Collections (bf.614)

Failed Tests.Release.ReleaseTest.testNoObsoleteClasses (from Tests)

Failing for the past 620 builds (Since #631 )
Took 92 ms.
Error Message

AnObsoleteMCMockClassA, AnObsoleteMCMockASubclass, AnObsoleteMCMockClassB, AnObsoleteMCMockClassD, AnObsoleteMCMockClassE, AnObsoleteMCMockClassF, AnObsoleteMCMockClassG, AnObsoleteMCMockClassH, AnObsoleteMCMockClassI, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild and AnObsoleteClassVarScopeGrandchild are obsolete

Stacktrace

ReleaseTest(TestCase)>>assert:description:
ReleaseTest>>testNoObsoleteClasses
ReleaseTest(TestCase)>>performTest
====================================================================================

- Bert -

smime.p7s (5K) Download Attachment

Nicolas Cellier

Re: WeakArray species (was The Trunk: Collections-bf.612.mcz)

This sounds like scories left by another TestCase...
So natural questions arising are
- doesn't it depend on the ordering of test cases?
- How are TestCase ordered exactly?

2015-04-10 17:03 GMT+02:00 Bert Freudenberg <[hidden email]>:

> On 10.04.2015, at 16:38, Bert Freudenberg <[hidden email]> wrote:
>
>
>> On 09.04.2015, at 20:27, Tobias Pape <[hidden email]> wrote:
>>
>> Hi,
>>
>> On 09.04.2015, at 12:03, Bert Freudenberg <[hidden email]> wrote:
>>
>>> On 08.04.2015, at 18:03, Chris Muller <[hidden email]> wrote:
>>>>
>>>> The impetus for the change was so that (WeakArray new = Array new)
>>>> would return true instead of false. But we could accomplish that by
>>>> overriding #= instead of #species, I think..
>>>>
>>>> Do you think overriding #species in WeakArray at this point in the
>>>> release process is too aggressive?
>>>
>>> Not sure. The #testNoObsoleteClasses test is still failing, even though there were fewer obsolete classes when we fixed WeakArray>>copyWith:. Would reverting #species help fix these for good?
>>
>> I am unsure. Is there a weak-ref-guru out there?
>
> It would help to know what still holds onto the obsolete classes. Unfortunately, the test image does not get saved for a post-mortem.
>
> Let's see if Collections-bf.614 makes a difference ...

Nope, no difference:

====================================================================================
http://build.squeak.org/job/SqueakTrunk/1250/console
2015-04-10T16:54:29.378+01:00: Collections (bf.614)

Failed Tests.Release.ReleaseTest.testNoObsoleteClasses (from Tests)

Failing for the past 620 builds (Since #631 )
Took 92 ms.
Error Message

AnObsoleteMCMockClassA, AnObsoleteMCMockASubclass, AnObsoleteMCMockClassB, AnObsoleteMCMockClassD, AnObsoleteMCMockClassE, AnObsoleteMCMockClassF, AnObsoleteMCMockClassG, AnObsoleteMCMockClassH, AnObsoleteMCMockClassI, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild, AnObsoleteClassVarScopeGrandchild, AnObsoleteClassVarScopeParent, AnObsoleteClassVarScopeChild and AnObsoleteClassVarScopeGrandchild are obsolete

Stacktrace

ReleaseTest(TestCase)>>assert:description:
ReleaseTest>>testNoObsoleteClasses
ReleaseTest(TestCase)>>performTest
====================================================================================

- Bert -

Bert Freudenberg

Re: WeakArray species (was The Trunk: Collections-bf.612.mcz)

On 10.04.2015, at 17:21, Nicolas Cellier <[hidden email]> wrote:

This sounds like scories left by another TestCase…

Yep.

So natural questions arising are
- doesn't it depend on the ordering of test cases?

It should.

- How are TestCase ordered exactly?

I though TestRunner randomized the execution order.

Good News is that I can reproduce (part of) the problem simply by running all tests even on my Mac. After that I inspect "SystemNavigation default obsoleteClasses” and get some. Here’s what PointerFinder tells me holds onto them:

globals: Environment

declarations: IdentityDictionary

#World -> PasteUpMorph

submorphs: Array

7: PluggableSystemWindow (1734)

model: TestRunner

result: TestResult

passed: OrderedCollection

array: Array

550: ClassVarScopeTest

parent: AnObsoleteClassVarScopeParent class

Apparently, that test modifies its own parent class?!

I did not see the obsolete MCMock* classes on the Mac, though.

- Bert -

smime.p7s (5K) Download Attachment

Levente Uzonyi-2

Re: WeakArray species (was The Trunk: Collections-bf.612.mcz)

On Fri, 10 Apr 2015, Bert Freudenberg wrote:

>
> On 10.04.2015, at 17:21, Nicolas Cellier <[hidden email]> wrote:
>
> This sounds like scories left by another TestCase…
>
>
> Yep.
>
> So natural questions arising are
> - doesn't it depend on the ordering of test cases?
>
>
> It should.
>
> - How are TestCase ordered exactly?
>
>
> I though TestRunner randomized the execution order.
> Good News is that I can reproduce (part of) the problem simply by running all tests even on my Mac. After that I inspect "SystemNavigation default obsoleteClasses” and get some. Here’s what PointerFinder tells me holds onto them:

IIRC the tests are run one by one on the build server, not from the
TestRunner.

>
>
> globals: Environment
> declarations: IdentityDictionary
> #World -> PasteUpMorph
> submorphs: Array
> 7: PluggableSystemWindow (1734)
> model: TestRunner
> result: TestResult
> passed: OrderedCollection
> array: Array
> 550: ClassVarScopeTest
> parent: AnObsoleteClassVarScopeParent class
>
> Apparently, that test modifies its own parent class?!

I doubt it. It creates three classes. But that problem should be gone by
now. The TestCase instance held references to the created classes.

Levente

>
> I did not see the obsolete MCMock* classes on the Mac, though.
>
> - Bert -
>
>
>