- Pharo-dev - lists.pharo.org

Re: [Pharo-project] FasterSets improvements

by Igor Stasenko

2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: > Unlike arrayed collections, hashed collections use a sentinel value for > implementation purposes. Â Such collections shouldn't be allowed to > contain the sentinel because doing so introduces unnecessary complexity > for the vast majority of practical applications. Â It's not worth it. Â If > adding nil to a set is important, then using self as the sentinel could > do (at the cost of initializing the set to contain self at all slots > when the instance is created, thus duplicating the nilling the VM does > for you). Â But then, the set shouldn't contain itself. > I think you are mixing an object's interface with its inner implementation details. A Set can include any object in itself, without any discrimination. But its implemented in such way that it's impossible to include nil in it. So, its not worth proving that due to limitation, which comes from implementation detail, its not worth to have Sets which can include nils. > Now, this is just my opinion... :). > > Igor Stasenko wrote: >> 2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: >> >>> I think it's just a bad idea... enumeration of the set would become >>> unnecessarily complicated, and a set that includes itself is asking for >>> trouble. Â For example, how do you hash the set? Â How do you determine >>> whether a set that includes itself is equal to itself? Â How do these >>> print themselves? Â In Smalltalk everything is an object, and >>> consequently, some object must represent the absence of an object in a >>> storage slot. Â That object cannot, at the same time, represent the >>> presence of something. >>> >>> >> well, you can try and see what will happen if you try this: >> >> set := Set new. >> set add: set. >> set printString. >> >> or try using Array instead of set. >> Or try using any other object which prints its slots, and put in one >> of its slots itself. >> What makes a Set an exceptional in this regard? >> >> >> >>> Igor Stasenko wrote: >>> >>>> 2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: >>>> >>>> >>>>> Keep in mind that, as they're implemented, there will be always some >>>>> object that cannot be added to a Set because the set uses the object as >>>>> a sentinel value. Â On the other hand, since it does not make a whole lot >>>>> of sense to add a set to itself, maybe it should use self as the >>>>> sentinel value. >>>>> >>>>> >>>>> >>>> There is no difference what object to use as a sentinel - self or nil. >>>> In fact you can use any object which fits your needs. >>>> AND be able to include it as an element of set. >>>> >>>> Just to illustrate: >>>> >>>> Set>>add: anObject >>>> Â sentinel == anObject ifTrue: [ >>>> Â Â includesSentinel := true. >>>> Â ]. >>>> .... rest of code ... >>>> >>>> Set>>includes: anObject >>>> Â anObject == sentinel Â ifTrue: [ ^ indludesSentinel ]. >>>> .... rest of code .. >>>> >>>> >>>> >>>> >>>>> Igor Stasenko wrote: >>>>> >>>>> >>>>>> Anyone considered fixing Sets to allow a nil as an element? >>>>>> >>>>>> 2009/7/3 Ralph Boland <rpboland(a)gmail.com>: >>>>>> >>>>>> >>>>>> >>>>>>> 2009/7/1 Henrik Johansen <henrik.s.johansen(a)veloxit.no>: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> I took some time to check this yesterday, my comments on the code itself are >>>>>>>> basically the same as what Lucas posted in the issue tracker. >>>>>>>> Works as advertised, but should probably clean out the "gunk" which is >>>>>>>> unused after it's integrated. >>>>>>>> >>>>>>>> Some feedback on possible improvements: >>>>>>>> - What it does, is basically speed up the adds to the new array in a grow >>>>>>>> that might happen Why not take this all to the logical conclusion, and >>>>>>>> instead optimize an addAll: method to be used in grow, that assumes elements >>>>>>>> are not in set (and themselves are not duplicates)? F.ex. it could also add >>>>>>>> to tally just once, instead of one increase per element as the noCompareAdd: >>>>>>>> does. (an add method with no tally add, no = object check, and no >>>>>>>> growCheck). >>>>>>>> >>>>>>>> >>>>>>>> - The usual addAll: method can be sped up in a similar way, by performing >>>>>>>> one grow operation before adding elements if necessary (pessimistically >>>>>>>> assuming all elements are unique), then use an add method which does not >>>>>>>> check for grow need at all. (an add method with tally add and = object >>>>>>>> check, but no growCheck) >>>>>>>> Â If desirable, you could also shrink the capacity after adds are done, to >>>>>>>> match the real amount of new elements. >>>>>>>> >>>>>>>> Adding (1 to: 1200000) to a new Set with such an addAll: method took 1/2 to >>>>>>>> Â 1/3rd the time of the regular addAll: method, even with the lower grow cost >>>>>>>> of FasterSets. (Lower gains if Set is presized, and if hash-calculation is >>>>>>>> bigger part of cost) >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> I considered this when I created FasterSets. Â I didn't do this because I was >>>>>>> being conservative. Â I didn't want FasterSets rejected because it did things >>>>>>> that weren't wanted. Â I am surprised Â though at the amount of performance >>>>>>> gain you see. >>>>>>> As things stand I see no reason for not including >>>>>>> FasterSets other than the potential of breaking packages that subclass >>>>>>> Set and then override methods crucial to FasterSets. >>>>>>> My vote is obviously for fixing any packages that get broken rather than >>>>>>> rejecting FasterSets. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>>> Note: If you want to stay ANSI-compatible, one tricky point in such an >>>>>>>> optimization is adding all objects in collection up to a nil element, then >>>>>>>> raising an error. (ie: must produce equivalent results to add: performed of >>>>>>>> each of collections elements in sequence ) >>>>>>>> >>>>>>>> Cheers, >>>>>>>> Henry >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> I created FasterSets for adding to Squeak and did not consider Pharo but >>>>>>> of course am delighted to have it considered for Pharo. >>>>>>> You can of course make whatever improvements you want. >>>>>>> I encourage though that you create a FasterSets2 containing all the >>>>>>> improvements and leave the original FasterSets alone. Â I am still interested >>>>>>> in having FasterSets included in Squeak and I do not know if the powers that >>>>>>> be will want the additional improvements or not so I prefer that both versions >>>>>>> exist. >>>>>>> >>>>>>> One thing that I have in my code is a method called nastyAddNew: Â which >>>>>>> adds an element to a set without checking if the element is already there. >>>>>>> If the element is already there of course it gets added again and your set >>>>>>> contains two copies of the element; thus your set is corrupted. This problem >>>>>>> explains the 'nasty' part of the name nastyAddNew. Â I also have a method >>>>>>> addNew: Â which reports an error if the object is already in the set. >>>>>>> nastyAddNew: can be used in set operations such as copy and intersection >>>>>>> making these operations faster and can also be used externally by brave users. >>>>>>> >>>>>>> If improvements to FasterSets are being contemplated then I suggest that >>>>>>> adding methods Â addNew: and Â nastyAddNew: Â be considered. Â If Â nastyAddNew: >>>>>>> is added I see no point in making it private because some users will >>>>>>> use it anyway. >>>>>>> I am not necessarily voting for their inclusion because we are then >>>>>>> giving the user the opportunity to corrupt his data. >>>>>>> Of course I do use them in my current project. >>>>>>> thus, for my current project at least, I am a brave user. >>>>>>> >>>>>>> Regards, >>>>>>> >>>>>>> Ralph Boland >>>>>>> >>>>>>> _______________________________________________ >>>>>>> Pharo-project mailing list >>>>>>> Pharo-project(a)lists.gforge.inria.fr >>>>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> -- >>>>>> Best regards, >>>>>> Igor Stasenko AKA sig. >>>>>> >>>>>> _______________________________________________ >>>>>> Pharo-project mailing list >>>>>> Pharo-project(a)lists.gforge.inria.fr >>>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>>>> . >>>>>> >>>>>> >>>>>> >>>>>> >>>>> _______________________________________________ >>>>> Pharo-project mailing list >>>>> Pharo-project(a)lists.gforge.inria.fr >>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>>> >>>>> >>>>> >>>> >>>> -- >>>> Best regards, >>>> Igor Stasenko AKA sig. >>>> . >>>> >>>> >>>> >>> _______________________________________________ >>> Pharo-project mailing list >>> Pharo-project(a)lists.gforge.inria.fr >>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>> >>> >> >> >> >> -- >> Best regards, >> Igor Stasenko AKA sig. >> . >> >> > > _______________________________________________ > Pharo-project mailing list > Pharo-project(a)lists.gforge.inria.fr > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > -- Best regards, Igor Stasenko AKA sig.

