[squeak-dev] The Inbox: Collections-ct.875.mcz

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Mon Feb 17 18:56:02 UTC 2020 

Le lun. 17 févr. 2020 à 19:14, Thiede, Christoph <
Christoph.Thiede at student.hpi.uni-potsdam.de> a écrit :

> > The current Spur VM makes x >= 1 about 1.5x faster than x >= y, because
> it has more type information available.
>
> Wow, this is interesting.
> 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.
>
> 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:?
>
> I think that you are right

indexOf: anElement ifAbsent: ... is like at: aKey ifAbsent: ... (anElement
is absent).

But when we provide a predicate, we might prefer ifNone: (none matches the
predicate)

Best,
> Christoph
>
> ------------------------------
> *Von:* Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im
> Auftrag von Levente Uzonyi <leves at caesar.elte.hu>
> *Gesendet:* Montag, 17. Februar 2020 15:10:10
> *An:* The general-purpose Squeak developers list
> *Betreff:* Re: [squeak-dev] The Inbox: Collections-ct.875.mcz
>
> Hi Christoph,
>
> On Mon, 17 Feb 2020, Thiede, Christoph wrote:
>
> >
> > Hi Levente,
> >
> >
> > alright, I'll try to measure and optimize them again.
> >
> >
> > > Two things I saw and you didn't mention are block creation
> >
> >
> > You mean that "ifNone: [0]"? One could replace it with "ifNone: 0" to
> avoid duplication.
>
> That's a possible way, but there's an even better one: put the actual
> implementation into #findFirst:startingAt:, and let
> #findFirst:startingAt:ifAbsent: use it. Just like how it's done in the
> #indexOf:* methods.
>
> >
> > > and introduction of non-jittable comparison.
> >
> > What are you referring to?
>
> The current Spur VM makes x >= 1 about 1.5x faster than x >= y, because it
> has more type information available.
>
> Another possible optimization in #findFirst: is to store #size in a
> temporary instead of accessing it in each iteration.
>
> Levente
>
> >
> > Best,
> > Christoph
> >
> >
> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> > Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im
> Auftrag von Levente Uzonyi <leves at caesar.elte.hu>
> > Gesendet: Montag, 17. Februar 2020 01:00:11
> > An: The general-purpose Squeak developers list
> > Betreff: Re: [squeak-dev] The Inbox: Collections-ct.875.mcz
> > Hi Christoph,
> >
> > On Sun, 16 Feb 2020, Thiede, Christoph wrote:
> >
> > >
> > > Hi Levente,
> > >
> > >
> > > thanks for the suggestions.
> > >
> > >
> > > > And one more thing: These method resemble #indexOf:* methods, so I
> think #ifAbsent: is a better choice than #ifNone:.
> > >
> > >
> > > Agreed :-)
> > >
> > > > I suggest keeping the all method comments
> > >
> > > 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 ...
> >
> > If one sees #findLast: being sent somewhere, and one doesn't know what it
> > does, with your changes, it means looking up the implementors three times
> > to get to the actual implementation and comment.
> >
> > >
> > > > measuring the performance impact of the rewrite.
> > >
> > > What is the critical point here?
> > > 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.
> >
> > Two things I saw and you didn't mention are block creation and
> > introduction of non-jittable comparison.
> >
> > >
> > > Please do not misunderstand me: I honor optimizing critical Smalltalk
> methods, but I always strive to avoid premature optimization.
> >
> > In my opinion, generic library methods like these should be optimized by
> > definition.
> >
> > Calling premature optimization means that you assume that if these
> methods
> > were written in a more optimal way, then their complexity and/or
> > legibility would be worse.
> > Have a look at #indexOf:*, and you'll see that's not the case.
> >
> > > 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?
> >
> > I think methods like these don't change often enough (presumably they
> > never change) to justify storing benchmarks for them.
> >
> >
> > Levente
> >
> > >
> > > Best,
> > > Christoph
> > >
> >
> >________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> > _
> > > Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im
> Auftrag von Levente Uzonyi <leves at caesar.elte.hu>
> > > Gesendet: Sonntag, 16. Februar 2020 22:47:10
> > > An: The general-purpose Squeak developers list
> > > Betreff: Re: [squeak-dev] The Inbox: Collections-ct.875.mcz
> > > And one more thing: These method resemble #indexOf:* methods, so I
> think
> > > #ifAbsent: is a better choice than #ifNone:.
> > >
> > > Levente
> > >
> > > On Sun, 16 Feb 2020, Levente Uzonyi wrote:
> > >
> > > > Hi Christoph,
> > > >
> > > > I suggest keeping the all method comments, and measuring the
> performance
> > > > impact of the rewrite.
> > > >
> > > > Levente
> > > >
> > > > On Sun, 16 Feb 2020, commits at source.squeak.org wrote:
> > > >
> > > >> A new version of Collections was added to project The Inbox:
> > > >> http://source.squeak.org/inbox/Collections-ct.875.mcz
> > > >>
> > > >> ==================== Summary ====================
> > > >>
> > > >> Name: Collections-ct.875
> > > >> Author: ct
> > > >> Time: 16 February 2020, 3:32:23.116 pm
> > > >> UUID: 61ea773b-1408-c040-a712-09238ee6fe2f
> > > >> Ancestors: Collections-topa.873
> > > >>
> > > >> Extends and realigns version of #findFirst: and #findLast:.
> > > >>
> > > >> =============== Diff against Collections-topa.873 ===============
> > > >>
> > > >> Item was changed:
> > > >>  ----- Method: SequenceableCollection>>findFirst: (in category
> > > > 'enumerating') -----
> > > >>  findFirst: aBlock
> > > >> -     "Return the index of my first element for which aBlock
> evaluates as
> > > > true."
> > > >>
> > > >> +     ^ self findFirst: aBlock startingAt: 1!
> > > >> -     | index |
> > > >> -     index := 0.
> > > >> -     [(index := index + 1) <= self size] whileTrue:
> > > >> -             [(aBlock value: (self at: index)) ifTrue: [^index]].
> > > >> -     ^ 0!
> > > >>
> > > >> Item was added:
> > > >> + ----- Method: SequenceableCollection>>findFirst:ifNone: (in
> category
> > > > 'enumerating') -----
> > > >> + findFirst: aBlock ifNone: anotherBlock
> > > >> +
> > > >> +     ^ self findFirst: aBlock startingAt: 1 ifNone: anotherBlock!
> > > >>
> > > >> Item was added:
> > > >> + ----- Method: SequenceableCollection>>findFirst:startingAt: (in
> category
> > > > 'enumerating') -----
> > > >> + findFirst: aBlock startingAt: minIndex
> > > >> +
> > > >> +     ^ self findFirst: aBlock startingAt: minIndex ifNone: [0]!
> > > >>
> > > >> Item was added:
> > > >> + ----- Method:
> SequenceableCollection>>findFirst:startingAt:ifNone: (in
> > > > category 'enumerating') -----
> > > >> + findFirst: aBlock startingAt: minIndex ifNone: anotherBlock
> > > >> +     "Return the index of my first element with index >= minIndex
> for
> > > > which aBlock evaluates as true. If no element is found, return the
> value of
> > > > anotherBlock."
> > > >> +
> > > >> +     | index |
> > > >> +     index := minIndex - 1.
> > > >> +     [(index := index + 1) <= self size] whileTrue:
> > > >> +             [(aBlock value: (self at: index)) ifTrue: [^index]].
> > > >> +     ^ anotherBlock value!
> > > >>
> > > >> Item was changed:
> > > >>  ----- Method: SequenceableCollection>>findLast: (in category
> > > > 'enumerating') -----
> > > >>  findLast: aBlock
> > > >> -     "Return the index of my last element for which aBlock
> evaluates as
> > > > true."
> > > >>
> > > >> +     ^ self findLast: aBlock startingAt: 1!
> > > >> -     | index |
> > > >> -     index := self size + 1.
> > > >> -     [(index := index - 1) >= 1] whileTrue:
> > > >> -             [(aBlock value: (self at: index)) ifTrue: [^index]].
> > > >> -     ^ 0!
> > > >>
> > > >> Item was added:
> > > >> + ----- Method: SequenceableCollection>>findLast:ifNone: (in
> category
> > > > 'enumerating') -----
> > > >> + findLast: aBlock ifNone: anotherBlock
> > > >> +
> > > >> +     ^ self findLast: aBlock startingAt: 1 ifNone: anotherBlock!
> > > >>
> > > >> Item was changed:
> > > >>  ----- Method: SequenceableCollection>>findLast:startingAt: (in
> category
> > > > 'enumerating') -----
> > > >> + findLast: aBlock startingAt: minIndex
> > > >> - findLast: aBlock startingAt: i
> > > >> -     "Return the index of my last element with index >= i for
> which aBlock
> > > > evaluates as true."
> > > >>
> > > >> +     ^ self findLast: aBlock startingAt: minIndex ifNone: [0]!
> > > >> -     | index |
> > > >> -     index := self size + 1.
> > > >> -     [(index := index - 1) >= i] whileTrue:
> > > >> -             [(aBlock value: (self at: index)) ifTrue: [^index]].
> > > >> -     ^ 0!
> > > >>
> > > >> Item was added:
> > > >> + ----- Method: SequenceableCollection>>findLast:startingAt:ifNone:
> (in
> > > > category 'enumerating') -----
> > > >> + findLast: aBlock startingAt: minIndex ifNone: anotherBlock
> > > >> +     "Return the index of my last element with index >= minIndex
> for which
> > > > aBlock evaluates as true.  If no element is found, return the value
> of
> > > > anotherBlock."
> > > >> +
> > > >> +     | index |
> > > >> +     index := self size + 1.
> > > >> +     [(index := index - 1) >= minIndex] whileTrue:
> > > >> +             [(aBlock value: (self at: index)) ifTrue: [^index]].
> > > >> +     ^ anotherBlock value!
> > > >
> > > >
> > >
> > >
> > >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20200217/2c79aa2d/attachment-0001.html>

More information about the Squeak-dev mailing list