Again I think you should push the asSet into selectors.
On Tue, Oct 20, 2009 at 5:55 PM, commits@source.squeak.org wrote:
Nicolas Cellier uploaded a new version of Traits to project The Trunk: http://source.squeak.org/trunk/Traits-nice.239.mcz
==================== Summary ====================
Name: Traits-nice.239 Author: nice Time: 21 October 2009, 2:55 am UUID: da00b4a5-33ee-4d7e-bdf5-41e2d7ff90ae Ancestors: Traits-nice.238
correct a missing #asSet after #keys refactoring
=============== Diff against Traits-nice.238 ===============
Item was changed: ----- Method: TraitBehavior>>allSelectors (in category 'accessing method dictionary') ----- allSelectors
^ self selectors asSet!
^ self selectors!
Hi Eliot, This is experimental, not guaranteed bugfree. Indeed, you cannot #add: to nor #remove: from an Array... and elements won't be unique. My trunk image did not freeze because I analyzed senders of #selectors in it. But sure, this might break foreign code... So yes, it deserves discussion.
What do others think ?
Fortunately, it is very easy to retract this change... ...just have to check if aClass selectors sort is not sent directly. I don't think it is essential and I won't fight to defend it.
Note that I also changed classVarNames as a sorted Array. I think this is a more interesting feature, because mostly used this way.
Do others agree on this one ?
Nicolas
2009/10/21 Eliot Miranda eliot.miranda@gmail.com:
Again I think you should push the asSet into selectors.
On Tue, Oct 20, 2009 at 5:55 PM, commits@source.squeak.org wrote:
Nicolas Cellier uploaded a new version of Traits to project The Trunk: http://source.squeak.org/trunk/Traits-nice.239.mcz
==================== Summary ====================
Name: Traits-nice.239 Author: nice Time: 21 October 2009, 2:55 am UUID: da00b4a5-33ee-4d7e-bdf5-41e2d7ff90ae Ancestors: Traits-nice.238
correct a missing #asSet after #keys refactoring
=============== Diff against Traits-nice.238 ===============
Item was changed: ----- Method: TraitBehavior>>allSelectors (in category 'accessing method dictionary') ----- allSelectors
- ^ self selectors asSet!
- ^ self selectors!
On 21.10.2009, at 20:11, Nicolas Cellier wrote:
Hi Eliot, This is experimental, not guaranteed bugfree. Indeed, you cannot #add: to nor #remove: from an Array... and elements won't be unique. My trunk image did not freeze because I analyzed senders of #selectors in it. But sure, this might break foreign code... So yes, it deserves discussion.
What do others think ?
I agree with Eliot. Keep the changes minimal - so just replace "keys" with "keys asSet" where necessary.
- Bert -
Fortunately, it is very easy to retract this change... ...just have to check if aClass selectors sort is not sent directly. I don't think it is essential and I won't fight to defend it.
Note that I also changed classVarNames as a sorted Array. I think this is a more interesting feature, because mostly used this way.
Do others agree on this one ?
Nicolas
2009/10/21 Eliot Miranda eliot.miranda@gmail.com:
Again I think you should push the asSet into selectors.
On Tue, Oct 20, 2009 at 5:55 PM, commits@source.squeak.org wrote:
Nicolas Cellier uploaded a new version of Traits to project The Trunk: http://source.squeak.org/trunk/Traits-nice.239.mcz
==================== Summary ====================
Name: Traits-nice.239 Author: nice Time: 21 October 2009, 2:55 am UUID: da00b4a5-33ee-4d7e-bdf5-41e2d7ff90ae Ancestors: Traits-nice.238
correct a missing #asSet after #keys refactoring
=============== Diff against Traits-nice.238 ===============
Item was changed: ----- Method: TraitBehavior>>allSelectors (in category 'accessing method dictionary') ----- allSelectors
^ self selectors asSet!
^ self selectors!
On 21-Oct-09, at 11:24 AM, Bert Freudenberg wrote:
On 21.10.2009, at 20:11, Nicolas Cellier wrote:
Hi Eliot, This is experimental, not guaranteed bugfree. Indeed, you cannot #add: to nor #remove: from an Array... and elements won't be unique. My trunk image did not freeze because I analyzed senders of #selectors in it. But sure, this might break foreign code... So yes, it deserves discussion.
What do others think ?
I agree with Eliot. Keep the changes minimal - so just replace "keys" with "keys asSet" where necessary.
- Bert
Me too. #selectors should return a Set.
Colin
2009/10/21 Colin Putney cputney@wiresong.ca:
On 21-Oct-09, at 11:24 AM, Bert Freudenberg wrote:
On 21.10.2009, at 20:11, Nicolas Cellier wrote:
Hi Eliot, This is experimental, not guaranteed bugfree. Indeed, you cannot #add: to nor #remove: from an Array... and elements won't be unique. My trunk image did not freeze because I analyzed senders of #selectors in it. But sure, this might break foreign code... So yes, it deserves discussion.
What do others think ?
I agree with Eliot. Keep the changes minimal - so just replace "keys" with "keys asSet" where necessary.
- Bert
Me too. #selectors should return a Set.
Colin
Though I found no such usage in MC1.0 :) But I didn't inquire MC2.0, DeltaStreams, Etoys nor any other reflexion crunching packages...
BTW, it would be great to have universal code analysis tools spreading over a public code database, not just my image. It dream of such database served through web pages...
Nicolas
2009/10/21 Nicolas Cellier nicolas.cellier.aka.nice@gmail.com:
2009/10/21 Colin Putney cputney@wiresong.ca:
On 21-Oct-09, at 11:24 AM, Bert Freudenberg wrote:
On 21.10.2009, at 20:11, Nicolas Cellier wrote:
Hi Eliot, This is experimental, not guaranteed bugfree. Indeed, you cannot #add: to nor #remove: from an Array... and elements won't be unique. My trunk image did not freeze because I analyzed senders of #selectors in it. But sure, this might break foreign code... So yes, it deserves discussion.
What do others think ?
I agree with Eliot. Keep the changes minimal - so just replace "keys" with "keys asSet" where necessary.
- Bert
Me too. #selectors should return a Set.
Colin
Though I found no such usage in MC1.0 :) But I didn't inquire MC2.0, DeltaStreams, Etoys nor any other reflexion crunching packages...
BTW, it would be great to have universal code analysis tools spreading over a public code database, not just my image. It dream of such database served through web pages...
Nicolas
Last thing, before retracting this change, I give hereafter a little rationale of my choice:
There were 92 senders of selectors in trunk, I classified into three categories
1) Some senders performing add: or remove: If I recall, only 3 of them were I placed some #asSet protection.
2) Some senders expecting unique keys : but hey, selectors have unique elements because methodDictionary keys are unique... Only those aggregating with other selectors care, but most of them explicitely addAll: to an empty IdentitySet...
3) Every others did use an Array compatible message like do: includes: select: collect: asSortedCollection etc...
Then I analyzed the cost of selectors:
1) Array will perform better than Set for all these selectors, but includes:. 2) Most direct senders of selector includes: SHOULD better be written includesSelector: Rationale: a single message send that will test includesKey: rather than two creating unecessarily an intermediate Set 3) 95% of senders would now uselessly perform an additional #asSet operation
So my analyze was that I would personnaly buy this little optimization with the drawback of having to rewrite 5% of senders at most. But of course, I don't offer to rewritte your code, so I understand your opinion may differ :) And of course I agree that optimized code that breaks has no value. That's just a matter of tradeoffs.
Nicolas
squeak-dev@lists.squeakfoundation.org
-
Bert Freudenberg -
Colin Putney -
Eliot Miranda -
Nicolas Cellier