July 3, 2009

Re: [Pharo-project] FasterSets improvements

by Andres Valloud

Unlike arrayed collections, hashed collections use a sentinel value for implementation purposes. Such collections shouldn't be allowed to contain the sentinel because doing so introduces unnecessary complexity for the vast majority of practical applications. It's not worth it. If adding nil to a set is important, then using self as the sentinel could do (at the cost of initializing the set to contain self at all slots when the instance is created, thus duplicating the nilling the VM does for you). But then, the set shouldn't contain itself. Now, this is just my opinion... :). Igor Stasenko wrote: > 2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: > >> I think it's just a bad idea... enumeration of the set would become >> unnecessarily complicated, and a set that includes itself is asking for >> trouble. For example, how do you hash the set? How do you determine >> whether a set that includes itself is equal to itself? How do these >> print themselves? In Smalltalk everything is an object, and >> consequently, some object must represent the absence of an object in a >> storage slot. That object cannot, at the same time, represent the >> presence of something. >> >> > well, you can try and see what will happen if you try this: > > set := Set new. > set add: set. > set printString. > > or try using Array instead of set. > Or try using any other object which prints its slots, and put in one > of its slots itself. > What makes a Set an exceptional in this regard? > > > >> Igor Stasenko wrote: >> >>> 2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: >>> >>> >>>> Keep in mind that, as they're implemented, there will be always some >>>> object that cannot be added to a Set because the set uses the object as >>>> a sentinel value. On the other hand, since it does not make a whole lot >>>> of sense to add a set to itself, maybe it should use self as the >>>> sentinel value. >>>> >>>> >>>> >>> There is no difference what object to use as a sentinel - self or nil. >>> In fact you can use any object which fits your needs. >>> AND be able to include it as an element of set. >>> >>> Just to illustrate: >>> >>> Set>>add: anObject >>> sentinel == anObject ifTrue: [ >>> includesSentinel := true. >>> ]. >>> .... rest of code ... >>> >>> Set>>includes: anObject >>> anObject == sentinel ifTrue: [ ^ indludesSentinel ]. >>> .... rest of code .. >>> >>> >>> >>> >>>> Igor Stasenko wrote: >>>> >>>> >>>>> Anyone considered fixing Sets to allow a nil as an element? >>>>> >>>>> 2009/7/3 Ralph Boland <rpboland(a)gmail.com>: >>>>> >>>>> >>>>> >>>>>> 2009/7/1 Henrik Johansen <henrik.s.johansen(a)veloxit.no>: >>>>>> >>>>>> >>>>>> >>>>>>> I took some time to check this yesterday, my comments on the code itself are >>>>>>> basically the same as what Lucas posted in the issue tracker. >>>>>>> Works as advertised, but should probably clean out the "gunk" which is >>>>>>> unused after it's integrated. >>>>>>> >>>>>>> Some feedback on possible improvements: >>>>>>> - What it does, is basically speed up the adds to the new array in a grow >>>>>>> that might happen Why not take this all to the logical conclusion, and >>>>>>> instead optimize an addAll: method to be used in grow, that assumes elements >>>>>>> are not in set (and themselves are not duplicates)? F.ex. it could also add >>>>>>> to tally just once, instead of one increase per element as the noCompareAdd: >>>>>>> does. (an add method with no tally add, no = object check, and no >>>>>>> growCheck). >>>>>>> >>>>>>> >>>>>>> - The usual addAll: method can be sped up in a similar way, by performing >>>>>>> one grow operation before adding elements if necessary (pessimistically >>>>>>> assuming all elements are unique), then use an add method which does not >>>>>>> check for grow need at all. (an add method with tally add and = object >>>>>>> check, but no growCheck) >>>>>>> If desirable, you could also shrink the capacity after adds are done, to >>>>>>> match the real amount of new elements. >>>>>>> >>>>>>> Adding (1 to: 1200000) to a new Set with such an addAll: method took 1/2 to >>>>>>> 1/3rd the time of the regular addAll: method, even with the lower grow cost >>>>>>> of FasterSets. (Lower gains if Set is presized, and if hash-calculation is >>>>>>> bigger part of cost) >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> I considered this when I created FasterSets. I didn't do this because I was >>>>>> being conservative. I didn't want FasterSets rejected because it did things >>>>>> that weren't wanted. I am surprised though at the amount of performance >>>>>> gain you see. >>>>>> As things stand I see no reason for not including >>>>>> FasterSets other than the potential of breaking packages that subclass >>>>>> Set and then override methods crucial to FasterSets. >>>>>> My vote is obviously for fixing any packages that get broken rather than >>>>>> rejecting FasterSets. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>>> Note: If you want to stay ANSI-compatible, one tricky point in such an >>>>>>> optimization is adding all objects in collection up to a nil element, then >>>>>>> raising an error. (ie: must produce equivalent results to add: performed of >>>>>>> each of collections elements in sequence ) >>>>>>> >>>>>>> Cheers, >>>>>>> Henry >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> I created FasterSets for adding to Squeak and did not consider Pharo but >>>>>> of course am delighted to have it considered for Pharo. >>>>>> You can of course make whatever improvements you want. >>>>>> I encourage though that you create a FasterSets2 containing all the >>>>>> improvements and leave the original FasterSets alone. I am still interested >>>>>> in having FasterSets included in Squeak and I do not know if the powers that >>>>>> be will want the additional improvements or not so I prefer that both versions >>>>>> exist. >>>>>> >>>>>> One thing that I have in my code is a method called nastyAddNew: which >>>>>> adds an element to a set without checking if the element is already there. >>>>>> If the element is already there of course it gets added again and your set >>>>>> contains two copies of the element; thus your set is corrupted. This problem >>>>>> explains the 'nasty' part of the name nastyAddNew. I also have a method >>>>>> addNew: which reports an error if the object is already in the set. >>>>>> nastyAddNew: can be used in set operations such as copy and intersection >>>>>> making these operations faster and can also be used externally by brave users. >>>>>> >>>>>> If improvements to FasterSets are being contemplated then I suggest that >>>>>> adding methods addNew: and nastyAddNew: be considered. If nastyAddNew: >>>>>> is added I see no point in making it private because some users will >>>>>> use it anyway. >>>>>> I am not necessarily voting for their inclusion because we are then >>>>>> giving the user the opportunity to corrupt his data. >>>>>> Of course I do use them in my current project. >>>>>> thus, for my current project at least, I am a brave user. >>>>>> >>>>>> Regards, >>>>>> >>>>>> Ralph Boland >>>>>> >>>>>> _______________________________________________ >>>>>> Pharo-project mailing list >>>>>> Pharo-project(a)lists.gforge.inria.fr >>>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>>>> >>>>>> >>>>>> >>>>>> >>>>> -- >>>>> Best regards, >>>>> Igor Stasenko AKA sig. >>>>> >>>>> _______________________________________________ >>>>> Pharo-project mailing list >>>>> Pharo-project(a)lists.gforge.inria.fr >>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>>> . >>>>> >>>>> >>>>> >>>>> >>>> _______________________________________________ >>>> Pharo-project mailing list >>>> Pharo-project(a)lists.gforge.inria.fr >>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>> >>>> >>>> >>> >>> -- >>> Best regards, >>> Igor Stasenko AKA sig. >>> . >>> >>> >>> >> _______________________________________________ >> Pharo-project mailing list >> Pharo-project(a)lists.gforge.inria.fr >> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >> >> > > > > -- > Best regards, > Igor Stasenko AKA sig. > . > >

