[squeak-dev] The Inbox: Collections-ct.875.mczThiede, Christoph Christoph.Thiede at student.hpi.uni-potsdam.deMon Feb 17 18:14:52 UTC 2020
> 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:? 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/4b5adb8c/attachment.html>
More information about the Squeak-dev mailing list |