<div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">> > > 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.<br>
> > ><br>
> > > {<br>
> > > '[ Dictionary new ]'->'100% of baseline rate, 27,600,000 per second. 36.2 nanoseconds per run. 11.33547 % GC time.'<br>
> > ><br>
> > > "performance pain?"<br>
> > > '[ Dictionary new: 1 ]'->'60% of baseline rate, 16,600,000 per second. 60.1 nanoseconds per run. 5.61888 % GC time.'<br>
> > > '[ Dictionary new: 2 ]'->'61% of baseline rate, 16,900,000 per second. 59.2 nanoseconds per run. 5.67886 % GC time.'<br>
> > > '[ Dictionary new: 3 ]'->'59% of baseline rate, 16,300,000 per second. 61.5 nanoseconds per run. 6.77864 % GC time.'<br>
> ><br>
> > Even if there's a performance overhead, you use less memory.<br>
> ><br>
> ><br>
> > But #new: is about optimization along *both* of those dimensions. Imagine how complicated a "manpage" for #new: would have to be if it weren't. #new: must _never_ perform significantly worse than #new (for sizes <= the default), because it would either trick or force developers into writing<br>
> less-performant<br>
> > code, or into acknowledging Squeak's internal Dictionary implementation in their own code. It feels like an API-design bug.<br>
><br>
> It was okay for quite a long time. :)<br>
> <br>
> <br>
> ... until this proposal which changed the smallest internal array size from 5 to 3 and introduced the above anomaly. I assume you made that change because you felt something else wasn't okay (to which I agree!).<br>
<br>
Capacity of 3 has been available through #compact, but #sizeFor: still <br>
doesn't return it due to a bug. </blockquote><div><br></div><div>That bug has been saving us from the anomaly now exposed by your improvements.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">> > > "into #sizeFor:"<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> ><br>
> > > '[ Dictionary new: 4 ]'->'57% of baseline rate, 15,800,000 per second. 63.5 nanoseconds per run. 7.87685 % GC time.'<br>
> ><br>
> > Starting from 4, you also save time by avoiding growing, which is<br>
> > more significant than what you "lose" during instance creation.<br>
> ><br>
> ><br>
> > Except my Dictionary is never going to grow.<br>
><br>
> So, we actually say the same thing.<br>
> <br>
> <br>
> Did we? You were arguing about saving time with growing, which I can never gain. I only lose during instance creation...<br>
<br>
We did. You just quoted me: "save time by _avoiding_ growing".<br></blockquote><div><br></div><div>Oh, okay, but that's not relevant. Our whole discussion is about fixing when the Request needs to make Dictionary's with 1 or 2 elements (in Collectoins-ul.871) and 3 (in trunk), not 4.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > > In case it helps bring clarity, my scenario is the GraphQL server. As a Request comes in, the server will know, depending on the type, how many named arguments to expect (for most "normal" schema's, 0 to 4, but it can define any number it wants). So it creates right sized Dictionary to hold them<br>
> all, and will<br>
> > never grow beyond that. I simply don't want the server to have to do extra work when **vast majority** of requests will have fewer than 4 arguments.<br>
> ><br>
> > We could get rid of the anomaly by changing #new to ^self new: 3.<br>
> ><br>
> ><br>
> > Yes, I'd be fine with that solution, too! For me, it's only about violation of #new:'s contract.<br>
><br>
> I don't see any broken contract here. It may be surprising to see<br>
> somewhat worse performance with #new: than with #new (25 nanoseconds per<br>
> instance according to your measurements),<br>
> <br>
> <br>
> a 40% hit...<br>
<br>
Only if you keep that Dictionary empty. </blockquote><div><br></div><div>No, I'm storing one element in (Dictionary new: 1) and two in (Dictionary new: 2). But those are runtime sizes, those Dictionary's are created from a variable in the code. I simply write #new: in the code because I can know the size up front, so its (<i>supposed to be!</i>) faster to pre-allocate to the correct size via #new:. But for the millions with 1 or 2 arguments, it'd have been better to ignore #new: "optimization" there, for simply #new. That's crazy.</div><div><br></div><div>I'd like to see a manpage-quality comment explaining this in HashedCollection class>>#new:. Either that, or we need your fixed version of my fix, please. :)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">And if you decide to do so, then <br>
why create it at all? :)<br></blockquote><div><br></div><div>Right, I don't. 0 arguments creates no Dictionary.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> <br>
> but both of those methods do<br>
> what they should. Just because there was an easy optimization applied to<br>
> #new, nothing was broken IMHO.<br>
> <br>
> <br>
> ... for using #new: that's supposed to be for optimization!<br></blockquote><div><br></div><div> ^^^ this is kind of the key point, what about this?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> <br>
> So if you were in my shoes (which, if this goes to trunk as-is eventually you will be!), would you take a 40% hit in performance-critical code needing to instantiate a Dictionary with:<br>
> <br>
> Dictionary new: runtimeSize<br>
> <br>
> Or, actually violate encapsulation to preserve performance?<br>
> <br>
> sz>3 ifTrue: [Dictionary new: sz] ifFalse: [Dictionary new]<br>
<br>
I don't see the broken encapsulation there.</blockquote><div><br></div><div></div><div>Here:<br></div><div><br></div><div> -------> sz>3 <---- ifTrue: [Dictionary new: sz] ifFalse: [Dictionary new]<br></div><div><br></div><div>you would introduce a dependency on the private, internal initial size employed by Dictionary (e.g., breaking its encapsulation). I want to keep the GraphQL framework as dialect-independent as possible, I can't bring myself to write that, and I hope you wouldn't either.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> That's just black box <br>
optimization based on measurements as long as you don't look under the <br>
hood, isn't it?<br></blockquote><div><br></div><div>Only for as long as what's under Dictionary's hood doesn't change and quietly break you. It could end up <i>just a bit slower</i> enough to cost you, but reamain under your radar. The worse code that was once as an optimization will have become insidious.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
> ?<br>
> <br>
> My past observations have been that you like and appreciate the most-efficient performing code, so I'm truly curious!<br>
<br>
I like optimizations where the cost can be considered smaller than the <br>
benefits.<br>
For example, in TextDiffBuilder, I even sacrificed legibility because the <br>
extra complexity is local to a single method: #lcsFor:and:, and the <br>
benefits are measurable. The previous implementations are available in <br>
the Versions Browser, which can help understanding how and why the code <br>
evolved into its current form.<br>
There are plenty of cases in the Collections hierarchy where methods could <br>
be overridden in various subclasses to gain a few percents, but the <br>
changes would be extensive and the code would become very hard to <br>
maintain.<br></blockquote><div><br></div><div>The issue is about the quality of Squeak's API as it relates to user-expectations (#new vs #new:), not gaining a few percents.</div><div><br></div><div>Best,</div><div> Chris</div></div></div>