The Inbox: Chronology-Core-eem.22.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

The Inbox: Chronology-Core-eem.22.mcz

commits-2
A new version of Chronology-Core was added to project The Inbox:
http://source.squeak.org/inbox/Chronology-Core-eem.22.mcz

==================== Summary ====================

Name: Chronology-Core-eem.22
Author: eem
Time: 9 January 2019, 6:53:52.265064 pm
UUID: 348c4ad7-0a27-4021-8788-9e3ec220df24
Ancestors: Chronology-Core-ul.21

Simple speedup by caching the result of getSeconds in an inst var.  Lots of other values are defined in terms of getSeconds so this simply eliminates duplicating the large integer arithmetic in getSeconds.

=============== Diff against Chronology-Core-ul.21 ===============

Item was changed:
  Magnitude subclass: #DateAndTime
+ instanceVariableNames: 'utcMicroseconds localOffsetSeconds seconds'
- instanceVariableNames: 'utcMicroseconds localOffsetSeconds'
  classVariableNames: 'AutomaticTimezone ClockProvider InitializeFromPrimitive LocalTimeZone PosixEpochJulianDays'
  poolDictionaries: 'ChronologyConstants'
  category: 'Chronology-Core'!
 
+ !DateAndTime commentStamp: 'eem 1/9/2019 18:44' prior: 0!
- !DateAndTime commentStamp: 'dtl 3/12/2016 10:32' prior: 0!
  I represent a point in UTC time as defined by ISO 8601. I have zero duration.
 
+ My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.  I cache the result of getSeconds in the seconds variable that is computed when required.
- My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.
 
  The implementation ignores leap seconds, which are adjustments made to maintain earth rotational clock time in synchronization with elapsed seconds.
 
  DateAndTime class>>now will use #primitiveUtcWithOffset to obtain current time in UTC microseconds with current local offset in seconds. The primitive provides an atomic query for UTC time and local offset as measured by the OS platform.  If primitiveUtcWithOffset is not available, the traditional implementation is used, which relies on a primitive for microseconds in the local time zone and derives UTC based on the TimeZone setting.
  !

Item was changed:
  ----- Method: DateAndTime>>getSeconds (in category 'accessing') -----
  getSeconds
-
  | posixDays posixSeconds localSeconds |
+ seconds ifNil:
+ [posixSeconds := utcMicroseconds // 1000000.
+ localSeconds := posixSeconds + localOffsetSeconds.
+ localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
+ posixDays := localSeconds // SecondsInDay.
+ seconds := localSeconds - (posixDays * SecondsInDay)].
+ ^seconds!
- posixSeconds := utcMicroseconds // 1000000.
- localSeconds := posixSeconds + localOffsetSeconds.
- localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
- posixDays := localSeconds // SecondsInDay.
- ^localSeconds - (posixDays * SecondsInDay).
- !

Item was changed:
  ----- Method: DateAndTime>>readDataFrom:size: (in category 'objects from disk') -----
  readDataFrom: aDataStream size: varsOnDisk
  "Fill in the fields of self based on the contents of aDataStream. The serialized
  data will have four instance variables, because all instances are serialized in a
  cononical format as if having originating from an instance with the traditional
  seconds/offset/jdn/nanos instance variables."
   
  | seconds offset jdn nanos |
  seconds := aDataStream next.
  offset := aDataStream next.
  jdn := aDataStream next.
  nanos := aDataStream next.
  localOffsetSeconds := offset ifNil: [ 0 ] ifNotNil: [ :off | off asSeconds ].
  utcMicroseconds := self
  microsecondsFromDay: jdn
  seconds: seconds
  nanos: nanos
+ offset: localOffsetSeconds..
+ seconds := nil!
- offset: localOffsetSeconds.!

Item was changed:
  ----- Method: DateAndTime>>setJdn:seconds:nano:localOffsetSeconds: (in category 'private') -----
  setJdn: jdn seconds: s nano: n localOffsetSeconds: offset
 
  localOffsetSeconds := offset.
  utcMicroseconds := self
  microsecondsFromDay: jdn
  seconds: s
  nanos: n
+ offset: offset.
+ seconds := nil!
- offset: offset!

Item was changed:
  ----- Method: DateAndTime>>ticks:offset: (in category 'private') -----
  ticks: ticks offset: utcOffset
  "ticks is {julianDayNumber. secondCount. nanoSeconds}"
 
  | jdn s nanos |
  self normalize: 3 ticks: ticks base: NanosInSecond.
  self normalize: 2 ticks: ticks base: SecondsInDay.
 
  jdn := ticks at: 1.
  s := ticks at: 2.
  nanos := ticks at: 3.
  localOffsetSeconds := utcOffset ifNil: [0] ifNotNil: [utcOffset asSeconds].
  utcMicroseconds := self microsecondsFromDay: jdn seconds: s nanos: nanos offset: localOffsetSeconds.
+ seconds := nil!
- !

Item was changed:
  ----- Method: DateAndTime>>utcMicroseconds: (in category 'initialize-release') -----
  utcMicroseconds: utcValue
  "Allow value to be modified during initialization from a primitive in order to support
  monotonically increasing clock behavior."
+ utcMicroseconds := utcValue.
+ seconds := nil!
- utcMicroseconds := utcValue!

Item was changed:
  ----- Method: DateAndTime>>utcMicroseconds:offset: (in category 'initialize-release') -----
  utcMicroseconds: microsecondsSincePosixEpoch offset: tzOffset
 
  utcMicroseconds := microsecondsSincePosixEpoch.
  localOffsetSeconds := tzOffset.
+ seconds := nil
  !


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

Chris Muller-3
-1.  DateAndTime is an object of which there are often many instances.
Adding another inst var increases size and processing to spin through
yet one more variable during disk-read, transmission and serialization
and materialization.  It's pure cost for 64-bit images, since there
isn't any LI arithmetic there anyway, and *we knew* going in that
UTCDateAndTime would be slower in 32-bit.  Avoiding LI arithmetic in
32-bit is what the original Chronology did....

Regards,
  Chris



On Wed, Jan 9, 2019 at 8:53 PM <[hidden email]> wrote:

