On Sat, Jan 12, 2019 at 01:56:23PM +0100, Levente Uzonyi wrote:
> On Fri, 11 Jan 2019, David T. Lewis wrote:
> 
> >On Sat, Jan 12, 2019 at 02:45:17AM +0100, Levente Uzonyi wrote:
> >>On Fri, 11 Jan 2019, David T. Lewis wrote:
> >>
> >>>On Sat, Jan 12, 2019 at 12:32:51AM +0100, Levente Uzonyi wrote:
> >>>>Since this thread is about optimization, I just had to chime in.
> >>>>On 64-bit Spur, I measured ~12.5x speedup for #getSeconds which is 
> >>>>impressive. It can increased further by returning the value of the 
> >>first >>expression.
> >>>>Also, I think the calculation is overcomplicated and the whole method 
> >>>>seems to be equivalent to:
> >>>>
> >>>>	^utcMicroseconds // 1000000 + localOffsetSeconds \\ 86400
> >>>
> >>>The implementation is simpler, but it seems to be a bit slower when I
> >>>measure it (DateAndTime>>asSeconds changed versus Chronology-Core-ul.21
> >>>on 64-bit Spur image).
> >>
> >>I guess you meant #getSeconds not #asSeconds.
> >>
> >>>
> >>>I tested with this:
> >>>
> >>> dt := DateAndTime now.
> >>> Time millisecondsToRun: [ 100000000 timesRepeat: [dt getSeconds ]].
> >>
> >>That is really interesting. Do you have numbers?
> >>
> >>Here's my benchmark:
> >>
> >>| d |
> >>d := DateAndTime now.
> >>(1 to: 5) collect: [ :run |
> >>	[ 1 to: 100000000 do: [ :i | d getSeconds ] ] timeToRun ]
> >>"Original ==> #(3449 3416 3430 3430 3435)"
> >>"Simplified ==> #(2538 2537 2566 2522 2558)"
> >>
> >>I thought that perhaps the negative offset can affect the results, but 
> >>no, it's the same with -7 * 3600 as offset.
> >
> >I was wrong, your simplified version *is* faster.
> >
> >Here is what I get:
> >
> > "Original:"
> >
> > Time millisecondsToRun: [ 100000000 timesRepeat: [dt getSeconds ]].
> >   "==> 4207"
> >
> > | d |
> > d := DateAndTime now.
> > (1 to: 5) collect: [ :run |
> >         [ 1 to: 100000000 do: [ :i | d getSeconds ] ] timeToRun ]
> >   "==> #(3706 3726 3725 3713 3689)"
> >
> >"Simplified:"
> >
> > Time millisecondsToRun: [ 100000000 timesRepeat: [dt getSeconds ]].
> >   "==> 3623"
> >
> > | d |
> > d := DateAndTime now.
> > (1 to: 5) collect: [ :run |
> >         [ 1 to: 100000000 do: [ :i | d getSeconds ] ] timeToRun ]
> >   "==> #(3165 3173 3174 3173 3222)"
> >
> >So your simplified version is both simpler and faster.
> >
> >Please commit it to trunk :-)
> 
> Great! I'll wait for Eliot's response, so that all the changes can be 
> pushed in one commit.
>

I also added a test (Chronology-Tests-dtl.15 in the inbox) that illustrates
the hazards of caching the getSeconds value.

Dave