Hi Levente,

Thank you for the review and for your updates. Printing dates seems
to be almost 50% faster now with your updates.

Dave

On Sun, Mar 20, 2016 at 12:39:38AM +0100, Levente Uzonyi wrote:
> Hi Dave,
> 
> I finally had some time to dig into the code and the tests.
> 
> On Sun, 13 Mar 2016, David T. Lewis wrote:
> 
> >On Sat, Mar 12, 2016 at 08:48:21PM +0100, Levente Uzonyi wrote:
> >>Hi Dave,
> >>
> >>I ran the tests and found that there are three types of failures.
> >>1. Some tests assume that the default offset is #localOffset. I don't
> >>think it's correct, but that's how is it in Trunk.
> >
> >There are 10 failing tests in my image. These are cases in which either
> >I was uncertain how to handle the issue, or where I thought that the
> >current behaviour in trunk may be wrong.
> >
> >>2. Hashes have changed. This implies that HashedCollections containing
> >>DateAndTime instances as key will have to be rehashed. In the base Trunk
> >>image there are appear to be no such HashedCollections, but not rehashing
> >>such collections will break images with custom packages in them after
> >>update.
> >
> >I am not expert in this area at all. But part of the update process
> >(in the MC postscripts) finds all instances of the old style DateAndTime,
> >and becomes them into the new ones. Would the HashedCollections contianing
> >DateAndTime instances as key still require rehash in that case?
> 
> Yes, it is. When their hash changes, these objects will not be found in 
> such collections.
> 
> >
> >Related to DateAndTime>>hash, I would also like to ask for guidance as
> >to how DateAndTime>>= should be defined. Maybe this is an ANSI Smalltalk
> >question, but I was not sure if we should be thinking of DateAndTime as
> >a magnitude, such that two instances are equal if their utcMicroseconds
> >are the same? Or are two instances equal if they have the same magnitude
> >and also the same localOffsetSeconds?
> >
> >I implemented the first approach (two instances are #= if they represent
> >the same UTC time, regardless of timezone). There may be use cases
> >where the other approach makes sense. But my thinking was that when
> >comparing two instances of DateAndTime, they should not be considered
> >the same unless they both represent the same point in time, hence UTC
> >time regardless of the time zone in which the instance was created.
> 
> As we discussed this at the last board meeting, the right thing is to use 
> UTC time only both for #hash and #=, so this is correct.
> 
> >
> >>3. Loss of resolution and strict monotonicity. In Trunk [DateAndTime now <
> >>DateAndTime now] is always true. This is possible because DateAndTime has
> >>nanosecond resolution, but the source clock only has microsecond
> >>resolution, and our machines are so slow that it takes more than a
> >>nanosecond to execute the primitive. All these allow us to use the
> >>sub-microsecond part of the timestamp to create intermediate strictly
> >>monotonic values.
> >>
> >
> >To be honest, I do not fully recall the rationale for this. One
> >important case is that of the system clock being set back due to
> >ntp synchronization. Fiddling with the nanoseconds number to make
> >it appear as if the OS had reported an increase in time (when in
> >fact it did not) seems questionable to me, but I know that it was
> >done for some good reason. I do remember the discussions, so I
> >should look back through the squeak-dev archives and find out the
> >reason this was done.
> 
> It comes handy in many situations where uniqueness is a must. E.g. 
> creating file names, storing them in a database. I don't know why it was 
> added originally.
> 
> The loss of the nanosecond part has another side effect. utcMicroseconds 
> will be a Fraction when the resolution of the parsed input is too high. 
> This is somewhat compatible, but it makes things slower. E.g.:
> 
> '2002-05-16T17:20:45.000000009+01:01' asDateAndTime utcMicroseconds
> "==> (1021565985000000009/1000)"
> 
> I found two other issues:
> 
> DateAndTime class >> #date:time: ignores the nanoseconds from the time 
> argument. This is present in the Trunk as well, but no test fails, because 
> in Trunk TimeStamps have their nanoseconds part cleared (in a very 
> inefficient way).
> 
> The other issue is that DateAndTime >> #getSeconds returns a fraction, 
> which contains the nanosecond part as well. This makes DateAndTime >> 
> #asTime return a Time instance which has its nanoseconds part doubled.
> I simply changed #/ to #// in #getSeconds to work that around. This 
> also satisfied the expectations of all other senders of #getSeconds, but 
> then it turned out to be the same as #getWholeSeconds.
> Since #getSeconds is a replacement for the original seconds variable (if 
> I'm not mistaken), I think it's safe to do this change and replace sends 
> of #getWholeSeconds to #getSeconds.
> 
> I found that I'm a developer on the project, so I have uploaded a new 
> version with the changes described above + some optimizations.
> 
> About some #FIXMEs:
> #offset: is plain wrong according to its comment. It should simply create 
> a copy with the new offset and the same utcMicrosecond value. Just like 
> how #utcOffset: does. We should keep one or the other but not both.
> 
> Levente
> 
> >
> >>Levente
> >>
> >
> >Thanks very much for looking at this. I added you and Chris Muller as
> >developers on the UTCDateAndTime repository on squeaksource.com in case
> >you want to make any changes.
> >
> >Much appreciated,
> >
> >Dave
> >
> >
> >