Hi Chris,

On Tue, 21 Jan 2020, Chris Muller wrote:

> Hi Levente,
>  
>       +       lowerLimit <= 7 ifTrue: [
>       +               lowerLimit <= 3 ifTrue: [ ^3 ].
>       +               lowerLimit <= 5 ifTrue: [ ^5 ].
>       +               ^7 ].
> 
> 
> That's better, but it still has that same fundamental problem.  Every time a developer makes a HashedCollection of a known-at-runtime size (e.g., in a variable), they're forced to choose between execution performance pain or code pain.
> 
>     {
>    '[ Dictionary new ]'->'100% of baseline rate, 27,600,000 per second. 36.2 nanoseconds per run. 11.33547 % GC time.'
> 
>     "performance pain?"
>     '[ Dictionary new: 1 ]'->'60% of baseline rate, 16,600,000 per second. 60.1 nanoseconds per run. 5.61888 % GC time.'
>     '[ Dictionary new: 2 ]'->'61% of baseline rate, 16,900,000 per second. 59.2 nanoseconds per run. 5.67886 % GC time.'
>     '[ Dictionary new: 3 ]'->'59% of baseline rate, 16,300,000 per second. 61.5 nanoseconds per run. 6.77864 % GC time.'

Even if there's a performance overhead, you use less memory.

> 
>     "into #sizeFor:"
>     '[ Dictionary new: 4 ]'->'57% of baseline rate, 15,800,000 per second. 63.5 nanoseconds per run. 7.87685 % GC time.'

Starting from 4, you also save time by avoiding growing, which is 
more significant than what you "lose" during instance creation.

> 
>     "or code pain?"
>     '[ sz > 3
>             ifTrue: [ Dictionary new: sz ]
>             ifFalse: [ Dictionary new ] ]'->'97% of baseline rate, 26,900,000 per second. 37.2 nanoseconds per run. 11.09778 % GC time.'}
> 
> For the most part, they won't even know about this anomaly (and shouldn't have to).  No one assumes such a heavy penalty for #new: over #new when the size is <= the default size.

We could get rid of the anomaly by changing #new to ^self new: 3.

> 
> I like every other aspect of your improvements here!  But may we go with the Collections-cmm.873 variation please?  It fixes that issue, check it out!
> 
>     {'[ Dictionary new ]'->'100% of baseline rate, 29,100,000 per second. 34.4 nanoseconds per run. 5.9988 % GC time.'
>     '[ Dictionary new: 1 ]'->'103% of baseline rate, 29,900,000 per second. 33.4 nanoseconds per run. 4.9 % GC time.'
>     '[ Dictionary new: 2 ]'->'103% of baseline rate, 30,100,000 per second. 33.2 nanoseconds per run. 4.88 % GC time.'
>     '[ Dictionary new: 3 ]'->'95% of baseline rate, 27,600,000 per second. 36.3 nanoseconds per run. 5.62 % GC time.'

If we decide to keep #new as it is, then I'm not against using a similar 
optimization scheme in #new:. But there should be some tests to verify 
that the methods always return valid dictionaries.
And, I'd also prefer to swap the branches in #new: so that < 4 is the 
first check and < 3 is the second. There should be a comment as well 
about the optimization.


Levente

> 
> Best,
>   Chris
> 
> 
>  
>        
> 
> 
>