July 3, 2009

Re: [Pharo-project] FasterSets improvements

by Igor Stasenko

2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: > I think it's just a bad idea... enumeration of the set would become > unnecessarily complicated, and a set that includes itself is asking for > trouble. Â For example, how do you hash the set? Â How do you determine > whether a set that includes itself is equal to itself? Â How do these > print themselves? Â In Smalltalk everything is an object, and > consequently, some object must represent the absence of an object in a > storage slot. Â That object cannot, at the same time, represent the > presence of something. > well, you can try and see what will happen if you try this: set := Set new. set add: set. set printString. or try using Array instead of set. Or try using any other object which prints its slots, and put in one of its slots itself. What makes a Set an exceptional in this regard? > Igor Stasenko wrote: >> 2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: >> >>> Keep in mind that, as they're implemented, there will be always some >>> object that cannot be added to a Set because the set uses the object as >>> a sentinel value. Â On the other hand, since it does not make a whole lot >>> of sense to add a set to itself, maybe it should use self as the >>> sentinel value. >>> >>> >> There is no difference what object to use as a sentinel - self or nil. >> In fact you can use any object which fits your needs. >> AND be able to include it as an element of set. >> >> Just to illustrate: >> >> Set>>add: anObject >> Â sentinel == anObject ifTrue: [ >> Â Â includesSentinel := true. >> Â ]. >> .... rest of code ... >> >> Set>>includes: anObject >> Â anObject == sentinel Â ifTrue: [ ^ indludesSentinel ]. >> .... rest of code .. >> >> >> >>> Igor Stasenko wrote: >>> >>>> Anyone considered fixing Sets to allow a nil as an element? >>>> >>>> 2009/7/3 Ralph Boland <rpboland(a)gmail.com>: >>>> >>>> >>>>> 2009/7/1 Henrik Johansen <henrik.s.johansen(a)veloxit.no>: >>>>> >>>>> >>>>>> I took some time to check this yesterday, my comments on the code itself are >>>>>> basically the same as what Lucas posted in the issue tracker. >>>>>> Works as advertised, but should probably clean out the "gunk" which is >>>>>> unused after it's integrated. >>>>>> >>>>>> Some feedback on possible improvements: >>>>>> - What it does, is basically speed up the adds to the new array in a grow >>>>>> that might happen Why not take this all to the logical conclusion, and >>>>>> instead optimize an addAll: method to be used in grow, that assumes elements >>>>>> are not in set (and themselves are not duplicates)? F.ex. it could also add >>>>>> to tally just once, instead of one increase per element as the noCompareAdd: >>>>>> does. (an add method with no tally add, no = object check, and no >>>>>> growCheck). >>>>>> >>>>>> >>>>>> - The usual addAll: method can be sped up in a similar way, by performing >>>>>> one grow operation before adding elements if necessary (pessimistically >>>>>> assuming all elements are unique), then use an add method which does not >>>>>> check for grow need at all. (an add method with tally add and = object >>>>>> check, but no growCheck) >>>>>> Â If desirable, you could also shrink the capacity after adds are done, to >>>>>> match the real amount of new elements. >>>>>> >>>>>> Adding (1 to: 1200000) to a new Set with such an addAll: method took 1/2 to >>>>>> Â 1/3rd the time of the regular addAll: method, even with the lower grow cost >>>>>> of FasterSets. (Lower gains if Set is presized, and if hash-calculation is >>>>>> bigger part of cost) >>>>>> >>>>>> >>>>>> >>>>> I considered this when I created FasterSets. Â I didn't do this because I was >>>>> being conservative. Â I didn't want FasterSets rejected because it did things >>>>> that weren't wanted. Â I am surprised Â though at the amount of performance >>>>> gain you see. >>>>> As things stand I see no reason for not including >>>>> FasterSets other than the potential of breaking packages that subclass >>>>> Set and then override methods crucial to FasterSets. >>>>> My vote is obviously for fixing any packages that get broken rather than >>>>> rejecting FasterSets. >>>>> >>>>> >>>>> >>>>>> Note: If you want to stay ANSI-compatible, one tricky point in such an >>>>>> optimization is adding all objects in collection up to a nil element, then >>>>>> raising an error. (ie: must produce equivalent results to add: performed of >>>>>> each of collections elements in sequence ) >>>>>> >>>>>> Cheers, >>>>>> Henry >>>>>> >>>>>> >>>>>> >>>>>> >>>>> I created FasterSets for adding to Squeak and did not consider Pharo but >>>>> of course am delighted to have it considered for Pharo. >>>>> You can of course make whatever improvements you want. >>>>> I encourage though that you create a FasterSets2 containing all the >>>>> improvements and leave the original FasterSets alone. Â I am still interested >>>>> in having FasterSets included in Squeak and I do not know if the powers that >>>>> be will want the additional improvements or not so I prefer that both versions >>>>> exist. >>>>> >>>>> One thing that I have in my code is a method called nastyAddNew: Â which >>>>> adds an element to a set without checking if the element is already there. >>>>> If the element is already there of course it gets added again and your set >>>>> contains two copies of the element; thus your set is corrupted. This problem >>>>> explains the 'nasty' part of the name nastyAddNew. Â I also have a method >>>>> addNew: Â which reports an error if the object is already in the set. >>>>> nastyAddNew: can be used in set operations such as copy and intersection >>>>> making these operations faster and can also be used externally by brave users. >>>>> >>>>> If improvements to FasterSets are being contemplated then I suggest that >>>>> adding methods Â addNew: and Â nastyAddNew: Â be considered. Â If Â nastyAddNew: >>>>> is added I see no point in making it private because some users will >>>>> use it anyway. >>>>> I am not necessarily voting for their inclusion because we are then >>>>> giving the user the opportunity to corrupt his data. >>>>> Of course I do use them in my current project. >>>>> thus, for my current project at least, I am a brave user. >>>>> >>>>> Regards, >>>>> >>>>> Ralph Boland >>>>> >>>>> _______________________________________________ >>>>> Pharo-project mailing list >>>>> Pharo-project(a)lists.gforge.inria.fr >>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>>> >>>>> >>>>> >>>> >>>> -- >>>> Best regards, >>>> Igor Stasenko AKA sig. >>>> >>>> _______________________________________________ >>>> Pharo-project mailing list >>>> Pharo-project(a)lists.gforge.inria.fr >>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>> . >>>> >>>> >>>> >>> _______________________________________________ >>> Pharo-project mailing list >>> Pharo-project(a)lists.gforge.inria.fr >>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>> >>> >> >> >> >> -- >> Best regards, >> Igor Stasenko AKA sig. >> . >> >> > > _______________________________________________ > Pharo-project mailing list > Pharo-project(a)lists.gforge.inria.fr > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > -- Best regards, Igor Stasenko AKA sig.