>
> A new version of Chronology-Core was added to project The Inbox:
> http://source.squeak.org/inbox/Chronology-Core-eem.22.mcz
>
> ==================== Summary ====================
>
> Name: Chronology-Core-eem.22
> Author: eem
> Time: 9 January 2019, 6:53:52.265064 pm
> UUID: 348c4ad7-0a27-4021-8788-9e3ec220df24
> Ancestors: Chronology-Core-ul.21
>
> Simple speedup by caching the result of getSeconds in an inst var.  Lots of other values are defined in terms of getSeconds so this simply eliminates duplicating the large integer arithmetic in getSeconds.
>
> =============== Diff against Chronology-Core-ul.21 ===============
>
> Item was changed:
>   Magnitude subclass: #DateAndTime
> +       instanceVariableNames: 'utcMicroseconds localOffsetSeconds seconds'
> -       instanceVariableNames: 'utcMicroseconds localOffsetSeconds'
>         classVariableNames: 'AutomaticTimezone ClockProvider InitializeFromPrimitive LocalTimeZone PosixEpochJulianDays'
>         poolDictionaries: 'ChronologyConstants'
>         category: 'Chronology-Core'!
>
> + !DateAndTime commentStamp: 'eem 1/9/2019 18:44' prior: 0!
> - !DateAndTime commentStamp: 'dtl 3/12/2016 10:32' prior: 0!
>   I represent a point in UTC time as defined by ISO 8601. I have zero duration.
>
> + My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.  I cache the result of getSeconds in the seconds variable that is computed when required.
> - My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.
>
>   The implementation ignores leap seconds, which are adjustments made to maintain earth rotational clock time in synchronization with elapsed seconds.
>
>   DateAndTime class>>now will use #primitiveUtcWithOffset to obtain current time in UTC microseconds with current local offset in seconds. The primitive provides an atomic query for UTC time and local offset as measured by the OS platform.  If primitiveUtcWithOffset is not available, the traditional implementation is used, which relies on a primitive for microseconds in the local time zone and derives UTC based on the TimeZone setting.
>   !
>
> Item was changed:
>   ----- Method: DateAndTime>>getSeconds (in category 'accessing') -----
>   getSeconds
> -
>         | posixDays posixSeconds localSeconds |
> +       seconds ifNil:
> +               [posixSeconds := utcMicroseconds // 1000000.
> +                localSeconds := posixSeconds + localOffsetSeconds.
> +                localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
> +                posixDays := localSeconds // SecondsInDay.
> +                seconds := localSeconds - (posixDays * SecondsInDay)].
> +       ^seconds!
> -       posixSeconds := utcMicroseconds // 1000000.
> -       localSeconds := posixSeconds + localOffsetSeconds.
> -       localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
> -       posixDays := localSeconds // SecondsInDay.
> -       ^localSeconds - (posixDays * SecondsInDay).
> - !
>
> Item was changed:
>   ----- Method: DateAndTime>>readDataFrom:size: (in category 'objects from disk') -----
>   readDataFrom: aDataStream size: varsOnDisk
>         "Fill in the fields of self based on the contents of aDataStream. The serialized
>         data will have four instance variables, because all instances are serialized in a
>         cononical format as if having originating from an instance with the traditional
>         seconds/offset/jdn/nanos instance variables."
>
>         | seconds offset jdn nanos |
>         seconds := aDataStream next.
>         offset := aDataStream next.
>         jdn := aDataStream next.
>         nanos := aDataStream next.
>         localOffsetSeconds := offset ifNil: [ 0 ] ifNotNil: [ :off | off asSeconds ].
>         utcMicroseconds := self
>                                 microsecondsFromDay: jdn
>                                 seconds: seconds
>                                 nanos: nanos
> +                               offset: localOffsetSeconds..
> +       seconds := nil!
> -                               offset: localOffsetSeconds.!
>
> Item was changed:
>   ----- Method: DateAndTime>>setJdn:seconds:nano:localOffsetSeconds: (in category 'private') -----
>   setJdn: jdn seconds: s nano: n localOffsetSeconds: offset
>
>         localOffsetSeconds := offset.
>         utcMicroseconds := self
>                                 microsecondsFromDay: jdn
>                                 seconds: s
>                                 nanos: n
> +                               offset: offset.
> +       seconds := nil!
> -                               offset: offset!
>
> Item was changed:
>   ----- Method: DateAndTime>>ticks:offset: (in category 'private') -----
>   ticks: ticks offset: utcOffset
>         "ticks is {julianDayNumber. secondCount. nanoSeconds}"
>
>         | jdn s nanos |
>         self normalize: 3 ticks: ticks base: NanosInSecond.
>         self normalize: 2 ticks: ticks base: SecondsInDay.
>
>         jdn     := ticks at: 1.
>         s := ticks at: 2.
>         nanos := ticks at: 3.
>         localOffsetSeconds := utcOffset ifNil: [0] ifNotNil: [utcOffset asSeconds].
>         utcMicroseconds := self microsecondsFromDay: jdn seconds: s nanos: nanos offset: localOffsetSeconds.
> +       seconds := nil!
> - !
>
> Item was changed:
>   ----- Method: DateAndTime>>utcMicroseconds: (in category 'initialize-release') -----
>   utcMicroseconds: utcValue
>         "Allow value to be modified during initialization from a primitive in order to support
>         monotonically increasing clock behavior."
> +       utcMicroseconds := utcValue.
> +       seconds := nil!
> -       utcMicroseconds := utcValue!
>
> Item was changed:
>   ----- Method: DateAndTime>>utcMicroseconds:offset: (in category 'initialize-release') -----
>   utcMicroseconds: microsecondsSincePosixEpoch offset: tzOffset
>
>         utcMicroseconds := microsecondsSincePosixEpoch.
>         localOffsetSeconds := tzOffset.
> +       seconds := nil
>   !
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

David T. Lewis
I am away and cannot look at this now, but on the face of it, it sounds
like a good optimization, so if it makes a meaningful difference to
performance then we should consider it.

One clarification to Chris' comment - we (or at least I) did not expect
UTCDateAndTime to be slower on 32-bit images. In fact, when I measured
it (quite a long time ago now) it was faster on 32-bit images, and faster
yet on 64-bit images. But my benchmarks were trivial, and Chris is
considering use cases that involve IO to disk, and those might have
very different performance considerations.

The simple performance checks that I was using can be found in
package Tests-UTCDateAndTime in the www.squeaksource.com/UTCDateAndTime
repository. I can't look at it now, but those tests might be a useful
reference here.

Dave

On Wed, Jan 09, 2019 at 10:38:04PM -0600, Chris Muller wrote:

> -1.  DateAndTime is an object of which there are often many instances.
> Adding another inst var increases size and processing to spin through
> yet one more variable during disk-read, transmission and serialization
> and materialization.  It's pure cost for 64-bit images, since there
> isn't any LI arithmetic there anyway, and *we knew* going in that
> UTCDateAndTime would be slower in 32-bit.  Avoiding LI arithmetic in
> 32-bit is what the original Chronology did....
>
> Regards,
>   Chris
>
>
>
> On Wed, Jan 9, 2019 at 8:53 PM <[hidden email]> wrote:
> >
> > A new version of Chronology-Core was added to project The Inbox:
> > http://source.squeak.org/inbox/Chronology-Core-eem.22.mcz
> >
> > ==================== Summary ====================
> >
> > Name: Chronology-Core-eem.22
> > Author: eem
> > Time: 9 January 2019, 6:53:52.265064 pm
> > UUID: 348c4ad7-0a27-4021-8788-9e3ec220df24
> > Ancestors: Chronology-Core-ul.21
> >
> > Simple speedup by caching the result of getSeconds in an inst var.  Lots of other values are defined in terms of getSeconds so this simply eliminates duplicating the large integer arithmetic in getSeconds.
> >
> > =============== Diff against Chronology-Core-ul.21 ===============
> >
> > Item was changed:
> >   Magnitude subclass: #DateAndTime
> > +       instanceVariableNames: 'utcMicroseconds localOffsetSeconds seconds'
> > -       instanceVariableNames: 'utcMicroseconds localOffsetSeconds'
> >         classVariableNames: 'AutomaticTimezone ClockProvider InitializeFromPrimitive LocalTimeZone PosixEpochJulianDays'
> >         poolDictionaries: 'ChronologyConstants'
> >         category: 'Chronology-Core'!
> >
> > + !DateAndTime commentStamp: 'eem 1/9/2019 18:44' prior: 0!
> > - !DateAndTime commentStamp: 'dtl 3/12/2016 10:32' prior: 0!
> >   I represent a point in UTC time as defined by ISO 8601. I have zero duration.
> >
> > + My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.  I cache the result of getSeconds in the seconds variable that is computed when required.
> > - My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.
> >
> >   The implementation ignores leap seconds, which are adjustments made to maintain earth rotational clock time in synchronization with elapsed seconds.
> >
> >   DateAndTime class>>now will use #primitiveUtcWithOffset to obtain current time in UTC microseconds with current local offset in seconds. The primitive provides an atomic query for UTC time and local offset as measured by the OS platform.  If primitiveUtcWithOffset is not available, the traditional implementation is used, which relies on a primitive for microseconds in the local time zone and derives UTC based on the TimeZone setting.
> >   !
> >
> > Item was changed:
> >   ----- Method: DateAndTime>>getSeconds (in category 'accessing') -----
> >   getSeconds
> > -
> >         | posixDays posixSeconds localSeconds |
> > +       seconds ifNil:
> > +               [posixSeconds := utcMicroseconds // 1000000.
> > +                localSeconds := posixSeconds + localOffsetSeconds.
> > +                localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
> > +                posixDays := localSeconds // SecondsInDay.
> > +                seconds := localSeconds - (posixDays * SecondsInDay)].
> > +       ^seconds!
> > -       posixSeconds := utcMicroseconds // 1000000.
> > -       localSeconds := posixSeconds + localOffsetSeconds.
> > -       localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
> > -       posixDays := localSeconds // SecondsInDay.
> > -       ^localSeconds - (posixDays * SecondsInDay).
> > - !
> >
> > Item was changed:
> >   ----- Method: DateAndTime>>readDataFrom:size: (in category 'objects from disk') -----
> >   readDataFrom: aDataStream size: varsOnDisk
> >         "Fill in the fields of self based on the contents of aDataStream. The serialized
> >         data will have four instance variables, because all instances are serialized in a
> >         cononical format as if having originating from an instance with the traditional
> >         seconds/offset/jdn/nanos instance variables."
> >
> >         | seconds offset jdn nanos |
> >         seconds := aDataStream next.
> >         offset := aDataStream next.
> >         jdn := aDataStream next.
> >         nanos := aDataStream next.
> >         localOffsetSeconds := offset ifNil: [ 0 ] ifNotNil: [ :off | off asSeconds ].
> >         utcMicroseconds := self
> >                                 microsecondsFromDay: jdn
> >                                 seconds: seconds
> >                                 nanos: nanos
> > +                               offset: localOffsetSeconds..
> > +       seconds := nil!
> > -                               offset: localOffsetSeconds.!
> >
> > Item was changed:
> >   ----- Method: DateAndTime>>setJdn:seconds:nano:localOffsetSeconds: (in category 'private') -----
> >   setJdn: jdn seconds: s nano: n localOffsetSeconds: offset
> >
> >         localOffsetSeconds := offset.
> >         utcMicroseconds := self
> >                                 microsecondsFromDay: jdn
> >                                 seconds: s
> >                                 nanos: n
> > +                               offset: offset.
> > +       seconds := nil!
> > -                               offset: offset!
> >
> > Item was changed:
> >   ----- Method: DateAndTime>>ticks:offset: (in category 'private') -----
> >   ticks: ticks offset: utcOffset
> >         "ticks is {julianDayNumber. secondCount. nanoSeconds}"
> >
> >         | jdn s nanos |
> >         self normalize: 3 ticks: ticks base: NanosInSecond.
> >         self normalize: 2 ticks: ticks base: SecondsInDay.
> >
> >         jdn     := ticks at: 1.
> >         s := ticks at: 2.
> >         nanos := ticks at: 3.
> >         localOffsetSeconds := utcOffset ifNil: [0] ifNotNil: [utcOffset asSeconds].
> >         utcMicroseconds := self microsecondsFromDay: jdn seconds: s nanos: nanos offset: localOffsetSeconds.
> > +       seconds := nil!
> > - !
> >
> > Item was changed:
> >   ----- Method: DateAndTime>>utcMicroseconds: (in category 'initialize-release') -----
> >   utcMicroseconds: utcValue
> >         "Allow value to be modified during initialization from a primitive in order to support
> >         monotonically increasing clock behavior."
> > +       utcMicroseconds := utcValue.
> > +       seconds := nil!
> > -       utcMicroseconds := utcValue!
> >
> > Item was changed:
> >   ----- Method: DateAndTime>>utcMicroseconds:offset: (in category 'initialize-release') -----
> >   utcMicroseconds: microsecondsSincePosixEpoch offset: tzOffset
> >
> >         utcMicroseconds := microsecondsSincePosixEpoch.
> >         localOffsetSeconds := tzOffset.
> > +       seconds := nil
> >   !
> >
> >
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

