about resetTo: in newFrom:
Locked
 
|
Hi levente
Reading your changes I was wondering if resetTo: should not be moved into OrderedCollection class >> new: and why only have it for the default new? OrderedCollection class >>newFrom: aCollection "Answer an instance of me containing the same elements as aCollection." + ^(self new: aCollection size) + resetTo: 1; + addAll: aCollection; + yourself - | newCollection | - newCollection := self new: aCollection size. - newCollection addAll: aCollection. - ^newCollection I did OrderedCollection new: 100. and contrary to what I expected firstIndex is not 1 but 33. Now when you add resetTo: 1 firstIndex get 1. new: anInteger "Create a collection with enough room allocated to contain up to anInteger elements. The new instance will be of size 0 (allocated room is not necessarily used)." ^self basicNew setCollection: (Array new: anInteger) setCollection: anArray array := anArray. self reset reset firstIndex := array size // 3 max: 1. lastIndex := firstIndex - 1 _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
Re: about resetTo: in newFrom:
|
|
On Fri, 25 Dec 2009, Stéphane Ducasse wrote:
> Hi levente > > Reading your changes I was wondering if resetTo: should not be moved into OrderedCollection class >> new: > and why only have it for the default new? The patch below had the purpose to speed up #asOrderedCollection in general. But I think #resetTo: can be moved to #new: or even to #setCollection: on the instance side, since I don't see any benefit for using array size // 3 for firstIndex. I guess the original idea was to leave enough space on both sides of the array, so adding to both sides has good performance, though in general this is not the case, because #makeRoomAtFirst and #makeRoomAtLast doesn't leave space on both sides. o := OrderedCollection new. [ 1 to: 10000 do: [ :each | o addFirst: each; addFirst: each ] ] timeToRun ===> 4 o := OrderedCollection new. [ 1 to: 10000 do: [ :each | o addLast: each; addLast: each ] ] timeToRun ===> 3 o := OrderedCollection new. [ 1 to: 10000 do: [ :each | o addFirst: each; addLast: each ] ] timeToRun ===> 20067 Levente > > OrderedCollection class >>newFrom: aCollection > "Answer an instance of me containing the same elements as aCollection." > > + ^(self new: aCollection size) > + resetTo: 1; > + addAll: aCollection; > + yourself > - | newCollection | > - newCollection := self new: aCollection size. > - newCollection addAll: aCollection. > - ^newCollection > > I did > OrderedCollection new: 100. > and contrary to what I expected firstIndex is not 1 but 33. > Now when you add resetTo: 1 firstIndex get 1. > > new: anInteger > "Create a collection with enough room allocated to contain up to anInteger elements. > The new instance will be of size 0 (allocated room is not necessarily used)." > ^self basicNew setCollection: (Array new: anInteger) > > setCollection: anArray > array := anArray. > self reset > > reset > firstIndex := array size // 3 max: 1. > lastIndex := firstIndex - 1 > > > > > _______________________________________________ > Pharo-project mailing list > [hidden email] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
Re: about resetTo: in newFrom:
|
|
ok thanks
I see the implementation was smart. Stef On Dec 25, 2009, at 7:46 PM, Levente Uzonyi wrote: > On Fri, 25 Dec 2009, Stéphane Ducasse wrote: > >> Hi levente >> >> Reading your changes I was wondering if resetTo: should not be moved into OrderedCollection class >> new: >> and why only have it for the default new? > > The patch below had the purpose to speed up #asOrderedCollection in general. But I think #resetTo: can be moved to #new: or even to #setCollection: on the instance side, since I don't see any benefit for using array size // 3 for firstIndex. I guess the original idea was to leave enough space on both sides of the array, so adding to both sides has good performance, though in general this is not the case, because #makeRoomAtFirst and #makeRoomAtLast doesn't leave space on both sides. > > o := OrderedCollection new. > [ > 1 to: 10000 do: [ :each | > o > addFirst: each; > addFirst: each ] ] timeToRun ===> 4 > > o := OrderedCollection new. > [ > 1 to: 10000 do: [ :each | > o > addLast: each; > addLast: each ] ] timeToRun ===> 3 > > o := OrderedCollection new. > [ > 1 to: 10000 do: [ :each | > o > addFirst: each; > addLast: each ] ] timeToRun ===> 20067 > > > Levente > >> >> OrderedCollection class >>newFrom: aCollection >> "Answer an instance of me containing the same elements as aCollection." >> >> + ^(self new: aCollection size) >> + resetTo: 1; >> + addAll: aCollection; >> + yourself >> - | newCollection | >> - newCollection := self new: aCollection size. >> - newCollection addAll: aCollection. >> - ^newCollection >> >> I did >> OrderedCollection new: 100. >> and contrary to what I expected firstIndex is not 1 but 33. >> Now when you add resetTo: 1 firstIndex get 1. >> >> new: anInteger >> "Create a collection with enough room allocated to contain up to anInteger elements. >> The new instance will be of size 0 (allocated room is not necessarily used)." >> ^self basicNew setCollection: (Array new: anInteger) >> >> setCollection: anArray >> array := anArray. >> self reset >> >> reset >> firstIndex := array size // 3 max: 1. >> lastIndex := firstIndex - 1 >> >> >> >> >> _______________________________________________ >> Pharo-project mailing list >> [hidden email] >> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > _______________________________________________ > Pharo-project mailing list > [hidden email] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
Re: about resetTo: in newFrom:
|
|
In the changes I did to OrderedCollection back at ESUG (but never
finished :/), I simply removed resetTo:, and changed the default to do reset to 1, for the same reasons Levente posted. Cheers, Henry On 25.12.2009 22:05, Stéphane Ducasse wrote: > ok thanks > I see the implementation was smart. > > Stef > On Dec 25, 2009, at 7:46 PM, Levente Uzonyi wrote: > > >> On Fri, 25 Dec 2009, Stéphane Ducasse wrote: >> >> >>> Hi levente >>> >>> Reading your changes I was wondering if resetTo: should not be moved into OrderedCollection class>> new: >>> and why only have it for the default new? >>> >> The patch below had the purpose to speed up #asOrderedCollection in general. But I think #resetTo: can be moved to #new: or even to #setCollection: on the instance side, since I don't see any benefit for using array size // 3 for firstIndex. I guess the original idea was to leave enough space on both sides of the array, so adding to both sides has good performance, though in general this is not the case, because #makeRoomAtFirst and #makeRoomAtLast doesn't leave space on both sides. >> >> o := OrderedCollection new. >> [ >> 1 to: 10000 do: [ :each | >> o >> addFirst: each; >> addFirst: each ] ] timeToRun ===> 4 >> >> o := OrderedCollection new. >> [ >> 1 to: 10000 do: [ :each | >> o >> addLast: each; >> addLast: each ] ] timeToRun ===> 3 >> >> o := OrderedCollection new. >> [ >> 1 to: 10000 do: [ :each | >> o >> addFirst: each; >> addLast: each ] ] timeToRun ===> 20067 >> >> >> Levente >> >> >>> OrderedCollection class>>newFrom: aCollection >>> "Answer an instance of me containing the same elements as aCollection." >>> >>> + ^(self new: aCollection size) >>> + resetTo: 1; >>> + addAll: aCollection; >>> + yourself >>> - | newCollection | >>> - newCollection := self new: aCollection size. >>> - newCollection addAll: aCollection. >>> - ^newCollection >>> >>> I did >>> OrderedCollection new: 100. >>> and contrary to what I expected firstIndex is not 1 but 33. >>> Now when you add resetTo: 1 firstIndex get 1. >>> >>> new: anInteger >>> "Create a collection with enough room allocated to contain up to anInteger elements. >>> The new instance will be of size 0 (allocated room is not necessarily used)." >>> ^self basicNew setCollection: (Array new: anInteger) >>> >>> setCollection: anArray >>> array := anArray. >>> self reset >>> >>> reset >>> firstIndex := array size // 3 max: 1. >>> lastIndex := firstIndex - 1 >>> >>> >>> >>> >>> _______________________________________________ >>> Pharo-project mailing list >>> [hidden email] >>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>> >> _______________________________________________ >> Pharo-project mailing list >> [hidden email] >> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >> > > _______________________________________________ > Pharo-project mailing list > [hidden email] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > > > _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
Re: about resetTo: in newFrom:
|
|
On Fri, 25 Dec 2009, Henrik Sperre Johansen wrote:
> In the changes I did to OrderedCollection back at ESUG (but never > finished :/), I simply removed resetTo:, and changed the default to do > reset to 1, for the same reasons Levente posted. IMO #resetTo: is useful and should not be private, it has senders too. #reset is less useful since it's just resetTo: (array size // 3 max: 1). OrderedCollection can be improved in the following two ways: 1. don't use #reset, but #resetTo: 1 in #setCollection: 2. Fix #makeRoomAtFirst and #makeRoomAtLast to leave free space on both sides, these to methods can probably be further improved. The first version doesn't support (with good performance) addition to both sides, the second does, but has some space overhead. So the question is: should OrderedCollection support additions to both ends with good performance? Levente > > Cheers, > Henry > > On 25.12.2009 22:05, Stéphane Ducasse wrote: >> ok thanks >> I see the implementation was smart. >> >> Stef >> On Dec 25, 2009, at 7:46 PM, Levente Uzonyi wrote: >> >> >>> On Fri, 25 Dec 2009, Stéphane Ducasse wrote: >>> >>> >>>> Hi levente >>>> >>>> Reading your changes I was wondering if resetTo: should not be moved into OrderedCollection class>> new: >>>> and why only have it for the default new? >>>> >>> The patch below had the purpose to speed up #asOrderedCollection in general. But I think #resetTo: can be moved to #new: or even to #setCollection: on the instance side, since I don't see any benefit for using array size // 3 for firstIndex. I guess the original idea was to leave enough space on both sides of the array, so adding to both sides has good performance, though in general this is not the case, because #makeRoomAtFirst and #makeRoomAtLast doesn't leave space on both sides. >>> >>> o := OrderedCollection new. >>> [ >>> 1 to: 10000 do: [ :each | >>> o >>> addFirst: each; >>> addFirst: each ] ] timeToRun ===> 4 >>> >>> o := OrderedCollection new. >>> [ >>> 1 to: 10000 do: [ :each | >>> o >>> addLast: each; >>> addLast: each ] ] timeToRun ===> 3 >>> >>> o := OrderedCollection new. >>> [ >>> 1 to: 10000 do: [ :each | >>> o >>> addFirst: each; >>> addLast: each ] ] timeToRun ===> 20067 >>> >>> >>> Levente >>> >>> >>>> OrderedCollection class>>newFrom: aCollection >>>> "Answer an instance of me containing the same elements as aCollection." >>>> >>>> + ^(self new: aCollection size) >>>> + resetTo: 1; >>>> + addAll: aCollection; >>>> + yourself >>>> - | newCollection | >>>> - newCollection := self new: aCollection size. >>>> - newCollection addAll: aCollection. >>>> - ^newCollection >>>> >>>> I did >>>> OrderedCollection new: 100. >>>> and contrary to what I expected firstIndex is not 1 but 33. >>>> Now when you add resetTo: 1 firstIndex get 1. >>>> >>>> new: anInteger >>>> "Create a collection with enough room allocated to contain up to anInteger elements. >>>> The new instance will be of size 0 (allocated room is not necessarily used)." >>>> ^self basicNew setCollection: (Array new: anInteger) >>>> >>>> setCollection: anArray >>>> array := anArray. >>>> self reset >>>> >>>> reset >>>> firstIndex := array size // 3 max: 1. >>>> lastIndex := firstIndex - 1 >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> Pharo-project mailing list >>>> [hidden email] >>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>> >>> _______________________________________________ >>> Pharo-project mailing list >>> [hidden email] >>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>> >> >> _______________________________________________ >> Pharo-project mailing list >> [hidden email] >> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >> >> >> > > > _______________________________________________ > Pharo-project mailing list > [hidden email] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
«
Return to Pharo Smalltalk Developers
|
1 view|%1 views
| Free forum by Nabble | Edit this page |