July 3, 2009

Re: [Pharo-project] FasterSets improvements

by Igor Stasenko

2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: > So you can't have a set with size = 0?... No set isEmpty?... > why? isEmpty ^ tally == 0 and: [ includesSentinel not] > Igor Stasenko wrote: >> 2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: >> >>> Keep in mind that, as they're implemented, there will be always some >>> object that cannot be added to a Set because the set uses the object as >>> a sentinel value. Â On the other hand, since it does not make a whole lot >>> of sense to add a set to itself, maybe it should use self as the >>> sentinel value. >>> >>> >> There is no difference what object to use as a sentinel - self or nil. >> In fact you can use any object which fits your needs. >> AND be able to include it as an element of set. >> >> Just to illustrate: >> >> Set>>add: anObject >> Â sentinel == anObject ifTrue: [ >> Â Â includesSentinel := true. >> Â ]. >> .... rest of code ... >> >> Set>>includes: anObject >> Â anObject == sentinel Â ifTrue: [ ^ indludesSentinel ]. >> .... rest of code .. >> >> >> >>> Igor Stasenko wrote: >>> >>>> Anyone considered fixing Sets to allow a nil as an element? >>>> >>>> 2009/7/3 Ralph Boland <rpboland(a)gmail.com>: >>>> >>>> >>>>> 2009/7/1 Henrik Johansen <henrik.s.johansen(a)veloxit.no>: >>>>> >>>>> >>>>>> I took some time to check this yesterday, my comments on the code itself are >>>>>> basically the same as what Lucas posted in the issue tracker. >>>>>> Works as advertised, but should probably clean out the "gunk" which is >>>>>> unused after it's integrated. >>>>>> >>>>>> Some feedback on possible improvements: >>>>>> - What it does, is basically speed up the adds to the new array in a grow >>>>>> that might happen Why not take this all to the logical conclusion, and >>>>>> instead optimize an addAll: method to be used in grow, that assumes elements >>>>>> are not in set (and themselves are not duplicates)? F.ex. it could also add >>>>>> to tally just once, instead of one increase per element as the noCompareAdd: >>>>>> does. (an add method with no tally add, no = object check, and no >>>>>> growCheck). >>>>>> >>>>>> >>>>>> - The usual addAll: method can be sped up in a similar way, by performing >>>>>> one grow operation before adding elements if necessary (pessimistically >>>>>> assuming all elements are unique), then use an add method which does not >>>>>> check for grow need at all. (an add method with tally add and = object >>>>>> check, but no growCheck) >>>>>> Â If desirable, you could also shrink the capacity after adds are done, to >>>>>> match the real amount of new elements. >>>>>> >>>>>> Adding (1 to: 1200000) to a new Set with such an addAll: method took 1/2 to >>>>>> Â 1/3rd the time of the regular addAll: method, even with the lower grow cost >>>>>> of FasterSets. (Lower gains if Set is presized, and if hash-calculation is >>>>>> bigger part of cost) >>>>>> >>>>>> >>>>>> >>>>> I considered this when I created FasterSets. Â I didn't do this because I was >>>>> being conservative. Â I didn't want FasterSets rejected because it did things >>>>> that weren't wanted. Â I am surprised Â though at the amount of performance >>>>> gain you see. >>>>> As things stand I see no reason for not including >>>>> FasterSets other than the potential of breaking packages that subclass >>>>> Set and then override methods crucial to FasterSets. >>>>> My vote is obviously for fixing any packages that get broken rather than >>>>> rejecting FasterSets. >>>>> >>>>> >>>>> >>>>>> Note: If you want to stay ANSI-compatible, one tricky point in such an >>>>>> optimization is adding all objects in collection up to a nil element, then >>>>>> raising an error. (ie: must produce equivalent results to add: performed of >>>>>> each of collections elements in sequence ) >>>>>> >>>>>> Cheers, >>>>>> Henry >>>>>> >>>>>> >>>>>> >>>>>> >>>>> I created FasterSets for adding to Squeak and did not consider Pharo but >>>>> of course am delighted to have it considered for Pharo. >>>>> You can of course make whatever improvements you want. >>>>> I encourage though that you create a FasterSets2 containing all the >>>>> improvements and leave the original FasterSets alone. Â I am still interested >>>>> in having FasterSets included in Squeak and I do not know if the powers that >>>>> be will want the additional improvements or not so I prefer that both versions >>>>> exist. >>>>> >>>>> One thing that I have in my code is a method called nastyAddNew: Â which >>>>> adds an element to a set without checking if the element is already there. >>>>> If the element is already there of course it gets added again and your set >>>>> contains two copies of the element; thus your set is corrupted. This problem >>>>> explains the 'nasty' part of the name nastyAddNew. Â I also have a method >>>>> addNew: Â which reports an error if the object is already in the set. >>>>> nastyAddNew: can be used in set operations such as copy and intersection >>>>> making these operations faster and can also be used externally by brave users. >>>>> >>>>> If improvements to FasterSets are being contemplated then I suggest that >>>>> adding methods Â addNew: and Â nastyAddNew: Â be considered. Â If Â nastyAddNew: >>>>> is added I see no point in making it private because some users will >>>>> use it anyway. >>>>> I am not necessarily voting for their inclusion because we are then >>>>> giving the user the opportunity to corrupt his data. >>>>> Of course I do use them in my current project. >>>>> thus, for my current project at least, I am a brave user. >>>>> >>>>> Regards, >>>>> >>>>> Ralph Boland >>>>> >>>>> _______________________________________________ >>>>> Pharo-project mailing list >>>>> Pharo-project(a)lists.gforge.inria.fr >>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>>> >>>>> >>>>> >>>> >>>> -- >>>> Best regards, >>>> Igor Stasenko AKA sig. >>>> >>>> _______________________________________________ >>>> Pharo-project mailing list >>>> Pharo-project(a)lists.gforge.inria.fr >>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>> . >>>> >>>> >>>> >>> _______________________________________________ >>> Pharo-project mailing list >>> Pharo-project(a)lists.gforge.inria.fr >>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>> >>> >> >> >> >> -- >> Best regards, >> Igor Stasenko AKA sig. >> . >> >> > > _______________________________________________ > Pharo-project mailing list > Pharo-project(a)lists.gforge.inria.fr > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > -- Best regards, Igor Stasenko AKA sig.