David T. Lewis
This is a good optimization. After taking a brief look at the performance
numbers, I can see a significant (almost 2X) improvement in DateAndTime>>asTime.
But I cannot really find any other examples where there is a measurable
improvement. I'm not sure that it's a big performance win in real world
use cases.

I recently spent a good deal of time with Chris Muller looking at performance
implications of the UTC updates for DateAndTime, and I learned that changes
to the shape of DateAndTime (specifically the instVar count) can have some
unanticipated side effects for IO to external systems (serialization,
databases, Magma).

My perspective is that this is a good optimization, but we might want
to hold off on putting it into trunk for a while, at least until folks
have had a chance to get comfortable with the new two instVar format
of DateAndTime.

Dave


On Thu, Jan 10, 2019 at 08:50:30AM -0500, David T. Lewis wrote:

> I am away and cannot look at this now, but on the face of it, it sounds
> like a good optimization, so if it makes a meaningful difference to
> performance then we should consider it.
>
> One clarification to Chris' comment - we (or at least I) did not expect
> UTCDateAndTime to be slower on 32-bit images. In fact, when I measured
> it (quite a long time ago now) it was faster on 32-bit images, and faster
> yet on 64-bit images. But my benchmarks were trivial, and Chris is
> considering use cases that involve IO to disk, and those might have
> very different performance considerations.
>
> The simple performance checks that I was using can be found in
> package Tests-UTCDateAndTime in the www.squeaksource.com/UTCDateAndTime
> repository. I can't look at it now, but those tests might be a useful
> reference here.
>
> Dave
>
> On Wed, Jan 09, 2019 at 10:38:04PM -0600, Chris Muller wrote:
> > -1.  DateAndTime is an object of which there are often many instances.
> > Adding another inst var increases size and processing to spin through
> > yet one more variable during disk-read, transmission and serialization
> > and materialization.  It's pure cost for 64-bit images, since there
> > isn't any LI arithmetic there anyway, and *we knew* going in that
> > UTCDateAndTime would be slower in 32-bit.  Avoiding LI arithmetic in
> > 32-bit is what the original Chronology did....
> >
> > Regards,
> >   Chris
> >
> >
> >
> > On Wed, Jan 9, 2019 at 8:53 PM <[hidden email]> wrote:
> > >
> > > A new version of Chronology-Core was added to project The Inbox:
> > > http://source.squeak.org/inbox/Chronology-Core-eem.22.mcz
> > >
> > > ==================== Summary ====================
> > >
> > > Name: Chronology-Core-eem.22
> > > Author: eem
> > > Time: 9 January 2019, 6:53:52.265064 pm
> > > UUID: 348c4ad7-0a27-4021-8788-9e3ec220df24
> > > Ancestors: Chronology-Core-ul.21
> > >
> > > Simple speedup by caching the result of getSeconds in an inst var.  Lots of other values are defined in terms of getSeconds so this simply eliminates duplicating the large integer arithmetic in getSeconds.
> > >
> > > =============== Diff against Chronology-Core-ul.21 ===============
> > >
> > > Item was changed:
> > >   Magnitude subclass: #DateAndTime
> > > +       instanceVariableNames: 'utcMicroseconds localOffsetSeconds seconds'
> > > -       instanceVariableNames: 'utcMicroseconds localOffsetSeconds'
> > >         classVariableNames: 'AutomaticTimezone ClockProvider InitializeFromPrimitive LocalTimeZone PosixEpochJulianDays'
> > >         poolDictionaries: 'ChronologyConstants'
> > >         category: 'Chronology-Core'!
> > >
> > > + !DateAndTime commentStamp: 'eem 1/9/2019 18:44' prior: 0!
> > > - !DateAndTime commentStamp: 'dtl 3/12/2016 10:32' prior: 0!
> > >   I represent a point in UTC time as defined by ISO 8601. I have zero duration.
> > >
> > > + My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.  I cache the result of getSeconds in the seconds variable that is computed when required.
> > > - My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.
> > >
> > >   The implementation ignores leap seconds, which are adjustments made to maintain earth rotational clock time in synchronization with elapsed seconds.
> > >
> > >   DateAndTime class>>now will use #primitiveUtcWithOffset to obtain current time in UTC microseconds with current local offset in seconds. The primitive provides an atomic query for UTC time and local offset as measured by the OS platform.  If primitiveUtcWithOffset is not available, the traditional implementation is used, which relies on a primitive for microseconds in the local time zone and derives UTC based on the TimeZone setting.
> > >   !
> > >
> > > Item was changed:
> > >   ----- Method: DateAndTime>>getSeconds (in category 'accessing') -----
> > >   getSeconds
> > > -
> > >         | posixDays posixSeconds localSeconds |
> > > +       seconds ifNil:
> > > +               [posixSeconds := utcMicroseconds // 1000000.
> > > +                localSeconds := posixSeconds + localOffsetSeconds.
> > > +                localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
> > > +                posixDays := localSeconds // SecondsInDay.
> > > +                seconds := localSeconds - (posixDays * SecondsInDay)].
> > > +       ^seconds!
> > > -       posixSeconds := utcMicroseconds // 1000000.
> > > -       localSeconds := posixSeconds + localOffsetSeconds.
> > > -       localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
> > > -       posixDays := localSeconds // SecondsInDay.
> > > -       ^localSeconds - (posixDays * SecondsInDay).
> > > - !
> > >
> > > Item was changed:
> > >   ----- Method: DateAndTime>>readDataFrom:size: (in category 'objects from disk') -----
> > >   readDataFrom: aDataStream size: varsOnDisk
> > >         "Fill in the fields of self based on the contents of aDataStream. The serialized
> > >         data will have four instance variables, because all instances are serialized in a
> > >         cononical format as if having originating from an instance with the traditional
> > >         seconds/offset/jdn/nanos instance variables."
> > >
> > >         | seconds offset jdn nanos |
> > >         seconds := aDataStream next.
> > >         offset := aDataStream next.
> > >         jdn := aDataStream next.
> > >         nanos := aDataStream next.
> > >         localOffsetSeconds := offset ifNil: [ 0 ] ifNotNil: [ :off | off asSeconds ].
> > >         utcMicroseconds := self
> > >                                 microsecondsFromDay: jdn
> > >                                 seconds: seconds
> > >                                 nanos: nanos
> > > +                               offset: localOffsetSeconds..
> > > +       seconds := nil!
> > > -                               offset: localOffsetSeconds.!
> > >
> > > Item was changed:
> > >   ----- Method: DateAndTime>>setJdn:seconds:nano:localOffsetSeconds: (in category 'private') -----
> > >   setJdn: jdn seconds: s nano: n localOffsetSeconds: offset
> > >
> > >         localOffsetSeconds := offset.
> > >         utcMicroseconds := self
> > >                                 microsecondsFromDay: jdn
> > >                                 seconds: s
> > >                                 nanos: n
> > > +                               offset: offset.
> > > +       seconds := nil!
> > > -                               offset: offset!
> > >
> > > Item was changed:
> > >   ----- Method: DateAndTime>>ticks:offset: (in category 'private') -----
> > >   ticks: ticks offset: utcOffset
> > >         "ticks is {julianDayNumber. secondCount. nanoSeconds}"
> > >
> > >         | jdn s nanos |
> > >         self normalize: 3 ticks: ticks base: NanosInSecond.
> > >         self normalize: 2 ticks: ticks base: SecondsInDay.
> > >
> > >         jdn     := ticks at: 1.
> > >         s := ticks at: 2.
> > >         nanos := ticks at: 3.
> > >         localOffsetSeconds := utcOffset ifNil: [0] ifNotNil: [utcOffset asSeconds].
> > >         utcMicroseconds := self microsecondsFromDay: jdn seconds: s nanos: nanos offset: localOffsetSeconds.
> > > +       seconds := nil!
> > > - !
> > >
> > > Item was changed:
> > >   ----- Method: DateAndTime>>utcMicroseconds: (in category 'initialize-release') -----
> > >   utcMicroseconds: utcValue
> > >         "Allow value to be modified during initialization from a primitive in order to support
> > >         monotonically increasing clock behavior."
> > > +       utcMicroseconds := utcValue.
> > > +       seconds := nil!
> > > -       utcMicroseconds := utcValue!
> > >
> > > Item was changed:
> > >   ----- Method: DateAndTime>>utcMicroseconds:offset: (in category 'initialize-release') -----
> > >   utcMicroseconds: microsecondsSincePosixEpoch offset: tzOffset
> > >
> > >         utcMicroseconds := microsecondsSincePosixEpoch.
> > >         localOffsetSeconds := tzOffset.
> > > +       seconds := nil
> > >   !
> > >
> > >
> >
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

