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.
> 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 >
