2015-12-01 10:26 GMT+01:00 Sven Van Caekenberghe <[email protected]>: > > > On 01 Dec 2015, at 10:24, Thierry Goubier <[email protected]> > wrote: > > > > Note that the sum with #anyOne is wrong. > > > > #(1 2 3 4 5) inject: #(1 2 3 4 5) anyOne into: [ :sum :each | sum + each > ] > > > > returns 16 instead of 15. > > You have subtract the element you picked with #anyOne ! >
I think he means the suggested implementation for newSum is wrong, just because of this reason. > > > Thierry > > > > 2015-12-01 10:18 GMT+01:00 Ben Coman <[email protected]>: > > On Tue, Dec 1, 2015 at 4:45 PM, Sven Van Caekenberghe <[email protected]> > wrote: > > > I am all for a cleanup, the current situation is confusing. > > > The basic #sum should be fast AND work for empty collections with 0 as > starting element. > > > > > I know why the #anyOne is used, and that use case should be preserved, > but it is less common IMHO. > > > > I'm curious to be learn why? > > cheers -ben > > > > > > > >> On 01 Dec 2015, at 09:38, Thierry Goubier <[email protected]> > wrote: > > >> > > >> Hi Max, > > >> > > >> Interesting results... > > >> > > >> replacing the #yourself in newSum: cuts the runtime by a factor of > two. > > >> using an [:x | x ] block cost 10% compared to a direct version of sum. > > >> > > >> So the fastest sum is: > > >> > > >> sum > > >> ^ self > > >> inject: > > >> (self ifEmpty: [ ^ 0 ] ifNotEmpty: [ self > anyOne ]) > > >> into: [ :sum :each | sum + each ] > > >> > > >> And > > >> > > >> sum > > >> ^ self sum: [:x | x ] > > >> > > >> is 10% slower. > > >> > > >> And > > >> > > >> sum > > >> ^ self sum: #yourself > > >> > > >> is 100% slower (i.e. x2) > > >> > > >> It may be wise to avoid using #yourself for a block. > > >> > > >> Thierry > > >> > > >> > > >> 2015-12-01 9:17 GMT+01:00 Max Leske <[email protected]>: > > >> Hi guys, > > >> > > >> Collection defines #sum:, #detectSum: and #sumNumbers:, all of which > accomplish the same (in principal) but with subtle differences: > > >> > > >> #sum: > > >> - uses #inject:into: with #anyOne as the injected element and will > thus fail for empty collections > > >> > > >> #detectSum: > > >> - uses “nextValue + sum” instead of “sum + nextValue” which makes it > a lot slower when dealing with large numbers (primitive fails and number > conversion is necessary) > > >> > > >> #sumNumbers: > > >> - same as #sum but doesn’t fail for empty collections. Only good for > numbers though. > > >> > > >> > > >> Benchmarks: > > >> > > >> [ 100 timesRepeat: [ (1 to: 1000000) sum: #yourself ] ] timeToRun > 18062 > > >> [ 100 timesRepeat: [ (1 to: 1000000) detectSum: #yourself ] ] > timeToRun 42391 > > >> [ 100 timesRepeat: [ (1 to: 1000000) sumNumbers: #yourself ] ] > timeToRun 18096 > > >> > > >> > > >> > > >> Can we settle for a single implementation? Such as (modified from > #sumNumbers:): > > >> > > >> newSum: aBlock > > >> ^ self > > >> inject: (self > > >> ifEmpty: [ 0 ] > > >> ifNotEmpty: [ self anyOne ]) > > >> into: [ :sum :each | sum + (aBlock value: each) ] > > >> > > >> This implementation combines the best of the three implementations I > think. Benchmark: > > >> > > >> [ 100 timesRepeat: [ (1 to: 1000000) newSum: #yourself ] ] timeToRun > 17955 > > >> > > >> > > >> BTW, there is also the message #sum, which suffers from the same > problem as #sum: (the implementation is basically a copy). #sum could be > implemented as > > >> > > >> sum > > >> ^ self sum: [ :x | x ] > > >> > > >> > > >> As for the name for the new method, I suggest #sum:. > > >> > > >> Cheers, > > >> Max > > >> > > > > > > > > > > > > >