Eliot Miranda-2


> On Jan 10, 2019, at 6:20 PM, David T. Lewis <[hidden email]> wrote:
>
> This is a good optimization. After taking a brief look at the performance
> numbers, I can see a significant (almost 2X) improvement in DateAndTime>>asTime.

In 32-bit or 64-bit?

> But I cannot really find any other examples where there is a measurable
> improvement. I'm not sure that it's a big performance win in real world
> use cases.
>
> I recently spent a good deal of time with Chris Muller looking at performance
> implications of the UTC updates for DateAndTime, and I learned that changes
> to the shape of DateAndTime (specifically the instVar count) can have some
> unanticipated side effects for IO to external systems (serialization,
> databases, Magma).
>
> My perspective is that this is a good optimization, but we might want
> to hold off on putting it into trunk for a while, at least until folks
> have had a chance to get comfortable with the new two instVar format
> of DateAndTime.

I’m good with that :-)

>
> Dave
>
>
>> On Thu, Jan 10, 2019 at 08:50:30AM -0500, David T. Lewis wrote:
>> I am away and cannot look at this now, but on the face of it, it sounds
>> like a good optimization, so if it makes a meaningful difference to
>> performance then we should consider it.
>>
>> One clarification to Chris' comment - we (or at least I) did not expect
>> UTCDateAndTime to be slower on 32-bit images. In fact, when I measured
>> it (quite a long time ago now) it was faster on 32-bit images, and faster
>> yet on 64-bit images. But my benchmarks were trivial, and Chris is
>> considering use cases that involve IO to disk, and those might have
>> very different performance considerations.
>>
>> The simple performance checks that I was using can be found in
>> package Tests-UTCDateAndTime in the www.squeaksource.com/UTCDateAndTime
>> repository. I can't look at it now, but those tests might be a useful
>> reference here.
>>
>> Dave
>>
>>> On Wed, Jan 09, 2019 at 10:38:04PM -0600, Chris Muller wrote:
>>> -1.  DateAndTime is an object of which there are often many instances.
>>> Adding another inst var increases size and processing to spin through
>>> yet one more variable during disk-read, transmission and serialization
>>> and materialization.  It's pure cost for 64-bit images, since there
>>> isn't any LI arithmetic there anyway, and *we knew* going in that
>>> UTCDateAndTime would be slower in 32-bit.  Avoiding LI arithmetic in
>>> 32-bit is what the original Chronology did....
>>>
>>> Regards,
>>>  Chris
>>>
>>>
>>>
>>>> On Wed, Jan 9, 2019 at 8:53 PM <[hidden email]> wrote:
>>>>
>>>> A new version of Chronology-Core was added to project The Inbox:
>>>> http://source.squeak.org/inbox/Chronology-Core-eem.22.mcz
>>>>
>>>> ==================== Summary ====================
>>>>
>>>> Name: Chronology-Core-eem.22
>>>> Author: eem
>>>> Time: 9 January 2019, 6:53:52.265064 pm
>>>> UUID: 348c4ad7-0a27-4021-8788-9e3ec220df24
>>>> Ancestors: Chronology-Core-ul.21
>>>>
>>>> Simple speedup by caching the result of getSeconds in an inst var.  Lots of other values are defined in terms of getSeconds so this simply eliminates duplicating the large integer arithmetic in getSeconds.
>>>>
>>>> =============== Diff against Chronology-Core-ul.21 ===============
>>>>
>>>> Item was changed:
>>>>  Magnitude subclass: #DateAndTime
>>>> +       instanceVariableNames: 'utcMicroseconds localOffsetSeconds seconds'
>>>> -       instanceVariableNames: 'utcMicroseconds localOffsetSeconds'
>>>>        classVariableNames: 'AutomaticTimezone ClockProvider InitializeFromPrimitive LocalTimeZone PosixEpochJulianDays'
>>>>        poolDictionaries: 'ChronologyConstants'
>>>>        category: 'Chronology-Core'!
>>>>
>>>> + !DateAndTime commentStamp: 'eem 1/9/2019 18:44' prior: 0!
>>>> - !DateAndTime commentStamp: 'dtl 3/12/2016 10:32' prior: 0!
>>>>  I represent a point in UTC time as defined by ISO 8601. I have zero duration.
>>>>
>>>> + My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.  I cache the result of getSeconds in the seconds variable that is computed when required.
>>>> - My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.
>>>>
>>>>  The implementation ignores leap seconds, which are adjustments made to maintain earth rotational clock time in synchronization with elapsed seconds.
>>>>
>>>>  DateAndTime class>>now will use #primitiveUtcWithOffset to obtain current time in UTC microseconds with current local offset in seconds. The primitive provides an atomic query for UTC time and local offset as measured by the OS platform.  If primitiveUtcWithOffset is not available, the traditional implementation is used, which relies on a primitive for microseconds in the local time zone and derives UTC based on the TimeZone setting.
>>>>  !
>>>>
>>>> Item was changed:
>>>>  ----- Method: DateAndTime>>getSeconds (in category 'accessing') -----
>>>>  getSeconds
>>>> -
>>>>        | posixDays posixSeconds localSeconds |
>>>> +       seconds ifNil:
>>>> +               [posixSeconds := utcMicroseconds // 1000000.
>>>> +                localSeconds := posixSeconds + localOffsetSeconds.
>>>> +                localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
>>>> +                posixDays := localSeconds // SecondsInDay.
>>>> +                seconds := localSeconds - (posixDays * SecondsInDay)].
>>>> +       ^seconds!
>>>> -       posixSeconds := utcMicroseconds // 1000000.
>>>> -       localSeconds := posixSeconds + localOffsetSeconds.
>>>> -       localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
>>>> -       posixDays := localSeconds // SecondsInDay.
>>>> -       ^localSeconds - (posixDays * SecondsInDay).
>>>> - !
>>>>
>>>> Item was changed:
>>>>  ----- Method: DateAndTime>>readDataFrom:size: (in category 'objects from disk') -----
>>>>  readDataFrom: aDataStream size: varsOnDisk
>>>>        "Fill in the fields of self based on the contents of aDataStream. The serialized
>>>>        data will have four instance variables, because all instances are serialized in a
>>>>        cononical format as if having originating from an instance with the traditional
>>>>        seconds/offset/jdn/nanos instance variables."
>>>>
>>>>        | seconds offset jdn nanos |
>>>>        seconds := aDataStream next.
>>>>        offset := aDataStream next.
>>>>        jdn := aDataStream next.
>>>>        nanos := aDataStream next.
>>>>        localOffsetSeconds := offset ifNil: [ 0 ] ifNotNil: [ :off | off asSeconds ].
>>>>        utcMicroseconds := self
>>>>                                microsecondsFromDay: jdn
>>>>                                seconds: seconds
>>>>                                nanos: nanos
>>>> +                               offset: localOffsetSeconds..
>>>> +       seconds := nil!
>>>> -                               offset: localOffsetSeconds.!
>>>>
>>>> Item was changed:
>>>>  ----- Method: DateAndTime>>setJdn:seconds:nano:localOffsetSeconds: (in category 'private') -----
>>>>  setJdn: jdn seconds: s nano: n localOffsetSeconds: offset
>>>>
>>>>        localOffsetSeconds := offset.
>>>>        utcMicroseconds := self
>>>>                                microsecondsFromDay: jdn
>>>>                                seconds: s
>>>>                                nanos: n
>>>> +                               offset: offset.
>>>> +       seconds := nil!
>>>> -                               offset: offset!
>>>>
>>>> Item was changed:
>>>>  ----- Method: DateAndTime>>ticks:offset: (in category 'private') -----
>>>>  ticks: ticks offset: utcOffset
>>>>        "ticks is {julianDayNumber. secondCount. nanoSeconds}"
>>>>
>>>>        | jdn s nanos |
>>>>        self normalize: 3 ticks: ticks base: NanosInSecond.
>>>>        self normalize: 2 ticks: ticks base: SecondsInDay.
>>>>
>>>>        jdn     := ticks at: 1.
>>>>        s := ticks at: 2.
>>>>        nanos := ticks at: 3.
>>>>        localOffsetSeconds := utcOffset ifNil: [0] ifNotNil: [utcOffset asSeconds].
>>>>        utcMicroseconds := self microsecondsFromDay: jdn seconds: s nanos: nanos offset: localOffsetSeconds.
>>>> +       seconds := nil!
>>>> - !
>>>>
>>>> Item was changed:
>>>>  ----- Method: DateAndTime>>utcMicroseconds: (in category 'initialize-release') -----
>>>>  utcMicroseconds: utcValue
>>>>        "Allow value to be modified during initialization from a primitive in order to support
>>>>        monotonically increasing clock behavior."
>>>> +       utcMicroseconds := utcValue.
>>>> +       seconds := nil!
>>>> -       utcMicroseconds := utcValue!
>>>>
>>>> Item was changed:
>>>>  ----- Method: DateAndTime>>utcMicroseconds:offset: (in category 'initialize-release') -----
>>>>  utcMicroseconds: microsecondsSincePosixEpoch offset: tzOffset
>>>>
>>>>        utcMicroseconds := microsecondsSincePosixEpoch.
>>>>        localOffsetSeconds := tzOffset.
>>>> +       seconds := nil
>>>>  !
>>>>
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