July 3, 2009

Re: [Pharo-project] FasterSets improvements

by Andres Valloud

I think it's just a bad idea... enumeration of the set would become unnecessarily complicated, and a set that includes itself is asking for trouble. For example, how do you hash the set? How do you determine whether a set that includes itself is equal to itself? How do these print themselves? In Smalltalk everything is an object, and consequently, some object must represent the absence of an object in a storage slot. That object cannot, at the same time, represent the presence of something. Igor Stasenko wrote: > 2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: > >> Keep in mind that, as they're implemented, there will be always some >> object that cannot be added to a Set because the set uses the object as >> a sentinel value. On the other hand, since it does not make a whole lot >> of sense to add a set to itself, maybe it should use self as the >> sentinel value. >> >> > There is no difference what object to use as a sentinel - self or nil. > In fact you can use any object which fits your needs. > AND be able to include it as an element of set. > > Just to illustrate: > > Set>>add: anObject > sentinel == anObject ifTrue: [ > includesSentinel := true. > ]. > .... rest of code ... > > Set>>includes: anObject > anObject == sentinel ifTrue: [ ^ indludesSentinel ]. > .... rest of code .. > > > >> Igor Stasenko wrote: >> >>> Anyone considered fixing Sets to allow a nil as an element? >>> >>> 2009/7/3 Ralph Boland <rpboland(a)gmail.com>: >>> >>> >>>> 2009/7/1 Henrik Johansen <henrik.s.johansen(a)veloxit.no>: >>>> >>>> >>>>> I took some time to check this yesterday, my comments on the code itself are >>>>> basically the same as what Lucas posted in the issue tracker. >>>>> Works as advertised, but should probably clean out the "gunk" which is >>>>> unused after it's integrated. >>>>> >>>>> Some feedback on possible improvements: >>>>> - What it does, is basically speed up the adds to the new array in a grow >>>>> that might happen Why not take this all to the logical conclusion, and >>>>> instead optimize an addAll: method to be used in grow, that assumes elements >>>>> are not in set (and themselves are not duplicates)? F.ex. it could also add >>>>> to tally just once, instead of one increase per element as the noCompareAdd: >>>>> does. (an add method with no tally add, no = object check, and no >>>>> growCheck). >>>>> >>>>> >>>>> - The usual addAll: method can be sped up in a similar way, by performing >>>>> one grow operation before adding elements if necessary (pessimistically >>>>> assuming all elements are unique), then use an add method which does not >>>>> check for grow need at all. (an add method with tally add and = object >>>>> check, but no growCheck) >>>>> If desirable, you could also shrink the capacity after adds are done, to >>>>> match the real amount of new elements. >>>>> >>>>> Adding (1 to: 1200000) to a new Set with such an addAll: method took 1/2 to >>>>> 1/3rd the time of the regular addAll: method, even with the lower grow cost >>>>> of FasterSets. (Lower gains if Set is presized, and if hash-calculation is >>>>> bigger part of cost) >>>>> >>>>> >>>>> >>>> I considered this when I created FasterSets. I didn't do this because I was >>>> being conservative. I didn't want FasterSets rejected because it did things >>>> that weren't wanted. I am surprised though at the amount of performance >>>> gain you see. >>>> As things stand I see no reason for not including >>>> FasterSets other than the potential of breaking packages that subclass >>>> Set and then override methods crucial to FasterSets. >>>> My vote is obviously for fixing any packages that get broken rather than >>>> rejecting FasterSets. >>>> >>>> >>>> >>>>> Note: If you want to stay ANSI-compatible, one tricky point in such an >>>>> optimization is adding all objects in collection up to a nil element, then >>>>> raising an error. (ie: must produce equivalent results to add: performed of >>>>> each of collections elements in sequence ) >>>>> >>>>> Cheers, >>>>> Henry >>>>> >>>>> >>>>> >>>>> >>>> I created FasterSets for adding to Squeak and did not consider Pharo but >>>> of course am delighted to have it considered for Pharo. >>>> You can of course make whatever improvements you want. >>>> I encourage though that you create a FasterSets2 containing all the >>>> improvements and leave the original FasterSets alone. I am still interested >>>> in having FasterSets included in Squeak and I do not know if the powers that >>>> be will want the additional improvements or not so I prefer that both versions >>>> exist. >>>> >>>> One thing that I have in my code is a method called nastyAddNew: which >>>> adds an element to a set without checking if the element is already there. >>>> If the element is already there of course it gets added again and your set >>>> contains two copies of the element; thus your set is corrupted. This problem >>>> explains the 'nasty' part of the name nastyAddNew. I also have a method >>>> addNew: which reports an error if the object is already in the set. >>>> nastyAddNew: can be used in set operations such as copy and intersection >>>> making these operations faster and can also be used externally by brave users. >>>> >>>> If improvements to FasterSets are being contemplated then I suggest that >>>> adding methods addNew: and nastyAddNew: be considered. If nastyAddNew: >>>> is added I see no point in making it private because some users will >>>> use it anyway. >>>> I am not necessarily voting for their inclusion because we are then >>>> giving the user the opportunity to corrupt his data. >>>> Of course I do use them in my current project. >>>> thus, for my current project at least, I am a brave user. >>>> >>>> Regards, >>>> >>>> Ralph Boland >>>> >>>> _______________________________________________ >>>> Pharo-project mailing list >>>> Pharo-project(a)lists.gforge.inria.fr >>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>> >>>> >>>> >>> >>> -- >>> Best regards, >>> Igor Stasenko AKA sig. >>> >>> _______________________________________________ >>> Pharo-project mailing list >>> Pharo-project(a)lists.gforge.inria.fr >>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>> . >>> >>> >>> >> _______________________________________________ >> Pharo-project mailing list >> Pharo-project(a)lists.gforge.inria.fr >> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >> >> > > > > -- > Best regards, > Igor Stasenko AKA sig. > . > >

July 3, 2009

Re: [Pharo-project] FasterSets improvements

by Andres Valloud

