<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le lun. 17 févr. 2020 à 19:14, Thiede, Christoph <<a href="mailto:Christoph.Thiede@student.hpi.uni-potsdam.de">Christoph.Thiede@student.hpi.uni-potsdam.de</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<div dir="ltr">
<div id="gmail-m_-1406148307137257671x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt;color:rgb(0,0,0);font-family:Calibri,Helvetica,sans-serif">
<p>> <span style="font-size:12pt">The current Spur VM makes x >= 1 about 1.5x faster than x >= y, because it </span><span style="font-size:12pt">has more type information available.</span></p>
<div><br>
</div>
<div><span style="font-size:12pt">Wow, this is interesting.</span><br>
</div>
<div><span style="font-size:12pt">However, I found out it is even faster to use #to:do: (4.2 ms vs. 5.19 ms). I will send this soon to the inbox.</span><br>
</div>
<div><br>
</div>
<div><span style="font-size:12pt">Just one other question before: I just discovered that we have #detect:ifNone: and also #find:ifNone: and #findBinary:ifNone:. #ifAbsent: rather appears to go with #at:, i. e. lookup operations, whereas #ifNone: goes with search
operations. Are you still sure that we shouldn't name the thing #findFirst:ifNone:?</span></div>
<div><span style="font-size:12pt"><br></span></div></div></div></div></blockquote><div>I think that you are right</div><div><br></div><div>indexOf: anElement ifAbsent: ... is like at: aKey ifAbsent: ... (anElement is absent).<br></div><div><br></div><div>But when we provide a predicate, we might prefer ifNone: (none matches the predicate)</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div dir="ltr"><div id="gmail-m_-1406148307137257671x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt;color:rgb(0,0,0);font-family:Calibri,Helvetica,sans-serif"><div><span style="font-size:12pt">
</span></div>
<div><span style="font-size:12pt">Best,</span></div>
<div><span style="font-size:12pt">Christoph</span></div>
<p></p>
<div id="gmail-m_-1406148307137257671x_Signature">
<div id="gmail-m_-1406148307137257671x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt;color:rgb(0,0,0);font-family:Calibri,Helvetica,sans-serif,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
<div name="x_divtagdefaultwrapper">
<div><font size="2" color="#808080"></font></div>
</div>
</div>
</div>
</div>
<hr style="display:inline-block;width:98%">
<div id="gmail-m_-1406148307137257671x_divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>Von:</b> Squeak-dev <<a href="mailto:squeak-dev-bounces@lists.squeakfoundation.org" target="_blank">squeak-dev-bounces@lists.squeakfoundation.org</a>> im Auftrag von Levente Uzonyi <<a href="mailto:leves@caesar.elte.hu" target="_blank">leves@caesar.elte.hu</a>><br>
<b>Gesendet:</b> Montag, 17. Februar 2020 15:10:10<br>
<b>An:</b> The general-purpose Squeak developers list<br>
<b>Betreff:</b> Re: [squeak-dev] The Inbox: Collections-ct.875.mcz</font>
<div> </div>
</div>
</div>
<font size="2"><span style="font-size:10pt">
<div>Hi Christoph,<br>
<br>
On Mon, 17 Feb 2020, Thiede, Christoph wrote:<br>
<br>
> <br>
> Hi Levente,<br>
> <br>
> <br>
> alright, I'll try to measure and optimize them again.<br>
> <br>
> <br>
> > Two things I saw and you didn't mention are block creation<br>
> <br>
> <br>
> You mean that "ifNone: [0]"? One could replace it with "ifNone: 0" to avoid duplication.<br>
<br>
That's a possible way, but there's an even better one: put the actual <br>
implementation into #findFirst:startingAt:, and let <br>
#findFirst:startingAt:ifAbsent: use it. Just like how it's done in the <br>
#indexOf:* methods.<br>
<br>
> <br>
> > and introduction of non-jittable comparison.<br>
> <br>
> What are you referring to?<br>
<br>
The current Spur VM makes x >= 1 about 1.5x faster than x >= y, because it <br>
has more type information available.<br>
<br>
Another possible optimization in #findFirst: is to store #size in a <br>
temporary instead of accessing it in each iteration.<br>
<br>
Levente<br>
<br>
> <br>
> Best,<br>
> Christoph<br>
> <br>
> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________<br>
> Von: Squeak-dev <<a href="mailto:squeak-dev-bounces@lists.squeakfoundation.org" target="_blank">squeak-dev-bounces@lists.squeakfoundation.org</a>> im Auftrag von Levente Uzonyi <<a href="mailto:leves@caesar.elte.hu" target="_blank">leves@caesar.elte.hu</a>><br>
> Gesendet: Montag, 17. Februar 2020 01:00:11<br>
> An: The general-purpose Squeak developers list<br>
> Betreff: Re: [squeak-dev] The Inbox: Collections-ct.875.mcz <br>
> Hi Christoph,<br>
> <br>
> On Sun, 16 Feb 2020, Thiede, Christoph wrote:<br>
> <br>
> ><br>
> > Hi Levente,<br>
> ><br>
> ><br>
> > thanks for the suggestions.<br>
> ><br>
> ><br>
> > > And one more thing: These method resemble #indexOf:* methods, so I think #ifAbsent: is a better choice than #ifNone:.<br>
> ><br>
> ><br>
> > Agreed :-)<br>
> ><br>
> > > I suggest keeping the all method comments<br>
> ><br>
> > Personally, I see little value in documenting a one-liner. Documentation is useful for explaining and optimized method that does not provide the typical Smalltalk readability. Plus, they all need to be maintained ...<br>
> <br>
> If one sees #findLast: being sent somewhere, and one doesn't know what it<br>
> does, with your changes, it means looking up the implementors three times<br>
> to get to the actual implementation and comment.<br>
> <br>
> ><br>
> > > measuring the performance impact of the rewrite.<br>
> ><br>
> > What is the critical point here?<br>
> > Of course, the stack will be a little higher, but the asymptotic cost does not change. We only evaluate anotherBlock at the end instead of returning 0.<br>
> <br>
> Two things I saw and you didn't mention are block creation and<br>
> introduction of non-jittable comparison.<br>
> <br>
> ><br>
> > Please do not misunderstand me: I honor optimizing critical Smalltalk methods, but I always strive to avoid premature optimization.<br>
> <br>
> In my opinion, generic library methods like these should be optimized by<br>
> definition.<br>
> <br>
> Calling premature optimization means that you assume that if these methods<br>
> were written in a more optimal way, then their complexity and/or<br>
> legibility would be worse.<br>
> Have a look at #indexOf:*, and you'll see that's not the case.<br>
> <br>
> > Should we maybe establish to practice to store such benchmarks somewhere in the trunk so that not every willing contributor needs to write them again?<br>
> <br>
> I think methods like these don't change often enough (presumably they<br>
> never change) to justify storing benchmarks for them.<br>
> <br>
> <br>
> Levente<br>
> <br>
> ><br>
> > Best,<br>
> > Christoph<br>
> ><br>
> >________________________________________________________________________________________________________________________________________________________________________________________________________________________________<br>
> _<br>
> > Von: Squeak-dev <<a href="mailto:squeak-dev-bounces@lists.squeakfoundation.org" target="_blank">squeak-dev-bounces@lists.squeakfoundation.org</a>> im Auftrag von Levente Uzonyi <<a href="mailto:leves@caesar.elte.hu" target="_blank">leves@caesar.elte.hu</a>><br>
> > Gesendet: Sonntag, 16. Februar 2020 22:47:10<br>
> > An: The general-purpose Squeak developers list<br>
> > Betreff: Re: [squeak-dev] The Inbox: Collections-ct.875.mcz <br>
> > And one more thing: These method resemble #indexOf:* methods, so I think<br>
> > #ifAbsent: is a better choice than #ifNone:.<br>
> ><br>
> > Levente<br>
> ><br>
> > On Sun, 16 Feb 2020, Levente Uzonyi wrote:<br>
> ><br>
> > > Hi Christoph,<br>
> > ><br>
> > > I suggest keeping the all method comments, and measuring the performance<br>
> > > impact of the rewrite.<br>
> > ><br>
> > > Levente<br>
> > ><br>
> > > On Sun, 16 Feb 2020, <a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a> wrote:<br>
> > ><br>
> > >> A new version of Collections was added to project The Inbox:<br>
> > >> <a href="http://source.squeak.org/inbox/Collections-ct.875.mcz" target="_blank">http://source.squeak.org/inbox/Collections-ct.875.mcz</a><br>
> > >><br>
> > >> ==================== Summary ====================<br>
> > >><br>
> > >> Name: Collections-ct.875<br>
> > >> Author: ct<br>
> > >> Time: 16 February 2020, 3:32:23.116 pm<br>
> > >> UUID: 61ea773b-1408-c040-a712-09238ee6fe2f<br>
> > >> Ancestors: Collections-topa.873<br>
> > >><br>
> > >> Extends and realigns version of #findFirst: and #findLast:.<br>
> > >><br>
> > >> =============== Diff against Collections-topa.873 ===============<br>
> > >><br>
> > >> Item was changed:<br>
> > >> ----- Method: SequenceableCollection>>findFirst: (in category<br>
> > > 'enumerating') -----<br>
> > >> findFirst: aBlock<br>
> > >> - "Return the index of my first element for which aBlock evaluates as<br>
> > > true."<br>
> > >><br>
> > >> + ^ self findFirst: aBlock startingAt: 1!<br>
> > >> - | index |<br>
> > >> - index := 0.<br>
> > >> - [(index := index + 1) <= self size] whileTrue:<br>
> > >> - [(aBlock value: (self at: index)) ifTrue: [^index]].<br>
> > >> - ^ 0!<br>
> > >><br>
> > >> Item was added:<br>
> > >> + ----- Method: SequenceableCollection>>findFirst:ifNone: (in category<br>
> > > 'enumerating') -----<br>
> > >> + findFirst: aBlock ifNone: anotherBlock<br>
> > >> +<br>
> > >> + ^ self findFirst: aBlock startingAt: 1 ifNone: anotherBlock!<br>
> > >><br>
> > >> Item was added:<br>
> > >> + ----- Method: SequenceableCollection>>findFirst:startingAt: (in category<br>
> > > 'enumerating') -----<br>
> > >> + findFirst: aBlock startingAt: minIndex<br>
> > >> +<br>
> > >> + ^ self findFirst: aBlock startingAt: minIndex ifNone: [0]!<br>
> > >><br>
> > >> Item was added:<br>
> > >> + ----- Method: SequenceableCollection>>findFirst:startingAt:ifNone: (in<br>
> > > category 'enumerating') -----<br>
> > >> + findFirst: aBlock startingAt: minIndex ifNone: anotherBlock<br>
> > >> + "Return the index of my first element with index >= minIndex for<br>
> > > which aBlock evaluates as true. If no element is found, return the value of<br>
> > > anotherBlock."<br>
> > >> +<br>
> > >> + | index |<br>
> > >> + index := minIndex - 1.<br>
> > >> + [(index := index + 1) <= self size] whileTrue:<br>
> > >> + [(aBlock value: (self at: index)) ifTrue: [^index]].<br>
> > >> + ^ anotherBlock value!<br>
> > >><br>
> > >> Item was changed:<br>
> > >> ----- Method: SequenceableCollection>>findLast: (in category<br>
> > > 'enumerating') -----<br>
> > >> findLast: aBlock<br>
> > >> - "Return the index of my last element for which aBlock evaluates as<br>
> > > true."<br>
> > >><br>
> > >> + ^ self findLast: aBlock startingAt: 1!<br>
> > >> - | index |<br>
> > >> - index := self size + 1.<br>
> > >> - [(index := index - 1) >= 1] whileTrue:<br>
> > >> - [(aBlock value: (self at: index)) ifTrue: [^index]].<br>
> > >> - ^ 0!<br>
> > >><br>
> > >> Item was added:<br>
> > >> + ----- Method: SequenceableCollection>>findLast:ifNone: (in category<br>
> > > 'enumerating') -----<br>
> > >> + findLast: aBlock ifNone: anotherBlock<br>
> > >> +<br>
> > >> + ^ self findLast: aBlock startingAt: 1 ifNone: anotherBlock!<br>
> > >><br>
> > >> Item was changed:<br>
> > >> ----- Method: SequenceableCollection>>findLast:startingAt: (in category<br>
> > > 'enumerating') -----<br>
> > >> + findLast: aBlock startingAt: minIndex<br>
> > >> - findLast: aBlock startingAt: i<br>
> > >> - "Return the index of my last element with index >= i for which aBlock<br>
> > > evaluates as true."<br>
> > >><br>
> > >> + ^ self findLast: aBlock startingAt: minIndex ifNone: [0]!<br>
> > >> - | index |<br>
> > >> - index := self size + 1.<br>
> > >> - [(index := index - 1) >= i] whileTrue:<br>
> > >> - [(aBlock value: (self at: index)) ifTrue: [^index]].<br>
> > >> - ^ 0!<br>
> > >><br>
> > >> Item was added:<br>
> > >> + ----- Method: SequenceableCollection>>findLast:startingAt:ifNone: (in<br>
> > > category 'enumerating') -----<br>
> > >> + findLast: aBlock startingAt: minIndex ifNone: anotherBlock<br>
> > >> + "Return the index of my last element with index >= minIndex for which<br>
> > > aBlock evaluates as true. If no element is found, return the value of<br>
> > > anotherBlock."<br>
> > >> +<br>
> > >> + | index |<br>
> > >> + index := self size + 1.<br>
> > >> + [(index := index - 1) >= minIndex] whileTrue:<br>
> > >> + [(aBlock value: (self at: index)) ifTrue: [^index]].<br>
> > >> + ^ anotherBlock value!<br>
> > ><br>
> > ><br>
> ><br>
> ><br>
> ><br>
> <br>
></div>
</span></font>
</div>
<br>
</blockquote></div></div>