David T. Lewis
On Fri, Jan 11, 2019 at 09:43:03AM -0800, Eliot Miranda wrote:
>
>
> > On Jan 10, 2019, at 6:20 PM, David T. Lewis <[hidden email]> wrote:
> >
> > This is a good optimization. After taking a brief look at the performance
> > numbers, I can see a significant (almost 2X) improvement in DateAndTime>>asTime.
>
> In 32-bit or 64-bit?

This is on 64-bit, I did not check 32-bit.

Dave

>
> > But I cannot really find any other examples where there is a measurable
> > improvement. I'm not sure that it's a big performance win in real world
> > use cases.
> >
> > I recently spent a good deal of time with Chris Muller looking at performance
> > implications of the UTC updates for DateAndTime, and I learned that changes
> > to the shape of DateAndTime (specifically the instVar count) can have some
> > unanticipated side effects for IO to external systems (serialization,
> > databases, Magma).
> >
> > My perspective is that this is a good optimization, but we might want
> > to hold off on putting it into trunk for a while, at least until folks
> > have had a chance to get comfortable with the new two instVar format
> > of DateAndTime.
>
> I???m good with that :-)
>
> >
> > Dave
> >
> >
> >> On Thu, Jan 10, 2019 at 08:50:30AM -0500, David T. Lewis wrote:
> >> I am away and cannot look at this now, but on the face of it, it sounds
> >> like a good optimization, so if it makes a meaningful difference to
> >> performance then we should consider it.
> >>
> >> One clarification to Chris' comment - we (or at least I) did not expect
> >> UTCDateAndTime to be slower on 32-bit images. In fact, when I measured
> >> it (quite a long time ago now) it was faster on 32-bit images, and faster
> >> yet on 64-bit images. But my benchmarks were trivial, and Chris is
> >> considering use cases that involve IO to disk, and those might have
> >> very different performance considerations.
> >>
> >> The simple performance checks that I was using can be found in
> >> package Tests-UTCDateAndTime in the www.squeaksource.com/UTCDateAndTime
> >> repository. I can't look at it now, but those tests might be a useful
> >> reference here.
> >>
> >> Dave
> >>
> >>> On Wed, Jan 09, 2019 at 10:38:04PM -0600, Chris Muller wrote:
> >>> -1.  DateAndTime is an object of which there are often many instances.
> >>> Adding another inst var increases size and processing to spin through
> >>> yet one more variable during disk-read, transmission and serialization
> >>> and materialization.  It's pure cost for 64-bit images, since there
> >>> isn't any LI arithmetic there anyway, and *we knew* going in that
> >>> UTCDateAndTime would be slower in 32-bit.  Avoiding LI arithmetic in
> >>> 32-bit is what the original Chronology did....
> >>>
> >>> Regards,
> >>>  Chris
> >>>
> >>>
> >>>
> >>>> On Wed, Jan 9, 2019 at 8:53 PM <[hidden email]> wrote:
> >>>>
> >>>> A new version of Chronology-Core was added to project The Inbox:
> >>>> http://source.squeak.org/inbox/Chronology-Core-eem.22.mcz
> >>>>
> >>>> ==================== Summary ====================
> >>>>
> >>>> Name: Chronology-Core-eem.22
> >>>> Author: eem
> >>>> Time: 9 January 2019, 6:53:52.265064 pm
> >>>> UUID: 348c4ad7-0a27-4021-8788-9e3ec220df24
> >>>> Ancestors: Chronology-Core-ul.21
> >>>>
> >>>> Simple speedup by caching the result of getSeconds in an inst var.  Lots of other values are defined in terms of getSeconds so this simply eliminates duplicating the large integer arithmetic in getSeconds.
> >>>>
> >>>> =============== Diff against Chronology-Core-ul.21 ===============
> >>>>
> >>>> Item was changed:
> >>>>  Magnitude subclass: #DateAndTime
> >>>> +       instanceVariableNames: 'utcMicroseconds localOffsetSeconds seconds'
> >>>> -       instanceVariableNames: 'utcMicroseconds localOffsetSeconds'
> >>>>        classVariableNames: 'AutomaticTimezone ClockProvider InitializeFromPrimitive LocalTimeZone PosixEpochJulianDays'
> >>>>        poolDictionaries: 'ChronologyConstants'
> >>>>        category: 'Chronology-Core'!
> >>>>
> >>>> + !DateAndTime commentStamp: 'eem 1/9/2019 18:44' prior: 0!
> >>>> - !DateAndTime commentStamp: 'dtl 3/12/2016 10:32' prior: 0!
> >>>>  I represent a point in UTC time as defined by ISO 8601. I have zero duration.
> >>>>
> >>>> + My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.  I cache the result of getSeconds in the seconds variable that is computed when required.
> >>>> - My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.
> >>>>
> >>>>  The implementation ignores leap seconds, which are adjustments made to maintain earth rotational clock time in synchronization with elapsed seconds.
> >>>>
> >>>>  DateAndTime class>>now will use #primitiveUtcWithOffset to obtain current time in UTC microseconds with current local offset in seconds. The primitive provides an atomic query for UTC time and local offset as measured by the OS platform.  If primitiveUtcWithOffset is not available, the traditional implementation is used, which relies on a primitive for microseconds in the local time zone and derives UTC based on the TimeZone setting.
> >>>>  !
> >>>>
> >>>> Item was changed:
> >>>>  ----- Method: DateAndTime>>getSeconds (in category 'accessing') -----
> >>>>  getSeconds
> >>>> -
> >>>>        | posixDays posixSeconds localSeconds |
> >>>> +       seconds ifNil:
> >>>> +               [posixSeconds := utcMicroseconds // 1000000.
> >>>> +                localSeconds := posixSeconds + localOffsetSeconds.
> >>>> +                localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
> >>>> +                posixDays := localSeconds // SecondsInDay.
> >>>> +                seconds := localSeconds - (posixDays * SecondsInDay)].
> >>>> +       ^seconds!
> >>>> -       posixSeconds := utcMicroseconds // 1000000.
> >>>> -       localSeconds := posixSeconds + localOffsetSeconds.
> >>>> -       localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
> >>>> -       posixDays := localSeconds // SecondsInDay.
> >>>> -       ^localSeconds - (posixDays * SecondsInDay).
> >>>> - !
> >>>>
> >>>> Item was changed:
> >>>>  ----- Method: DateAndTime>>readDataFrom:size: (in category 'objects from disk') -----
> >>>>  readDataFrom: aDataStream size: varsOnDisk
> >>>>        "Fill in the fields of self based on the contents of aDataStream. The serialized
> >>>>        data will have four instance variables, because all instances are serialized in a
> >>>>        cononical format as if having originating from an instance with the traditional
> >>>>        seconds/offset/jdn/nanos instance variables."
> >>>>
> >>>>        | seconds offset jdn nanos |
> >>>>        seconds := aDataStream next.
> >>>>        offset := aDataStream next.
> >>>>        jdn := aDataStream next.
> >>>>        nanos := aDataStream next.
> >>>>        localOffsetSeconds := offset ifNil: [ 0 ] ifNotNil: [ :off | off asSeconds ].
> >>>>        utcMicroseconds := self
> >>>>                                microsecondsFromDay: jdn
> >>>>                                seconds: seconds
> >>>>                                nanos: nanos
> >>>> +                               offset: localOffsetSeconds..
> >>>> +       seconds := nil!
> >>>> -                               offset: localOffsetSeconds.!
> >>>>
> >>>> Item was changed:
> >>>>  ----- Method: DateAndTime>>setJdn:seconds:nano:localOffsetSeconds: (in category 'private') -----
> >>>>  setJdn: jdn seconds: s nano: n localOffsetSeconds: offset
> >>>>
> >>>>        localOffsetSeconds := offset.
> >>>>        utcMicroseconds := self
> >>>>                                microsecondsFromDay: jdn
> >>>>                                seconds: s
> >>>>                                nanos: n
> >>>> +                               offset: offset.
> >>>> +       seconds := nil!
> >>>> -                               offset: offset!
> >>>>
> >>>> Item was changed:
> >>>>  ----- Method: DateAndTime>>ticks:offset: (in category 'private') -----
> >>>>  ticks: ticks offset: utcOffset
> >>>>        "ticks is {julianDayNumber. secondCount. nanoSeconds}"
> >>>>
> >>>>        | jdn s nanos |
> >>>>        self normalize: 3 ticks: ticks base: NanosInSecond.
> >>>>        self normalize: 2 ticks: ticks base: SecondsInDay.
> >>>>
> >>>>        jdn     := ticks at: 1.
> >>>>        s := ticks at: 2.
> >>>>        nanos := ticks at: 3.
> >>>>        localOffsetSeconds := utcOffset ifNil: [0] ifNotNil: [utcOffset asSeconds].
> >>>>        utcMicroseconds := self microsecondsFromDay: jdn seconds: s nanos: nanos offset: localOffsetSeconds.
> >>>> +       seconds := nil!
> >>>> - !
> >>>>
> >>>> Item was changed:
> >>>>  ----- Method: DateAndTime>>utcMicroseconds: (in category 'initialize-release') -----
> >>>>  utcMicroseconds: utcValue
> >>>>        "Allow value to be modified during initialization from a primitive in order to support
> >>>>        monotonically increasing clock behavior."
> >>>> +       utcMicroseconds := utcValue.
> >>>> +       seconds := nil!
> >>>> -       utcMicroseconds := utcValue!
> >>>>
> >>>> Item was changed:
> >>>>  ----- Method: DateAndTime>>utcMicroseconds:offset: (in category 'initialize-release') -----
> >>>>  utcMicroseconds: microsecondsSincePosixEpoch offset: tzOffset
> >>>>
> >>>>        utcMicroseconds := microsecondsSincePosixEpoch.
> >>>>        localOffsetSeconds := tzOffset.
> >>>> +       seconds := nil
> >>>>  !
> >>>>
> >>>>
> >>>
> >>
> >
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