So you can't have a set with size = 0?... No set isEmpty?... Igor Stasenko wrote: > 2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: > >> Keep in mind that, as they're implemented, there will be always some >> object that cannot be added to a Set because the set uses the object as >> a sentinel value. On the other hand, since it does not make a whole lot >> of sense to add a set to itself, maybe it should use self as the >> sentinel value. >> >> > There is no difference what object to use as a sentinel - self or nil. > In fact you can use any object which fits your needs. > AND be able to include it as an element of set. > > Just to illustrate: > > Set>>add: anObject > sentinel == anObject ifTrue: [ > includesSentinel := true. > ]. > .... rest of code ... > > Set>>includes: anObject > anObject == sentinel ifTrue: [ ^ indludesSentinel ]. > .... rest of code .. > > > >> Igor Stasenko wrote: >> >>> Anyone considered fixing Sets to allow a nil as an element? >>> >>> 2009/7/3 Ralph Boland <rpboland(a)gmail.com>: >>> >>> >>>> 2009/7/1 Henrik Johansen <henrik.s.johansen(a)veloxit.no>: >>>> >>>> >>>>> I took some time to check this yesterday, my comments on the code itself are >>>>> basically the same as what Lucas posted in the issue tracker. >>>>> Works as advertised, but should probably clean out the "gunk" which is >>>>> unused after it's integrated. >>>>> >>>>> Some feedback on possible improvements: >>>>> - What it does, is basically speed up the adds to the new array in a grow >>>>> that might happen Why not take this all to the logical conclusion, and >>>>> instead optimize an addAll: method to be used in grow, that assumes elements >>>>> are not in set (and themselves are not duplicates)? F.ex. it could also add >>>>> to tally just once, instead of one increase per element as the noCompareAdd: >>>>> does. (an add method with no tally add, no = object check, and no >>>>> growCheck). >>>>> >>>>> >>>>> - The usual addAll: method can be sped up in a similar way, by performing >>>>> one grow operation before adding elements if necessary (pessimistically >>>>> assuming all elements are unique), then use an add method which does not >>>>> check for grow need at all. (an add method with tally add and = object >>>>> check, but no growCheck) >>>>> If desirable, you could also shrink the capacity after adds are done, to >>>>> match the real amount of new elements. >>>>> >>>>> Adding (1 to: 1200000) to a new Set with such an addAll: method took 1/2 to >>>>> 1/3rd the time of the regular addAll: method, even with the lower grow cost >>>>> of FasterSets. (Lower gains if Set is presized, and if hash-calculation is >>>>> bigger part of cost) >>>>> >>>>> >>>>> >>>> I considered this when I created FasterSets. I didn't do this because I was >>>> being conservative. I didn't want FasterSets rejected because it did things >>>> that weren't wanted. I am surprised though at the amount of performance >>>> gain you see. >>>> As things stand I see no reason for not including >>>> FasterSets other than the potential of breaking packages that subclass >>>> Set and then override methods crucial to FasterSets. >>>> My vote is obviously for fixing any packages that get broken rather than >>>> rejecting FasterSets. >>>> >>>> >>>> >>>>> Note: If you want to stay ANSI-compatible, one tricky point in such an >>>>> optimization is adding all objects in collection up to a nil element, then >>>>> raising an error. (ie: must produce equivalent results to add: performed of >>>>> each of collections elements in sequence ) >>>>> >>>>> Cheers, >>>>> Henry >>>>> >>>>> >>>>> >>>>> >>>> I created FasterSets for adding to Squeak and did not consider Pharo but >>>> of course am delighted to have it considered for Pharo. >>>> You can of course make whatever improvements you want. >>>> I encourage though that you create a FasterSets2 containing all the >>>> improvements and leave the original FasterSets alone. I am still interested >>>> in having FasterSets included in Squeak and I do not know if the powers that >>>> be will want the additional improvements or not so I prefer that both versions >>>> exist. >>>> >>>> One thing that I have in my code is a method called nastyAddNew: which >>>> adds an element to a set without checking if the element is already there. >>>> If the element is already there of course it gets added again and your set >>>> contains two copies of the element; thus your set is corrupted. This problem >>>> explains the 'nasty' part of the name nastyAddNew. I also have a method >>>> addNew: which reports an error if the object is already in the set. >>>> nastyAddNew: can be used in set operations such as copy and intersection >>>> making these operations faster and can also be used externally by brave users. >>>> >>>> If improvements to FasterSets are being contemplated then I suggest that >>>> adding methods addNew: and nastyAddNew: be considered. If nastyAddNew: >>>> is added I see no point in making it private because some users will >>>> use it anyway. >>>> I am not necessarily voting for their inclusion because we are then >>>> giving the user the opportunity to corrupt his data. >>>> Of course I do use them in my current project. >>>> thus, for my current project at least, I am a brave user. >>>> >>>> Regards, >>>> >>>> Ralph Boland >>>> >>>> _______________________________________________ >>>> Pharo-project mailing list >>>> Pharo-project(a)lists.gforge.inria.fr >>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>> >>>> >>>> >>> >>> -- >>> Best regards, >>> Igor Stasenko AKA sig. >>> >>> _______________________________________________ >>> Pharo-project mailing list >>> Pharo-project(a)lists.gforge.inria.fr >>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>> . >>> >>> >>> >> _______________________________________________ >> Pharo-project mailing list >> Pharo-project(a)lists.gforge.inria.fr >> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >> >> > > > > -- > Best regards, > Igor Stasenko AKA sig. > . > >

July 3, 2009

Re: [Pharo-project] FasterSets improvements

by Igor Stasenko