Eliot Miranda-2
Hi David,

> On Jan 11, 2019, at 11:15 AM, David T. Lewis <[hidden email]> wrote:
>
>> On Fri, Jan 11, 2019 at 09:43:03AM -0800, Eliot Miranda wrote:
>>
>>
>>> On Jan 10, 2019, at 6:20 PM, David T. Lewis <[hidden email]> wrote:
>>>
>>> This is a good optimization. After taking a brief look at the performance
>>> numbers, I can see a significant (almost 2X) improvement in DateAndTime>>asTime.
>>
>> In 32-bit or 64-bit?
>
> This is on 64-bit, I did not check 32-bit.

Can you post the test?  I’d like to measure 32-bits where the difference should be larger.

>
> Dave
>
>>
>>> But I cannot really find any other examples where there is a measurable
>>> improvement. I'm not sure that it's a big performance win in real world
>>> use cases.
>>>
>>> I recently spent a good deal of time with Chris Muller looking at performance
>>> implications of the UTC updates for DateAndTime, and I learned that changes
>>> to the shape of DateAndTime (specifically the instVar count) can have some
>>> unanticipated side effects for IO to external systems (serialization,
>>> databases, Magma).
>>>
>>> My perspective is that this is a good optimization, but we might want
>>> to hold off on putting it into trunk for a while, at least until folks
>>> have had a chance to get comfortable with the new two instVar format
>>> of DateAndTime.
>>
>> I???m good with that :-)
>>
>>>
>>> Dave
>>>
>>>
>>>> On Thu, Jan 10, 2019 at 08:50:30AM -0500, David T. Lewis wrote:
>>>> I am away and cannot look at this now, but on the face of it, it sounds
>>>> like a good optimization, so if it makes a meaningful difference to
>>>> performance then we should consider it.
>>>>
>>>> One clarification to Chris' comment - we (or at least I) did not expect
>>>> UTCDateAndTime to be slower on 32-bit images. In fact, when I measured
>>>> it (quite a long time ago now) it was faster on 32-bit images, and faster
>>>> yet on 64-bit images. But my benchmarks were trivial, and Chris is
>>>> considering use cases that involve IO to disk, and those might have
>>>> very different performance considerations.
>>>>
>>>> The simple performance checks that I was using can be found in
>>>> package Tests-UTCDateAndTime in the www.squeaksource.com/UTCDateAndTime
>>>> repository. I can't look at it now, but those tests might be a useful
>>>> reference here.
>>>>
>>>> Dave
>>>>
>>>>> On Wed, Jan 09, 2019 at 10:38:04PM -0600, Chris Muller wrote:
>>>>> -1.  DateAndTime is an object of which there are often many instances.
>>>>> Adding another inst var increases size and processing to spin through
>>>>> yet one more variable during disk-read, transmission and serialization
>>>>> and materialization.  It's pure cost for 64-bit images, since there
>>>>> isn't any LI arithmetic there anyway, and *we knew* going in that
>>>>> UTCDateAndTime would be slower in 32-bit.  Avoiding LI arithmetic in
>>>>> 32-bit is what the original Chronology did....
>>>>>
>>>>> Regards,
>>>>> Chris
>>>>>
>>>>>
>>>>>
>>>>>> On Wed, Jan 9, 2019 at 8:53 PM <[hidden email]> wrote:
>>>>>>
>>>>>> A new version of Chronology-Core was added to project The Inbox:
>>>>>> http://source.squeak.org/inbox/Chronology-Core-eem.22.mcz
>>>>>>
>>>>>> ==================== Summary ====================
>>>>>>
>>>>>> Name: Chronology-Core-eem.22
>>>>>> Author: eem
>>>>>> Time: 9 January 2019, 6:53:52.265064 pm
>>>>>> UUID: 348c4ad7-0a27-4021-8788-9e3ec220df24
>>>>>> Ancestors: Chronology-Core-ul.21
>>>>>>
>>>>>> Simple speedup by caching the result of getSeconds in an inst var.  Lots of other values are defined in terms of getSeconds so this simply eliminates duplicating the large integer arithmetic in getSeconds.
>>>>>>
>>>>>> =============== Diff against Chronology-Core-ul.21 ===============
>>>>>>
>>>>>> Item was changed:
>>>>>> Magnitude subclass: #DateAndTime
>>>>>> +       instanceVariableNames: 'utcMicroseconds localOffsetSeconds seconds'
>>>>>> -       instanceVariableNames: 'utcMicroseconds localOffsetSeconds'
>>>>>>       classVariableNames: 'AutomaticTimezone ClockProvider InitializeFromPrimitive LocalTimeZone PosixEpochJulianDays'
>>>>>>       poolDictionaries: 'ChronologyConstants'
>>>>>>       category: 'Chronology-Core'!
>>>>>>
>>>>>> + !DateAndTime commentStamp: 'eem 1/9/2019 18:44' prior: 0!
>>>>>> - !DateAndTime commentStamp: 'dtl 3/12/2016 10:32' prior: 0!
>>>>>> I represent a point in UTC time as defined by ISO 8601. I have zero duration.
>>>>>>
>>>>>> + My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.  I cache the result of getSeconds in the seconds variable that is computed when required.
>>>>>> - My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.
>>>>>>
>>>>>> The implementation ignores leap seconds, which are adjustments made to maintain earth rotational clock time in synchronization with elapsed seconds.
>>>>>>
>>>>>> DateAndTime class>>now will use #primitiveUtcWithOffset to obtain current time in UTC microseconds with current local offset in seconds. The primitive provides an atomic query for UTC time and local offset as measured by the OS platform.  If primitiveUtcWithOffset is not available, the traditional implementation is used, which relies on a primitive for microseconds in the local time zone and derives UTC based on the TimeZone setting.
>>>>>> !
>>>>>>
>>>>>> Item was changed:
>>>>>> ----- Method: DateAndTime>>getSeconds (in category 'accessing') -----
>>>>>> getSeconds
>>>>>> -
>>>>>>       | posixDays posixSeconds localSeconds |
>>>>>> +       seconds ifNil:
>>>>>> +               [posixSeconds := utcMicroseconds // 1000000.
>>>>>> +                localSeconds := posixSeconds + localOffsetSeconds.
>>>>>> +                localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
>>>>>> +                posixDays := localSeconds // SecondsInDay.
>>>>>> +                seconds := localSeconds - (posixDays * SecondsInDay)].
>>>>>> +       ^seconds!
>>>>>> -       posixSeconds := utcMicroseconds // 1000000.
>>>>>> -       localSeconds := posixSeconds + localOffsetSeconds.
>>>>>> -       localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
>>>>>> -       posixDays := localSeconds // SecondsInDay.
>>>>>> -       ^localSeconds - (posixDays * SecondsInDay).
>>>>>> - !
>>>>>>
>>>>>> Item was changed:
>>>>>> ----- Method: DateAndTime>>readDataFrom:size: (in category 'objects from disk') -----
>>>>>> readDataFrom: aDataStream size: varsOnDisk
>>>>>>       "Fill in the fields of self based on the contents of aDataStream. The serialized
>>>>>>       data will have four instance variables, because all instances are serialized in a
>>>>>>       cononical format as if having originating from an instance with the traditional
>>>>>>       seconds/offset/jdn/nanos instance variables."
>>>>>>
>>>>>>       | seconds offset jdn nanos |
>>>>>>       seconds := aDataStream next.
>>>>>>       offset := aDataStream next.
>>>>>>       jdn := aDataStream next.
>>>>>>       nanos := aDataStream next.
>>>>>>       localOffsetSeconds := offset ifNil: [ 0 ] ifNotNil: [ :off | off asSeconds ].
>>>>>>       utcMicroseconds := self
>>>>>>                               microsecondsFromDay: jdn
>>>>>>                               seconds: seconds
>>>>>>                               nanos: nanos
>>>>>> +                               offset: localOffsetSeconds..
>>>>>> +       seconds := nil!
>>>>>> -                               offset: localOffsetSeconds.!
>>>>>>
>>>>>> Item was changed:
>>>>>> ----- Method: DateAndTime>>setJdn:seconds:nano:localOffsetSeconds: (in category 'private') -----
>>>>>> setJdn: jdn seconds: s nano: n localOffsetSeconds: offset
>>>>>>
>>>>>>       localOffsetSeconds := offset.
>>>>>>       utcMicroseconds := self
>>>>>>                               microsecondsFromDay: jdn
>>>>>>                               seconds: s
>>>>>>                               nanos: n
>>>>>> +                               offset: offset.
>>>>>> +       seconds := nil!
>>>>>> -                               offset: offset!
>>>>>>
>>>>>> Item was changed:
>>>>>> ----- Method: DateAndTime>>ticks:offset: (in category 'private') -----
>>>>>> ticks: ticks offset: utcOffset
>>>>>>       "ticks is {julianDayNumber. secondCount. nanoSeconds}"
>>>>>>
>>>>>>       | jdn s nanos |
>>>>>>       self normalize: 3 ticks: ticks base: NanosInSecond.
>>>>>>       self normalize: 2 ticks: ticks base: SecondsInDay.
>>>>>>
>>>>>>       jdn     := ticks at: 1.
>>>>>>       s := ticks at: 2.
>>>>>>       nanos := ticks at: 3.
>>>>>>       localOffsetSeconds := utcOffset ifNil: [0] ifNotNil: [utcOffset asSeconds].
>>>>>>       utcMicroseconds := self microsecondsFromDay: jdn seconds: s nanos: nanos offset: localOffsetSeconds.
>>>>>> +       seconds := nil!
>>>>>> - !
>>>>>>
>>>>>> Item was changed:
>>>>>> ----- Method: DateAndTime>>utcMicroseconds: (in category 'initialize-release') -----
>>>>>> utcMicroseconds: utcValue
>>>>>>       "Allow value to be modified during initialization from a primitive in order to support
>>>>>>       monotonically increasing clock behavior."
>>>>>> +       utcMicroseconds := utcValue.
>>>>>> +       seconds := nil!
>>>>>> -       utcMicroseconds := utcValue!
>>>>>>
>>>>>> Item was changed:
>>>>>> ----- Method: DateAndTime>>utcMicroseconds:offset: (in category 'initialize-release') -----
>>>>>> utcMicroseconds: microsecondsSincePosixEpoch offset: tzOffset
>>>>>>
>>>>>>       utcMicroseconds := microsecondsSincePosixEpoch.
>>>>>>       localOffsetSeconds := tzOffset.
>>>>>> +       seconds := nil
>>>>>> !
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

Levente Uzonyi
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

This version is a bit faster, the proposed optimization is "just" ~9x
quicker.

However, I still don't see when is #getSeconds invoked multiple times to
justify caching the value.

Levente

On Fri, 11 Jan 2019, Eliot Miranda wrote:

> Hi David,
>
>> On Jan 11, 2019, at 11:15 AM, David T. Lewis <[hidden email]> wrote:
>>
>>> On Fri, Jan 11, 2019 at 09:43:03AM -0800, Eliot Miranda wrote:
>>>
>>>
>>>> On Jan 10, 2019, at 6:20 PM, David T. Lewis <[hidden email]> wrote:
>>>>
>>>> This is a good optimization. After taking a brief look at the performance
>>>> numbers, I can see a significant (almost 2X) improvement in DateAndTime>>asTime.
>>>
>>> In 32-bit or 64-bit?
>>
>> This is on 64-bit, I did not check 32-bit.
>
> Can you post the test?  I’d like to measure 32-bits where the difference should be larger.
>
>>
>> Dave
>>
>>>
>>>> But I cannot really find any other examples where there is a measurable
>>>> improvement. I'm not sure that it's a big performance win in real world
>>>> use cases.
>>>>
>>>> I recently spent a good deal of time with Chris Muller looking at performance
>>>> implications of the UTC updates for DateAndTime, and I learned that changes
>>>> to the shape of DateAndTime (specifically the instVar count) can have some
>>>> unanticipated side effects for IO to external systems (serialization,
>>>> databases, Magma).
>>>>
>>>> My perspective is that this is a good optimization, but we might want
>>>> to hold off on putting it into trunk for a while, at least until folks
>>>> have had a chance to get comfortable with the new two instVar format
>>>> of DateAndTime.
>>>
>>> I???m good with that :-)
>>>
>>>>
>>>> Dave
>>>>
>>>>
>>>>> On Thu, Jan 10, 2019 at 08:50:30AM -0500, David T. Lewis wrote:
>>>>> I am away and cannot look at this now, but on the face of it, it sounds
>>>>> like a good optimization, so if it makes a meaningful difference to
>>>>> performance then we should consider it.
>>>>>
>>>>> One clarification to Chris' comment - we (or at least I) did not expect
>>>>> UTCDateAndTime to be slower on 32-bit images. In fact, when I measured
>>>>> it (quite a long time ago now) it was faster on 32-bit images, and faster
>>>>> yet on 64-bit images. But my benchmarks were trivial, and Chris is
>>>>> considering use cases that involve IO to disk, and those might have
>>>>> very different performance considerations.
>>>>>
>>>>> The simple performance checks that I was using can be found in
>>>>> package Tests-UTCDateAndTime in the www.squeaksource.com/UTCDateAndTime
>>>>> repository. I can't look at it now, but those tests might be a useful
>>>>> reference here.
>>>>>
>>>>> Dave
>>>>>
>>>>>> On Wed, Jan 09, 2019 at 10:38:04PM -0600, Chris Muller wrote:
>>>>>> -1.  DateAndTime is an object of which there are often many instances.
>>>>>> Adding another inst var increases size and processing to spin through
>>>>>> yet one more variable during disk-read, transmission and serialization
>>>>>> and materialization.  It's pure cost for 64-bit images, since there
>>>>>> isn't any LI arithmetic there anyway, and *we knew* going in that
>>>>>> UTCDateAndTime would be slower in 32-bit.  Avoiding LI arithmetic in
>>>>>> 32-bit is what the original Chronology did....
>>>>>>
>>>>>> Regards,
>>>>>> Chris
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On Wed, Jan 9, 2019 at 8:53 PM <[hidden email]> wrote:
>>>>>>>
>>>>>>> A new version of Chronology-Core was added to project The Inbox:
>>>>>>> http://source.squeak.org/inbox/Chronology-Core-eem.22.mcz
>>>>>>>
>>>>>>> ==================== Summary ====================
>>>>>>>
>>>>>>> Name: Chronology-Core-eem.22
>>>>>>> Author: eem
>>>>>>> Time: 9 January 2019, 6:53:52.265064 pm
>>>>>>> UUID: 348c4ad7-0a27-4021-8788-9e3ec220df24
>>>>>>> Ancestors: Chronology-Core-ul.21
>>>>>>>
>>>>>>> Simple speedup by caching the result of getSeconds in an inst var.  Lots of other values are defined in terms of getSeconds so this simply eliminates duplicating the large integer arithmetic in getSeconds.
>>>>>>>
>>>>>>> =============== Diff against Chronology-Core-ul.21 ===============
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> Magnitude subclass: #DateAndTime
>>>>>>> +       instanceVariableNames: 'utcMicroseconds localOffsetSeconds seconds'
>>>>>>> -       instanceVariableNames: 'utcMicroseconds localOffsetSeconds'
>>>>>>>       classVariableNames: 'AutomaticTimezone ClockProvider InitializeFromPrimitive LocalTimeZone PosixEpochJulianDays'
>>>>>>>       poolDictionaries: 'ChronologyConstants'
>>>>>>>       category: 'Chronology-Core'!
>>>>>>>
>>>>>>> + !DateAndTime commentStamp: 'eem 1/9/2019 18:44' prior: 0!
>>>>>>> - !DateAndTime commentStamp: 'dtl 3/12/2016 10:32' prior: 0!
>>>>>>> I represent a point in UTC time as defined by ISO 8601. I have zero duration.
>>>>>>>
>>>>>>> + My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.  I cache the result of getSeconds in the seconds variable that is computed when required.
>>>>>>> - My implementation uses variables utcMicroseconds and localOffsetSeconds. This represents time magnitude as elapsed microseconds since the Posix epoch, with localOffsetSeconds representing local offset from UTC. The magnitude is used for comparison and duration calculations, and the local offset is used for displaying this magnitude in the context of a local time zone.
>>>>>>>
>>>>>>> The implementation ignores leap seconds, which are adjustments made to maintain earth rotational clock time in synchronization with elapsed seconds.
>>>>>>>
>>>>>>> DateAndTime class>>now will use #primitiveUtcWithOffset to obtain current time in UTC microseconds with current local offset in seconds. The primitive provides an atomic query for UTC time and local offset as measured by the OS platform.  If primitiveUtcWithOffset is not available, the traditional implementation is used, which relies on a primitive for microseconds in the local time zone and derives UTC based on the TimeZone setting.
>>>>>>> !
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: DateAndTime>>getSeconds (in category 'accessing') -----
>>>>>>> getSeconds
>>>>>>> -
>>>>>>>       | posixDays posixSeconds localSeconds |
>>>>>>> +       seconds ifNil:
>>>>>>> +               [posixSeconds := utcMicroseconds // 1000000.
>>>>>>> +                localSeconds := posixSeconds + localOffsetSeconds.
>>>>>>> +                localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
>>>>>>> +                posixDays := localSeconds // SecondsInDay.
>>>>>>> +                seconds := localSeconds - (posixDays * SecondsInDay)].
>>>>>>> +       ^seconds!
>>>>>>> -       posixSeconds := utcMicroseconds // 1000000.
>>>>>>> -       localSeconds := posixSeconds + localOffsetSeconds.
>>>>>>> -       localSeconds < 0 ifTrue: [localSeconds := localSeconds \\ SecondsInDay]. "normalize"
>>>>>>> -       posixDays := localSeconds // SecondsInDay.
>>>>>>> -       ^localSeconds - (posixDays * SecondsInDay).
>>>>>>> - !
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: DateAndTime>>readDataFrom:size: (in category 'objects from disk') -----
>>>>>>> readDataFrom: aDataStream size: varsOnDisk
>>>>>>>       "Fill in the fields of self based on the contents of aDataStream. The serialized
>>>>>>>       data will have four instance variables, because all instances are serialized in a
>>>>>>>       cononical format as if having originating from an instance with the traditional
>>>>>>>       seconds/offset/jdn/nanos instance variables."
>>>>>>>
>>>>>>>       | seconds offset jdn nanos |
>>>>>>>       seconds := aDataStream next.
>>>>>>>       offset := aDataStream next.
>>>>>>>       jdn := aDataStream next.
>>>>>>>       nanos := aDataStream next.
>>>>>>>       localOffsetSeconds := offset ifNil: [ 0 ] ifNotNil: [ :off | off asSeconds ].
>>>>>>>       utcMicroseconds := self
>>>>>>>                               microsecondsFromDay: jdn
>>>>>>>                               seconds: seconds
>>>>>>>                               nanos: nanos
>>>>>>> +                               offset: localOffsetSeconds..
>>>>>>> +       seconds := nil!
>>>>>>> -                               offset: localOffsetSeconds.!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: DateAndTime>>setJdn:seconds:nano:localOffsetSeconds: (in category 'private') -----
>>>>>>> setJdn: jdn seconds: s nano: n localOffsetSeconds: offset
>>>>>>>
>>>>>>>       localOffsetSeconds := offset.
>>>>>>>       utcMicroseconds := self
>>>>>>>                               microsecondsFromDay: jdn
>>>>>>>                               seconds: s
>>>>>>>                               nanos: n
>>>>>>> +                               offset: offset.
>>>>>>> +       seconds := nil!
>>>>>>> -                               offset: offset!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: DateAndTime>>ticks:offset: (in category 'private') -----
>>>>>>> ticks: ticks offset: utcOffset
>>>>>>>       "ticks is {julianDayNumber. secondCount. nanoSeconds}"
>>>>>>>
>>>>>>>       | jdn s nanos |
>>>>>>>       self normalize: 3 ticks: ticks base: NanosInSecond.
>>>>>>>       self normalize: 2 ticks: ticks base: SecondsInDay.
>>>>>>>
>>>>>>>       jdn     := ticks at: 1.
>>>>>>>       s := ticks at: 2.
>>>>>>>       nanos := ticks at: 3.
>>>>>>>       localOffsetSeconds := utcOffset ifNil: [0] ifNotNil: [utcOffset asSeconds].
>>>>>>>       utcMicroseconds := self microsecondsFromDay: jdn seconds: s nanos: nanos offset: localOffsetSeconds.
>>>>>>> +       seconds := nil!
>>>>>>> - !
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: DateAndTime>>utcMicroseconds: (in category 'initialize-release') -----
>>>>>>> utcMicroseconds: utcValue
>>>>>>>       "Allow value to be modified during initialization from a primitive in order to support
>>>>>>>       monotonically increasing clock behavior."
>>>>>>> +       utcMicroseconds := utcValue.
>>>>>>> +       seconds := nil!
>>>>>>> -       utcMicroseconds := utcValue!
>>>>>>>
>>>>>>> Item was changed:
>>>>>>> ----- Method: DateAndTime>>utcMicroseconds:offset: (in category 'initialize-release') -----
>>>>>>> utcMicroseconds: microsecondsSincePosixEpoch offset: tzOffset
>>>>>>>
>>>>>>>       utcMicroseconds := microsecondsSincePosixEpoch.
>>>>>>>       localOffsetSeconds := tzOffset.
>>>>>>> +       seconds := nil
>>>>>>> !
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