2009/7/3 Andres Valloud <avalloud(a)smalltalk.comcastbiz.net>: > Keep in mind that, as they're implemented, there will be always some > object that cannot be added to a Set because the set uses the object as > a sentinel value. Â On the other hand, since it does not make a whole lot > of sense to add a set to itself, maybe it should use self as the > sentinel value. > There is no difference what object to use as a sentinel - self or nil. In fact you can use any object which fits your needs. AND be able to include it as an element of set. Just to illustrate: Set>>add: anObject sentinel == anObject ifTrue: [ includesSentinel := true. ]. .... rest of code ... Set>>includes: anObject anObject == sentinel ifTrue: [ ^ indludesSentinel ]. .... rest of code .. > Igor Stasenko wrote: >> Anyone considered fixing Sets to allow a nil as an element? >> >> 2009/7/3 Ralph Boland <rpboland(a)gmail.com>: >> >>> 2009/7/1 Henrik Johansen <henrik.s.johansen(a)veloxit.no>: >>> >>>> I took some time to check this yesterday, my comments on the code itself are >>>> basically the same as what Lucas posted in the issue tracker. >>>> Works as advertised, but should probably clean out the "gunk" which is >>>> unused after it's integrated. >>>> >>>> Some feedback on possible improvements: >>>> - What it does, is basically speed up the adds to the new array in a grow >>>> that might happen Why not take this all to the logical conclusion, and >>>> instead optimize an addAll: method to be used in grow, that assumes elements >>>> are not in set (and themselves are not duplicates)? F.ex. it could also add >>>> to tally just once, instead of one increase per element as the noCompareAdd: >>>> does. (an add method with no tally add, no = object check, and no >>>> growCheck). >>>> >>> >>>> - The usual addAll: method can be sped up in a similar way, by performing >>>> one grow operation before adding elements if necessary (pessimistically >>>> assuming all elements are unique), then use an add method which does not >>>> check for grow need at all. (an add method with tally add and = object >>>> check, but no growCheck) >>>> Â If desirable, you could also shrink the capacity after adds are done, to >>>> match the real amount of new elements. >>>> >>>> Adding (1 to: 1200000) to a new Set with such an addAll: method took 1/2 to >>>> Â 1/3rd the time of the regular addAll: method, even with the lower grow cost >>>> of FasterSets. (Lower gains if Set is presized, and if hash-calculation is >>>> bigger part of cost) >>>> >>>> >>> I considered this when I created FasterSets. Â I didn't do this because I was >>> being conservative. Â I didn't want FasterSets rejected because it did things >>> that weren't wanted. Â I am surprised Â though at the amount of performance >>> gain you see. >>> As things stand I see no reason for not including >>> FasterSets other than the potential of breaking packages that subclass >>> Set and then override methods crucial to FasterSets. >>> My vote is obviously for fixing any packages that get broken rather than >>> rejecting FasterSets. >>> >>> >>>> Note: If you want to stay ANSI-compatible, one tricky point in such an >>>> optimization is adding all objects in collection up to a nil element, then >>>> raising an error. (ie: must produce equivalent results to add: performed of >>>> each of collections elements in sequence ) >>>> >>>> Cheers, >>>> Henry >>>> >>>> >>>> >>> I created FasterSets for adding to Squeak and did not consider Pharo but >>> of course am delighted to have it considered for Pharo. >>> You can of course make whatever improvements you want. >>> I encourage though that you create a FasterSets2 containing all the >>> improvements and leave the original FasterSets alone. Â I am still interested >>> in having FasterSets included in Squeak and I do not know if the powers that >>> be will want the additional improvements or not so I prefer that both versions >>> exist. >>> >>> One thing that I have in my code is a method called nastyAddNew: Â which >>> adds an element to a set without checking if the element is already there. >>> If the element is already there of course it gets added again and your set >>> contains two copies of the element; thus your set is corrupted. This problem >>> explains the 'nasty' part of the name nastyAddNew. Â I also have a method >>> addNew: Â which reports an error if the object is already in the set. >>> nastyAddNew: can be used in set operations such as copy and intersection >>> making these operations faster and can also be used externally by brave users. >>> >>> If improvements to FasterSets are being contemplated then I suggest that >>> adding methods Â addNew: and Â nastyAddNew: Â be considered. Â If Â nastyAddNew: >>> is added I see no point in making it private because some users will >>> use it anyway. >>> I am not necessarily voting for their inclusion because we are then >>> giving the user the opportunity to corrupt his data. >>> Of course I do use them in my current project. >>> thus, for my current project at least, I am a brave user. >>> >>> Regards, >>> >>> Ralph Boland >>> >>> _______________________________________________ >>> Pharo-project mailing list >>> Pharo-project(a)lists.gforge.inria.fr >>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>> >>> >> >> >> >> -- >> Best regards, >> Igor Stasenko AKA sig. >> >> _______________________________________________ >> Pharo-project mailing list >> Pharo-project(a)lists.gforge.inria.fr >> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >> . >> >> > > _______________________________________________ > Pharo-project mailing list > Pharo-project(a)lists.gforge.inria.fr > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > -- Best regards, Igor Stasenko AKA sig.

July 3, 2009

Re: [Pharo-project] FasterSets improvements

by Andres Valloud

Keep in mind that, as they're implemented, there will be always some object that cannot be added to a Set because the set uses the object as a sentinel value. On the other hand, since it does not make a whole lot of sense to add a set to itself, maybe it should use self as the sentinel value. Igor Stasenko wrote: > Anyone considered fixing Sets to allow a nil as an element? > > 2009/7/3 Ralph Boland <rpboland(a)gmail.com>: > >> 2009/7/1 Henrik Johansen <henrik.s.johansen(a)veloxit.no>: >> >>> I took some time to check this yesterday, my comments on the code itself are >>> basically the same as what Lucas posted in the issue tracker. >>> Works as advertised, but should probably clean out the "gunk" which is >>> unused after it's integrated. >>> >>> Some feedback on possible improvements: >>> - What it does, is basically speed up the adds to the new array in a grow >>> that might happen Why not take this all to the logical conclusion, and >>> instead optimize an addAll: method to be used in grow, that assumes elements >>> are not in set (and themselves are not duplicates)? F.ex. it could also add >>> to tally just once, instead of one increase per element as the noCompareAdd: >>> does. (an add method with no tally add, no = object check, and no >>> growCheck). >>> >> >>> - The usual addAll: method can be sped up in a similar way, by performing >>> one grow operation before adding elements if necessary (pessimistically >>> assuming all elements are unique), then use an add method which does not >>> check for grow need at all. (an add method with tally add and = object >>> check, but no growCheck) >>> If desirable, you could also shrink the capacity after adds are done, to >>> match the real amount of new elements. >>> >>> Adding (1 to: 1200000) to a new Set with such an addAll: method took 1/2 to >>> 1/3rd the time of the regular addAll: method, even with the lower grow cost >>> of FasterSets. (Lower gains if Set is presized, and if hash-calculation is >>> bigger part of cost) >>> >>> >> I considered this when I created FasterSets. I didn't do this because I was >> being conservative. I didn't want FasterSets rejected because it did things >> that weren't wanted. I am surprised though at the amount of performance >> gain you see. >> As things stand I see no reason for not including >> FasterSets other than the potential of breaking packages that subclass >> Set and then override methods crucial to FasterSets. >> My vote is obviously for fixing any packages that get broken rather than >> rejecting FasterSets. >> >> >>> Note: If you want to stay ANSI-compatible, one tricky point in such an >>> optimization is adding all objects in collection up to a nil element, then >>> raising an error. (ie: must produce equivalent results to add: performed of >>> each of collections elements in sequence ) >>> >>> Cheers, >>> Henry >>> >>> >>> >> I created FasterSets for adding to Squeak and did not consider Pharo but >> of course am delighted to have it considered for Pharo. >> You can of course make whatever improvements you want. >> I encourage though that you create a FasterSets2 containing all the >> improvements and leave the original FasterSets alone. I am still interested >> in having FasterSets included in Squeak and I do not know if the powers that >> be will want the additional improvements or not so I prefer that both versions >> exist. >> >> One thing that I have in my code is a method called nastyAddNew: which >> adds an element to a set without checking if the element is already there. >> If the element is already there of course it gets added again and your set >> contains two copies of the element; thus your set is corrupted. This problem >> explains the 'nasty' part of the name nastyAddNew. I also have a method >> addNew: which reports an error if the object is already in the set. >> nastyAddNew: can be used in set operations such as copy and intersection >> making these operations faster and can also be used externally by brave users. >> >> If improvements to FasterSets are being contemplated then I suggest that >> adding methods addNew: and nastyAddNew: be considered. If nastyAddNew: >> is added I see no point in making it private because some users will >> use it anyway. >> I am not necessarily voting for their inclusion because we are then >> giving the user the opportunity to corrupt his data. >> Of course I do use them in my current project. >> thus, for my current project at least, I am a brave user. >> >> Regards, >> >> Ralph Boland >> >> _______________________________________________ >> Pharo-project mailing list >> Pharo-project(a)lists.gforge.inria.fr >> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >> >> > > > > -- > Best regards, > Igor Stasenko AKA sig. > > _______________________________________________ > Pharo-project mailing list > Pharo-project(a)lists.gforge.inria.fr > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > . > >

July 3, 2009

Re: [Pharo-project] FasterSets improvements

by Igor Stasenko

Anyone considered fixing Sets to allow a nil as an element? 2009/7/3 Ralph Boland <rpboland(a)gmail.com>: > 2009/7/1 Henrik Johansen <henrik.s.johansen(a)veloxit.no>: >> I took some time to check this yesterday, my comments on the code itself are >> basically the same as what Lucas posted in the issue tracker. >> Works as advertised, but should probably clean out the "gunk" which is >> unused after it's integrated. >> >> Some feedback on possible improvements: >> - What it does, is basically speed up the adds to the new array in a grow >> that might happen Why not take this all to the logical conclusion, and >> instead optimize an addAll: method to be used in grow, that assumes elements >> are not in set (and themselves are not duplicates)? F.ex. it could also add >> to tally just once, instead of one increase per element as the noCompareAdd: >> does. (an add method with no tally add, no = object check, and no >> growCheck). > > >> >> - The usual addAll: method can be sped up in a similar way, by performing >> one grow operation before adding elements if necessary (pessimistically >> assuming all elements are unique), then use an add method which does not >> check for grow need at all. (an add method with tally add and = object >> check, but no growCheck) >> Â If desirable, you could also shrink the capacity after adds are done, to >> match the real amount of new elements. >> >> Adding (1 to: 1200000) to a new Set with such an addAll: method took 1/2 to >> Â 1/3rd the time of the regular addAll: method, even with the lower grow cost >> of FasterSets. (Lower gains if Set is presized, and if hash-calculation is >> bigger part of cost) >> > > I considered this when I created FasterSets. Â I didn't do this because I was > being conservative. Â I didn't want FasterSets rejected because it did things > that weren't wanted. Â I am surprised Â though at the amount of performance > gain you see. > As things stand I see no reason for not including > FasterSets other than the potential of breaking packages that subclass > Set and then override methods crucial to FasterSets. > My vote is obviously for fixing any packages that get broken rather than > rejecting FasterSets. > >> Note: If you want to stay ANSI-compatible, one tricky point in such an >> optimization is adding all objects in collection up to a nil element, then >> raising an error. (ie: must produce equivalent results to add: performed of >> each of collections elements in sequence ) >> >> Cheers, >> Henry >> >> > > I created FasterSets for adding to Squeak and did not consider Pharo but > of course am delighted to have it considered for Pharo. > You can of course make whatever improvements you want. > I encourage though that you create a FasterSets2 containing all the > improvements and leave the original FasterSets alone. Â I am still interested > in having FasterSets included in Squeak and I do not know if the powers that > be will want the additional improvements or not so I prefer that both versions > exist. > > One thing that I have in my code is a method called nastyAddNew: Â which > adds an element to a set without checking if the element is already there. > If the element is already there of course it gets added again and your set > contains two copies of the element; thus your set is corrupted. This problem > explains the 'nasty' part of the name nastyAddNew. Â I also have a method > addNew: Â which reports an error if the object is already in the set. > nastyAddNew: can be used in set operations such as copy and intersection > making these operations faster and can also be used externally by brave users. > > If improvements to FasterSets are being contemplated then I suggest that > adding methods Â addNew: and Â nastyAddNew: Â be considered. Â If Â nastyAddNew: > is added I see no point in making it private because some users will > use it anyway. > I am not necessarily voting for their inclusion because we are then > giving the user the opportunity to corrupt his data. > Of course I do use them in my current project. > thus, for my current project at least, I am a brave user. > > Regards, > > Ralph Boland > > _______________________________________________ > Pharo-project mailing list > Pharo-project(a)lists.gforge.inria.fr > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > -- Best regards, Igor Stasenko AKA sig.

July 3, 2009

[Pharo-project] FasterSets improvements

by Ralph Boland

2009/7/1 Henrik Johansen <henrik.s.johansen(a)veloxit.no>: > I took some time to check this yesterday, my comments on the code itself are > basically the same as what Lucas posted in the issue tracker. > Works as advertised, but should probably clean out the "gunk" which is > unused after it's integrated. > > Some feedback on possible improvements: > - What it does, is basically speed up the adds to the new array in a grow > that might happen Why not take this all to the logical conclusion, and > instead optimize an addAll: method to be used in grow, that assumes elements > are not in set (and themselves are not duplicates)? F.ex. it could also add > to tally just once, instead of one increase per element as the noCompareAdd: > does. (an add method with no tally add, no = object check, and no > growCheck). > > - The usual addAll: method can be sped up in a similar way, by performing > one grow operation before adding elements if necessary (pessimistically > assuming all elements are unique), then use an add method which does not > check for grow need at all. (an add method with tally add and = object > check, but no growCheck) > If desirable, you could also shrink the capacity after adds are done, to > match the real amount of new elements. > > Adding (1 to: 1200000) to a new Set with such an addAll: method took 1/2 to > 1/3rd the time of the regular addAll: method, even with the lower grow cost > of FasterSets. (Lower gains if Set is presized, and if hash-calculation is > bigger part of cost) > I considered this when I created FasterSets. I didn't do this because I was being conservative. I didn't want FasterSets rejected because it did things that weren't wanted. I am surprised though at the amount of performance gain you see. As things stand I see no reason for not including FasterSets other than the potential of breaking packages that subclass Set and then override methods crucial to FasterSets. My vote is obviously for fixing any packages that get broken rather than rejecting FasterSets. > Note: If you want to stay ANSI-compatible, one tricky point in such an > optimization is adding all objects in collection up to a nil element, then > raising an error. (ie: must produce equivalent results to add: performed of > each of collections elements in sequence ) > > Cheers, > Henry > > I created FasterSets for adding to Squeak and did not consider Pharo but of course am delighted to have it considered for Pharo. You can of course make whatever improvements you want. I encourage though that you create a FasterSets2 containing all the improvements and leave the original FasterSets alone. I am still interested in having FasterSets included in Squeak and I do not know if the powers that be will want the additional improvements or not so I prefer that both versions exist. One thing that I have in my code is a method called nastyAddNew: which adds an element to a set without checking if the element is already there. If the element is already there of course it gets added again and your set contains two copies of the element; thus your set is corrupted. This problem explains the 'nasty' part of the name nastyAddNew. I also have a method addNew: which reports an error if the object is already in the set. nastyAddNew: can be used in set operations such as copy and intersection making these operations faster and can also be used externally by brave users. If improvements to FasterSets are being contemplated then I suggest that adding methods addNew: and nastyAddNew: be considered. If nastyAddNew: is added I see no point in making it private because some users will use it anyway. I am not necessarily voting for their inclusion because we are then giving the user the opportunity to corrupt his data. Of course I do use them in my current project. thus, for my current project at least, I am a brave user. Regards, Ralph Boland

July 2, 2009