David T. Lewis
In reply to this post by Eliot Miranda-2
On Fri, Jan 11, 2019 at 11:29:53AM -0800, Eliot Miranda wrote:

> Hi David,
>
> > On Jan 11, 2019, at 11:15 AM, David T. Lewis <[hidden email]> wrote:
> >
> >> On Fri, Jan 11, 2019 at 09:43:03AM -0800, Eliot Miranda wrote:
> >>
> >>
> >>> On Jan 10, 2019, at 6:20 PM, David T. Lewis <[hidden email]> wrote:
> >>>
> >>> This is a good optimization. After taking a brief look at the performance
> >>> numbers, I can see a significant (almost 2X) improvement in DateAndTime>>asTime.
> >>
> >> In 32-bit or 64-bit?
> >
> > This is on 64-bit, I did not check 32-bit.
>
> Can you post the test?  I???d like to measure 32-bits where the difference should be larger.

Ugh, I cannot even reproduce my own results, so now I am not sure if there
is a performance difference or not.

I was comparing performance on a 64-bit Spur image with Chronology-Core-eem.22
loaded (optimized version), versus the same image with Chronology-Core-ul.21
loaded (no optimization).

I started by running this (from the http://www.squeaksource.com/UTCDateAndTime
repository):

  LXTestDateAndTimePerformance new test

Then I looked for senders of #asSeconds and tried a couple of those in big
loops. For #asTime, I compared the two Chronolgy-Core versions using this:

  Time millisecondsToRun: [ 100000000 timesRepeat: [DateAndTime now asTime]]

I thought that I was seeing a big performance difference, but I may have been
getting fooled by jit warmup times and so forth. It was just a quick test,
and I did not take the time to make sure it was reproducible.

Obviously these are very naive tests. I thought I was seeing a big performance
difference in the #asTime loop, but now I am not sure.

Dave


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

David T. Lewis
In reply to this post by Levente Uzonyi
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 tested with this:

  dt := DateAndTime now.
  Time millisecondsToRun: [ 100000000 timesRepeat: [dt getSeconds ]].

Dave


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

Levente Uzonyi
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.


Levente

>
> Dave

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

David T. Lewis
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 :-)

Thanks,
Dave


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

Levente Uzonyi
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.

Levente

>
> Thanks,
> Dave

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

David T. Lewis
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
 

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-eem.22.mcz

Eliot Miranda-2
In reply to this post by Levente Uzonyi
Hi Levente,

> On Jan 12, 2019, at 4:56 AM, Levente Uzonyi <[hidden email]> 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 trust you, man ;-). Go ahead and commit what makes sense to you.

>
> Levente
>
>>
>> Thanks,
>> Dave
>