The Inbox: Chronology-Core-cmm.56.mcz

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

The Inbox: Chronology-Core-cmm.56.mcz

commits-2
Chris Muller uploaded a new version of Chronology-Core to project The Inbox:
http://source.squeak.org/inbox/Chronology-Core-cmm.56.mcz

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

Name: Chronology-Core-cmm.56
Author: cmm
Time: 4 May 2020, 6:42:27.516151 pm
UUID: 3c97bedc-c192-4872-b0f2-e93199f506ce
Ancestors: Chronology-Core-ul.54

Building on Chronology-Core-ul.54:

- Let #microsecondClockValue complement #millisecondClockValue, to offer higher precision.
- New constructor, DateAndTime class>>#utcMicroseconds:, complements the above.
- Make Time class>>#utcMicrosecondClock private to help avoid accidental improper use with the above.

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

Item was added:
+ ----- Method: DateAndTime class>>microsecondClockValue (in category 'smalltalk-80') -----
+ microsecondClockValue
+ ^ self clock microsecondClockValue!

Item was added:
+ ----- Method: DateAndTime class>>utcMicroseconds: (in category 'instance creation') -----
+ utcMicroseconds: microsecondClockValue
+ "Instantiate a DateAndTime in UTC from the value of my #microsecondClockValue."
+ ^ super new
+ utcMicroseconds: microsecondClockValue
+ offset: 0!

Item was changed:
  ----- Method: Time class>>estimateHighResClockTicksPerMillisecond (in category 'clock') -----
  estimateHighResClockTicksPerMillisecond
 
  | t0 t1 t2 t3 |
 
  "Count the ticks ellapsed during a 10ms busy loop"
+ t0 := Time microsecondClockValue + 200.
+ [Time microsecondClockValue >= t0] whileFalse.
- t0 := Time utcMicrosecondClock + 200.
- [Time utcMicrosecondClock >= t0] whileFalse.
  t1 := self highResClock.
+ [Time microsecondClockValue >= (t0 + 10000)] whileFalse.
+ t1 := self highResClock - t1 * 1000 // (Time microsecondClockValue - t0).
- [Time utcMicrosecondClock >= (t0 + 10000)] whileFalse.
- t1 := self highResClock - t1 * 1000 // (Time utcMicrosecondClock - t0).
 
  "Count the ticks ellapsed during a 20ms busy loop"
+ t0 := Time microsecondClockValue + 200.
+ [Time microsecondClockValue >= t0] whileFalse.
- t0 := Time utcMicrosecondClock + 200.
- [Time utcMicrosecondClock >= t0] whileFalse.
  t2 := self highResClock.
+ [Time microsecondClockValue >= (t0 + 20000)] whileFalse.
+ t2 := self highResClock - t2 * 1000 // (Time microsecondClockValue - t0).
- [Time utcMicrosecondClock >= (t0 + 20000)] whileFalse.
- t2 := self highResClock - t2 * 1000 // (Time utcMicrosecondClock - t0).
 
  "Count the ticks ellapsed during a 30ms busy loop"
+ t0 := Time microsecondClockValue + 200.
+ [Time microsecondClockValue >= t0] whileFalse.
- t0 := Time utcMicrosecondClock + 200.
- [Time utcMicrosecondClock >= t0] whileFalse.
  t3 := self highResClock.
+ [Time microsecondClockValue >= (t0 + 30000)] whileFalse.
+ t3 := self highResClock - t3 * 1000 // (Time microsecondClockValue - t0).
- [Time utcMicrosecondClock >= (t0 + 30000)] whileFalse.
- t3 := self highResClock - t3 * 1000 // (Time utcMicrosecondClock - t0).
 
  "Take the median of the 3 estimates as the best"
  ^ t1 <= t2
  ifTrue: [t2 <= t3
  ifTrue: [t2]
  ifFalse: [t1 <= t3
  ifTrue: [t3]
  ifFalse: [t1]]]
  ifFalse: [t1 <= t3
  ifTrue: [t1]
  ifFalse: [t2 <= t3
  ifTrue: [t3]
  ifFalse: [t2]]]!

Item was added:
+ ----- Method: Time class>>microsecondClockValue (in category 'general inquiries') -----
+ microsecondClockValue
+ "Answer the value of the microsecond clock, the number of microseconds elapsed since 1/1/1901 @ 00:00:00."
+ ^ self utcMicrosecondClock - MicrosecondsBetweenPosixEpochAndSqueakEpoch!

Item was changed:
  ----- Method: Time class>>microsecondsToRun: (in category 'general inquiries') -----
  microsecondsToRun: timedBlock
  "Answer the number of microseconds timedBlock takes to return its value."
 
  | startUsecs |
  (self useHighResClockForTiming and: [self highResClock ~= 0])
  ifTrue: [ ^(self nanosecondsToRunHighRes: timedBlock) + 500 // 1000].
+ startUsecs := self microsecondClockValue.
- startUsecs := self utcMicrosecondClock.
  timedBlock value.
+ ^self microsecondClockValue - startUsecs!
- ^self utcMicrosecondClock - startUsecs!

Item was changed:
  ----- Method: Time class>>millisecondClockValue (in category 'general inquiries') -----
  millisecondClockValue
  "Answer the value of the millisecond clock."
+ ^self microsecondClockValue // 1000!
-
- ^self utcMicrosecondClock // 1000!

Item was changed:
  ----- Method: Time class>>nanosecondsToRun: (in category 'general inquiries') -----
  nanosecondsToRun: timedBlock
  "Answer the number of nanoseconds timedBlock takes to return its value.
  Use high resolution clock if available and preferred."
 
  | startUsecs |
  (self useHighResClockForTiming and: [self highResClock ~= 0])
  ifTrue: [ ^(self nanosecondsToRunHighRes: timedBlock)].
  "Fallback to microseconds clock"
+ startUsecs := self microsecondClockValue.
- startUsecs := self utcMicrosecondClock.
  timedBlock value.
+ ^self microsecondClockValue - startUsecs * 1000!
- ^self utcMicrosecondClock - startUsecs * 1000!

Item was removed:
- ----- Method: Time class>>posixUtcMicrosecondClock (in category 'clock') -----
- posixUtcMicrosecondClock
- "Answer the UTC microseconds since the POSIX epoch (January 1st 1970 00:00:00 UTC)."
-
- ^self utcMicrosecondClock - MicrosecondsBetweenPosixEpochAndSqueakEpoch!

Item was changed:
+ ----- Method: Time class>>utcMicrosecondClock (in category 'private') -----
- ----- Method: Time class>>utcMicrosecondClock (in category 'clock') -----
  utcMicrosecondClock
  "Answer the UTC microseconds since the Smalltalk epoch (January 1st 1901, the start of the 20th century).
  The value is derived from the Posix epoch with a constant offset corresponding to elapsed microseconds
  between the two epochs according to RFC 868."
  <primitive: 240>
  ^0!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-cmm.56.mcz

Chris Muller-3
Hi Levente,

This trades the "posix" nomenclature in the API for #microsecondClockValue.  "Posix" is a private matter to Chronology's implementation that should not be exposed in the API.

 - Chris


On Mon, May 4, 2020 at 6:42 PM <[hidden email]> wrote:
Chris Muller uploaded a new version of Chronology-Core to project The Inbox:
http://source.squeak.org/inbox/Chronology-Core-cmm.56.mcz

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

Name: Chronology-Core-cmm.56
Author: cmm
Time: 4 May 2020, 6:42:27.516151 pm
UUID: 3c97bedc-c192-4872-b0f2-e93199f506ce
Ancestors: Chronology-Core-ul.54

Building on Chronology-Core-ul.54:

- Let #microsecondClockValue complement #millisecondClockValue, to offer higher precision.
- New constructor, DateAndTime class>>#utcMicroseconds:, complements the above.
- Make Time class>>#utcMicrosecondClock private to help avoid accidental improper use with the above.

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

Item was added:
+ ----- Method: DateAndTime class>>microsecondClockValue (in category 'smalltalk-80') -----
+ microsecondClockValue
+       ^ self clock microsecondClockValue!

Item was added:
+ ----- Method: DateAndTime class>>utcMicroseconds: (in category 'instance creation') -----
+ utcMicroseconds: microsecondClockValue
+       "Instantiate a DateAndTime in UTC from the value of my #microsecondClockValue."
+       ^ super new
+               utcMicroseconds: microsecondClockValue
+               offset: 0!

Item was changed:
  ----- Method: Time class>>estimateHighResClockTicksPerMillisecond (in category 'clock') -----
  estimateHighResClockTicksPerMillisecond

        | t0 t1 t2 t3 |

        "Count the ticks ellapsed during a 10ms busy loop"
+       t0 := Time microsecondClockValue + 200.
+       [Time microsecondClockValue >= t0] whileFalse.
-       t0 := Time utcMicrosecondClock + 200.
-       [Time utcMicrosecondClock >= t0] whileFalse.
        t1 := self highResClock.
+       [Time microsecondClockValue >= (t0 + 10000)] whileFalse.
+       t1 := self highResClock - t1 * 1000 // (Time microsecondClockValue - t0).
-       [Time utcMicrosecondClock >= (t0 + 10000)] whileFalse.
-       t1 := self highResClock - t1 * 1000 // (Time utcMicrosecondClock - t0).

        "Count the ticks ellapsed during a 20ms busy loop"
+       t0 := Time microsecondClockValue + 200.
+       [Time microsecondClockValue >= t0] whileFalse.
-       t0 := Time utcMicrosecondClock + 200.
-       [Time utcMicrosecondClock >= t0] whileFalse.
        t2 := self highResClock.
+       [Time microsecondClockValue >= (t0 + 20000)] whileFalse.
+       t2 := self highResClock - t2 * 1000 // (Time microsecondClockValue - t0).
-       [Time utcMicrosecondClock >= (t0 + 20000)] whileFalse.
-       t2 := self highResClock - t2 * 1000 // (Time utcMicrosecondClock - t0).

        "Count the ticks ellapsed during a 30ms busy loop"
+       t0 := Time microsecondClockValue + 200.
+       [Time microsecondClockValue >= t0] whileFalse.
-       t0 := Time utcMicrosecondClock + 200.
-       [Time utcMicrosecondClock >= t0] whileFalse.
        t3 := self highResClock.
+       [Time microsecondClockValue >= (t0 + 30000)] whileFalse.
+       t3 := self highResClock - t3 * 1000 // (Time microsecondClockValue - t0).
-       [Time utcMicrosecondClock >= (t0 + 30000)] whileFalse.
-       t3 := self highResClock - t3 * 1000 // (Time utcMicrosecondClock - t0).

        "Take the median of the 3 estimates as the best"
        ^ t1 <= t2
                ifTrue: [t2 <= t3
                                ifTrue: [t2]
                                ifFalse: [t1 <= t3
                                                ifTrue: [t3]
                                                ifFalse: [t1]]]
                ifFalse: [t1 <= t3
                                ifTrue: [t1]
                                ifFalse: [t2 <= t3
                                                ifTrue: [t3]
                                                ifFalse: [t2]]]!

Item was added:
+ ----- Method: Time class>>microsecondClockValue (in category 'general inquiries') -----
+ microsecondClockValue
+       "Answer the value of the microsecond clock, the number of microseconds elapsed since 1/1/1901 @ 00:00:00."
+       ^ self utcMicrosecondClock - MicrosecondsBetweenPosixEpochAndSqueakEpoch!

Item was changed:
  ----- Method: Time class>>microsecondsToRun: (in category 'general inquiries') -----
  microsecondsToRun: timedBlock
        "Answer the number of microseconds timedBlock takes to return its value."

        | startUsecs |
        (self useHighResClockForTiming and: [self highResClock ~= 0])
                ifTrue: [       ^(self nanosecondsToRunHighRes: timedBlock) + 500 // 1000].
+       startUsecs := self microsecondClockValue.
-       startUsecs := self utcMicrosecondClock.
        timedBlock value.
+       ^self microsecondClockValue - startUsecs!
-       ^self utcMicrosecondClock - startUsecs!

Item was changed:
  ----- Method: Time class>>millisecondClockValue (in category 'general inquiries') -----
  millisecondClockValue
        "Answer the value of the millisecond clock."
+       ^self microsecondClockValue // 1000!
-
-       ^self utcMicrosecondClock // 1000!

Item was changed:
  ----- Method: Time class>>nanosecondsToRun: (in category 'general inquiries') -----
  nanosecondsToRun: timedBlock
        "Answer the number of nanoseconds timedBlock takes to return its value.
        Use high resolution clock if available and preferred."

        | startUsecs |
        (self useHighResClockForTiming and: [self highResClock ~= 0])
                ifTrue: [       ^(self nanosecondsToRunHighRes: timedBlock)].
        "Fallback to microseconds clock"
+       startUsecs := self microsecondClockValue.
-       startUsecs := self utcMicrosecondClock.
        timedBlock value.
+       ^self microsecondClockValue - startUsecs * 1000!
-       ^self utcMicrosecondClock - startUsecs * 1000!

Item was removed:
- ----- Method: Time class>>posixUtcMicrosecondClock (in category 'clock') -----
- posixUtcMicrosecondClock
-       "Answer the UTC microseconds since the POSIX epoch (January 1st 1970 00:00:00 UTC)."
-
-       ^self utcMicrosecondClock - MicrosecondsBetweenPosixEpochAndSqueakEpoch!

Item was changed:
+ ----- Method: Time class>>utcMicrosecondClock (in category 'private') -----
- ----- Method: Time class>>utcMicrosecondClock (in category 'clock') -----
  utcMicrosecondClock
        "Answer the UTC microseconds since the Smalltalk epoch (January 1st 1901, the start of the 20th century).
         The value is derived from the Posix epoch with a constant offset corresponding to elapsed microseconds
         between the two epochs according to RFC 868."
        <primitive: 240>
        ^0!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-cmm.56.mcz

Levente Uzonyi
Hi Chris,

On Mon, 4 May 2020, Chris Muller wrote:

> Hi Levente,
> This trades the "posix" nomenclature in the API for #microsecondClockValue.  "Posix" is a private matter to Chronology's implementation that should not be exposed in the API.

"Posix" has been there since 2016: Time class >> posixMicrosecondClockWithOffset:.
Everyone gets an idea what kind of timestamp a method with posix in its name will return.
The same does not apply to #microsecondClockValue, does it?

In my opinion exposing "Posix" has benefits.
So, -1 to these changes. Details below.

>
>  - Chris
>
>
> On Mon, May 4, 2020 at 6:42 PM <[hidden email]> wrote:
>       Chris Muller uploaded a new version of Chronology-Core to project The Inbox:
>       http://source.squeak.org/inbox/Chronology-Core-cmm.56.mcz
>
>       ==================== Summary ====================
>
>       Name: Chronology-Core-cmm.56
>       Author: cmm
>       Time: 4 May 2020, 6:42:27.516151 pm
>       UUID: 3c97bedc-c192-4872-b0f2-e93199f506ce
>       Ancestors: Chronology-Core-ul.54
>
>       Building on Chronology-Core-ul.54:
>
>       - Let #microsecondClockValue complement #millisecondClockValue, to offer higher precision.
>       - New constructor, DateAndTime class>>#utcMicroseconds:, complements the above.
>       - Make Time class>>#utcMicrosecondClock private to help avoid accidental improper use with the above.
>
>       =============== Diff against Chronology-Core-ul.54 ===============
>
>       Item was added:
>       + ----- Method: DateAndTime class>>microsecondClockValue (in category 'smalltalk-80') -----
>       + microsecondClockValue
>       +       ^ self clock microsecondClockValue!
Why do we need this here? Especially in that category. This is clearly
not part of any smalltalk-80 protocol.
DateAndTime class >> ##millisecondClockValue has no senders, so we should
remove that method instead of adding another unused one.

>
>       Item was added:
>       + ----- Method: DateAndTime class>>utcMicroseconds: (in category 'instance creation') -----
>       + utcMicroseconds: microsecondClockValue
>       +       "Instantiate a DateAndTime in UTC from the value of my #microsecondClockValue."
>       +       ^ super new
>       +               utcMicroseconds: microsecondClockValue
>       +               offset: 0!
>
>       Item was changed:
>         ----- Method: Time class>>estimateHighResClockTicksPerMillisecond (in category 'clock') -----
>         estimateHighResClockTicksPerMillisecond
>
>               | t0 t1 t2 t3 |
>
>               "Count the ticks ellapsed during a 10ms busy loop"
>       +       t0 := Time microsecondClockValue + 200.
>       +       [Time microsecondClockValue >= t0] whileFalse.
>       -       t0 := Time utcMicrosecondClock + 200.
>       -       [Time utcMicrosecondClock >= t0] whileFalse.
>               t1 := self highResClock.
>       +       [Time microsecondClockValue >= (t0 + 10000)] whileFalse.
>       +       t1 := self highResClock - t1 * 1000 // (Time microsecondClockValue - t0).
>       -       [Time utcMicrosecondClock >= (t0 + 10000)] whileFalse.
>       -       t1 := self highResClock - t1 * 1000 // (Time utcMicrosecondClock - t0).
The method uses the primitive directly to make the measurements as
accurate as possible. This change would make them less accurate.

>
>               "Count the ticks ellapsed during a 20ms busy loop"
>       +       t0 := Time microsecondClockValue + 200.
>       +       [Time microsecondClockValue >= t0] whileFalse.
>       -       t0 := Time utcMicrosecondClock + 200.
>       -       [Time utcMicrosecondClock >= t0] whileFalse.
>               t2 := self highResClock.
>       +       [Time microsecondClockValue >= (t0 + 20000)] whileFalse.
>       +       t2 := self highResClock - t2 * 1000 // (Time microsecondClockValue - t0).
>       -       [Time utcMicrosecondClock >= (t0 + 20000)] whileFalse.
>       -       t2 := self highResClock - t2 * 1000 // (Time utcMicrosecondClock - t0).
>
>               "Count the ticks ellapsed during a 30ms busy loop"
>       +       t0 := Time microsecondClockValue + 200.
>       +       [Time microsecondClockValue >= t0] whileFalse.
>       -       t0 := Time utcMicrosecondClock + 200.
>       -       [Time utcMicrosecondClock >= t0] whileFalse.
>               t3 := self highResClock.
>       +       [Time microsecondClockValue >= (t0 + 30000)] whileFalse.
>       +       t3 := self highResClock - t3 * 1000 // (Time microsecondClockValue - t0).
>       -       [Time utcMicrosecondClock >= (t0 + 30000)] whileFalse.
>       -       t3 := self highResClock - t3 * 1000 // (Time utcMicrosecondClock - t0).
>
>               "Take the median of the 3 estimates as the best"
>               ^ t1 <= t2
>                       ifTrue: [t2 <= t3
>                                       ifTrue: [t2]
>                                       ifFalse: [t1 <= t3
>                                                       ifTrue: [t3]
>                                                       ifFalse: [t1]]]
>                       ifFalse: [t1 <= t3
>                                       ifTrue: [t1]
>                                       ifFalse: [t2 <= t3
>                                                       ifTrue: [t3]
>                                                       ifFalse: [t2]]]!
>
>       Item was added:
>       + ----- Method: Time class>>microsecondClockValue (in category 'general inquiries') -----
>       + microsecondClockValue
>       +       "Answer the value of the microsecond clock, the number of microseconds elapsed since 1/1/1901 @ 00:00:00."
Wrong date there.

>       +       ^ self utcMicrosecondClock - MicrosecondsBetweenPosixEpochAndSqueakEpoch!
>
>       Item was changed:
>         ----- Method: Time class>>microsecondsToRun: (in category 'general inquiries') -----
>         microsecondsToRun: timedBlock
>               "Answer the number of microseconds timedBlock takes to return its value."
>
>               | startUsecs |
>               (self useHighResClockForTiming and: [self highResClock ~= 0])
>                       ifTrue: [       ^(self nanosecondsToRunHighRes: timedBlock) + 500 // 1000].
>       +       startUsecs := self microsecondClockValue.
>       -       startUsecs := self utcMicrosecondClock.
>               timedBlock value.
>       +       ^self microsecondClockValue - startUsecs!
>       -       ^self utcMicrosecondClock - startUsecs!
Again, decreases the measurement's accuracy.

>
>       Item was changed:
>         ----- Method: Time class>>millisecondClockValue (in category 'general inquiries') -----
>         millisecondClockValue
>               "Answer the value of the millisecond clock."
>       +       ^self microsecondClockValue // 1000!
>       -
>       -       ^self utcMicrosecondClock // 1000!

After loading, this change would break code relying on previous results of
this method.

>
>       Item was changed:
>         ----- Method: Time class>>nanosecondsToRun: (in category 'general inquiries') -----
>         nanosecondsToRun: timedBlock
>               "Answer the number of nanoseconds timedBlock takes to return its value.
>               Use high resolution clock if available and preferred."
>
>               | startUsecs |
>               (self useHighResClockForTiming and: [self highResClock ~= 0])
>                       ifTrue: [       ^(self nanosecondsToRunHighRes: timedBlock)].
>               "Fallback to microseconds clock"
>       +       startUsecs := self microsecondClockValue.
>       -       startUsecs := self utcMicrosecondClock.
>               timedBlock value.
>       +       ^self microsecondClockValue - startUsecs * 1000!
>       -       ^self utcMicrosecondClock - startUsecs * 1000!
Accuracy again.

>
>       Item was removed:
>       - ----- Method: Time class>>posixUtcMicrosecondClock (in category 'clock') -----
>       - posixUtcMicrosecondClock
>       -       "Answer the UTC microseconds since the POSIX epoch (January 1st 1970 00:00:00 UTC)."
>       -
>       -       ^self utcMicrosecondClock - MicrosecondsBetweenPosixEpochAndSqueakEpoch!
>
>       Item was changed:
>       + ----- Method: Time class>>utcMicrosecondClock (in category 'private') -----
>       - ----- Method: Time class>>utcMicrosecondClock (in category 'clock') -----
Does this mean that I have to reimplement the primitive in my external
packages from now on?


Levente

>         utcMicrosecondClock
>               "Answer the UTC microseconds since the Smalltalk epoch (January 1st 1901, the start of the 20th century).
>                The value is derived from the Posix epoch with a constant offset corresponding to elapsed microseconds
>                between the two epochs according to RFC 868."
>               <primitive: 240>
>               ^0!
>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-cmm.56.mcz

Chris Muller-3
> This trades the "posix" nomenclature in the API for #microsecondClockValue.  "Posix" is a private matter to Chronology's implementation that should not be exposed in the API.

"Posix" has been there since 2016: Time class >> posixMicrosecondClockWithOffset:.

We should work to remove them for the reason stated.
 
Everyone gets an idea what kind of timestamp a method with posix in its name will return.

What are the possible "kinds"?
 
The same does not apply to #microsecondClockValue, does it?

If "kind" refers to a timezone indication, then yes, it does (but "posix" doesn't).  Again, I ask you to zoom out and look at the API as a whole, instead of only individual methods out-of-context of the broader "language" of the API.
 
In my opinion exposing "Posix" has benefits.

It sounds like you confused epoch with with timezone indication.  I explained why "posix" is negative, but you ignored it.  Isn't encapsulation important?  It seems you value a tight API with only as many concepts as necessary.  Why do you feel the API to to support two different epochs?

 
>       =============== Diff against Chronology-Core-ul.54 ===============
>
>       Item was added:
>       + ----- Method: DateAndTime class>>microsecondClockValue (in category 'smalltalk-80') -----
>       + microsecondClockValue
>       +       ^ self clock microsecondClockValue!

Why do we need this here?

Because you said:
   __________
   It is unfortunate that
   - #utcMicrosecondsClock is implemented as a class side method of Time,
   because it's unrelated to Time (renaming it to #microsecondsClock would
   not help with that)
   ___________

To which I agree.  The proper delegate of this responsibility is DateAndTime.  But it can still delegate to private methods on Time, so we could keep these on DateAndTime and move the ones on Time out of the public-API to private.
 
Especially in that category. This is clearly
not part of any smalltalk-80 protocol.

The category doesn't really matter, may we focus on the UX question please?
 
DateAndTime class >> ##millisecondClockValue has no senders, so we should
remove that method instead of adding another unused one.

That one's been there since 2006!   :-p    (sorry, couldn't resist...)
 
>       Item was added:
>       + ----- Method: DateAndTime class>>utcMicroseconds: (in category 'instance creation') -----
>       + utcMicroseconds: microsecondClockValue
>       +       "Instantiate a DateAndTime in UTC from the value of my #microsecondClockValue."
>       +       ^ super new
>       +               utcMicroseconds: microsecondClockValue
>       +               offset: 0!
>
>       Item was changed:
>         ----- Method: Time class>>estimateHighResClockTicksPerMillisecond (in category 'clock') -----
>         estimateHighResClockTicksPerMillisecond
>
>               | t0 t1 t2 t3 |
>
>               "Count the ticks ellapsed during a 10ms busy loop"
>       +       t0 := Time microsecondClockValue + 200.
>       +       [Time microsecondClockValue >= t0] whileFalse.
>       -       t0 := Time utcMicrosecondClock + 200.
>       -       [Time utcMicrosecondClock >= t0] whileFalse.
>               t1 := self highResClock.
>       +       [Time microsecondClockValue >= (t0 + 10000)] whileFalse.
>       +       t1 := self highResClock - t1 * 1000 // (Time microsecondClockValue - t0).
>       -       [Time utcMicrosecondClock >= (t0 + 10000)] whileFalse.
>       -       t1 := self highResClock - t1 * 1000 // (Time utcMicrosecondClock - t0).

The method uses the primitive directly to make the measurements as
accurate as possible. This change would make them less accurate.

How does it know it's using the primitive directly?  Is it depending on a known implementation and expecting it never to change?

If it wants / needs the efficiency of primitive, it should say so by calling "primSomethingSomething" then...

How many other primitive accessing methods are in the system without the "prim" prefix?

How cool it would be if Squeak users could simply type "prim" in a Message Names browser and instantly see just the methods that make up the "seam" between the image and the VM?  This user would love that!

Instead, #utcMicrosecondClock conceals that's its a primitive accessor, while being incompatible and unusable with any other API in the image.  It's actually quite horrible..
 

>
>               "Count the ticks ellapsed during a 20ms busy loop"
>       +       t0 := Time microsecondClockValue + 200.
>       +       [Time microsecondClockValue >= t0] whileFalse.
>       -       t0 := Time utcMicrosecondClock + 200.
>       -       [Time utcMicrosecondClock >= t0] whileFalse.
>               t2 := self highResClock.
>       +       [Time microsecondClockValue >= (t0 + 20000)] whileFalse.
>       +       t2 := self highResClock - t2 * 1000 // (Time microsecondClockValue - t0).
>       -       [Time utcMicrosecondClock >= (t0 + 20000)] whileFalse.
>       -       t2 := self highResClock - t2 * 1000 // (Time utcMicrosecondClock - t0).
>
>               "Count the ticks ellapsed during a 30ms busy loop"
>       +       t0 := Time microsecondClockValue + 200.
>       +       [Time microsecondClockValue >= t0] whileFalse.
>       -       t0 := Time utcMicrosecondClock + 200.
>       -       [Time utcMicrosecondClock >= t0] whileFalse.
>               t3 := self highResClock.
>       +       [Time microsecondClockValue >= (t0 + 30000)] whileFalse.
>       +       t3 := self highResClock - t3 * 1000 // (Time microsecondClockValue - t0).
>       -       [Time utcMicrosecondClock >= (t0 + 30000)] whileFalse.
>       -       t3 := self highResClock - t3 * 1000 // (Time utcMicrosecondClock - t0).
>
>               "Take the median of the 3 estimates as the best"
>               ^ t1 <= t2
>                       ifTrue: [t2 <= t3
>                                       ifTrue: [t2]
>                                       ifFalse: [t1 <= t3
>                                                       ifTrue: [t3]
>                                                       ifFalse: [t1]]]
>                       ifFalse: [t1 <= t3
>                                       ifTrue: [t1]
>                                       ifFalse: [t2 <= t3
>                                                       ifTrue: [t3]
>                                                       ifFalse: [t2]]]!
>
>       Item was added:
>       + ----- Method: Time class>>microsecondClockValue (in category 'general inquiries') -----
>       + microsecondClockValue
>       +       "Answer the value of the microsecond clock, the number of microseconds elapsed since 1/1/1901 @ 00:00:00."

Wrong date there.

This is just a draft for discussion.  I'm looking for basic consensus before investing a bunch of time...
 

>       +       ^ self utcMicrosecondClock - MicrosecondsBetweenPosixEpochAndSqueakEpoch!
>
>       Item was changed:
>         ----- Method: Time class>>microsecondsToRun: (in category 'general inquiries') -----
>         microsecondsToRun: timedBlock
>               "Answer the number of microseconds timedBlock takes to return its value."
>
>               | startUsecs |
>               (self useHighResClockForTiming and: [self highResClock ~= 0])
>                       ifTrue: [       ^(self nanosecondsToRunHighRes: timedBlock) + 500 // 1000].
>       +       startUsecs := self microsecondClockValue.
>       -       startUsecs := self utcMicrosecondClock.
>               timedBlock value.
>       +       ^self microsecondClockValue - startUsecs!
>       -       ^self utcMicrosecondClock - startUsecs!

Again, decreases the measurement's accuracy.

Perhaps, but we can also acknowledge this is yet another place where the value itself didn't matter, just the delta.  So, deprecation is not as disruptive as you thought.
 
>       Item was changed:
>         ----- Method: Time class>>millisecondClockValue (in category 'general inquiries') -----
>         millisecondClockValue
>               "Answer the value of the millisecond clock."
>       +       ^self microsecondClockValue // 1000!
>       -
>       -       ^self utcMicrosecondClock // 1000!

After loading, this change would break code relying on previous results of
this method.

There is no such code and, if there was, it was already broken because this has never been a reliable value.  It even used to roll over, remember?

That's why nothing broke when #utcMicrosecondClock went in and changed it...
 
>       Item was changed:
>         ----- Method: Time class>>nanosecondsToRun: (in category 'general inquiries') -----
>         nanosecondsToRun: timedBlock
>               "Answer the number of nanoseconds timedBlock takes to return its value.
>               Use high resolution clock if available and preferred."
>
>               | startUsecs |
>               (self useHighResClockForTiming and: [self highResClock ~= 0])
>                       ifTrue: [       ^(self nanosecondsToRunHighRes: timedBlock)].
>               "Fallback to microseconds clock"
>       +       startUsecs := self microsecondClockValue.
>       -       startUsecs := self utcMicrosecondClock.
>               timedBlock value.
>       +       ^self microsecondClockValue - startUsecs * 1000!
>       -       ^self utcMicrosecondClock - startUsecs * 1000!

Accuracy again.

Really, this is bad code then to be relying on a method's implementation not named "prim" to remain a primitive,  Intention-revealing code to achieve highest accuracy is achieved by writing "primUtcMicrosecondClock".
 
>       Item was removed:
>       - ----- Method: Time class>>posixUtcMicrosecondClock (in category 'clock') -----
>       - posixUtcMicrosecondClock
>       -       "Answer the UTC microseconds since the POSIX epoch (January 1st 1970 00:00:00 UTC)."
>       -
>       -       ^self utcMicrosecondClock - MicrosecondsBetweenPosixEpochAndSqueakEpoch!
>
>       Item was changed:
>       + ----- Method: Time class>>utcMicrosecondClock (in category 'private') -----
>       - ----- Method: Time class>>utcMicrosecondClock (in category 'clock') -----

Does this mean that I have to reimplement the primitive in my external
packages from now on?

No.

 - Chris 


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Chronology-Core-cmm.56.mcz

Tobias Pape
In reply to this post by Levente Uzonyi
What he says :)

> On 05.05.2020, at 03:17, Levente Uzonyi <[hidden email]> wrote:
>
> Hi Chris,
>
> On Mon, 4 May 2020, Chris Muller wrote:
>
>> Hi Levente,
>> This trades the "posix" nomenclature in the API for #microsecondClockValue.  "Posix" is a private matter to Chronology's implementation that should not be exposed in the API.
>
> "Posix" has been there since 2016: Time class >> posixMicrosecondClockWithOffset:.
> Everyone gets an idea what kind of timestamp a method with posix in its name will return.
> The same does not apply to #microsecondClockValue, does it?
>
> In my opinion exposing "Posix" has benefits.
> So, -1 to these changes. Details below.
>
>>  - Chris
>> On Mon, May 4, 2020 at 6:42 PM <[hidden email]> wrote:
>>      Chris Muller uploaded a new version of Chronology-Core to project The Inbox:
>>      http://source.squeak.org/inbox/Chronology-Core-cmm.56.mcz
>>
>>      ==================== Summary ====================
>>
>>      Name: Chronology-Core-cmm.56
>>      Author: cmm
>>      Time: 4 May 2020, 6:42:27.516151 pm
>>      UUID: 3c97bedc-c192-4872-b0f2-e93199f506ce
>>      Ancestors: Chronology-Core-ul.54
>>
>>      Building on Chronology-Core-ul.54:
>>
>>      - Let #microsecondClockValue complement #millisecondClockValue, to offer higher precision.
>>      - New constructor, DateAndTime class>>#utcMicroseconds:, complements the above.
>>      - Make Time class>>#utcMicrosecondClock private to help avoid accidental improper use with the above.
>>
>>      =============== Diff against Chronology-Core-ul.54 ===============
>>
>>      Item was added:
>>      + ----- Method: DateAndTime class>>microsecondClockValue (in category 'smalltalk-80') -----
>>      + microsecondClockValue
>>      +       ^ self clock microsecondClockValue!
>
> Why do we need this here? Especially in that category. This is clearly not part of any smalltalk-80 protocol.
> DateAndTime class >> ##millisecondClockValue has no senders, so we should remove that method instead of adding another unused one.
>
>>
>>      Item was added:
>>      + ----- Method: DateAndTime class>>utcMicroseconds: (in category 'instance creation') -----
>>      + utcMicroseconds: microsecondClockValue
>>      +       "Instantiate a DateAndTime in UTC from the value of my #microsecondClockValue."
>>      +       ^ super new
>>      +               utcMicroseconds: microsecondClockValue
>>      +               offset: 0!
>>
>>      Item was changed:
>>        ----- Method: Time class>>estimateHighResClockTicksPerMillisecond (in category 'clock') -----
>>        estimateHighResClockTicksPerMillisecond
>>
>>              | t0 t1 t2 t3 |
>>
>>              "Count the ticks ellapsed during a 10ms busy loop"
>>      +       t0 := Time microsecondClockValue + 200.
>>      +       [Time microsecondClockValue >= t0] whileFalse.
>>      -       t0 := Time utcMicrosecondClock + 200.
>>      -       [Time utcMicrosecondClock >= t0] whileFalse.
>>              t1 := self highResClock.
>>      +       [Time microsecondClockValue >= (t0 + 10000)] whileFalse.
>>      +       t1 := self highResClock - t1 * 1000 // (Time microsecondClockValue - t0).
>>      -       [Time utcMicrosecondClock >= (t0 + 10000)] whileFalse.
>>      -       t1 := self highResClock - t1 * 1000 // (Time utcMicrosecondClock - t0).
>
> The method uses the primitive directly to make the measurements as accurate as possible. This change would make them less accurate.
>
>>
>>              "Count the ticks ellapsed during a 20ms busy loop"
>>      +       t0 := Time microsecondClockValue + 200.
>>      +       [Time microsecondClockValue >= t0] whileFalse.
>>      -       t0 := Time utcMicrosecondClock + 200.
>>      -       [Time utcMicrosecondClock >= t0] whileFalse.
>>              t2 := self highResClock.
>>      +       [Time microsecondClockValue >= (t0 + 20000)] whileFalse.
>>      +       t2 := self highResClock - t2 * 1000 // (Time microsecondClockValue - t0).
>>      -       [Time utcMicrosecondClock >= (t0 + 20000)] whileFalse.
>>      -       t2 := self highResClock - t2 * 1000 // (Time utcMicrosecondClock - t0).
>>
>>              "Count the ticks ellapsed during a 30ms busy loop"
>>      +       t0 := Time microsecondClockValue + 200.
>>      +       [Time microsecondClockValue >= t0] whileFalse.
>>      -       t0 := Time utcMicrosecondClock + 200.
>>      -       [Time utcMicrosecondClock >= t0] whileFalse.
>>              t3 := self highResClock.
>>      +       [Time microsecondClockValue >= (t0 + 30000)] whileFalse.
>>      +       t3 := self highResClock - t3 * 1000 // (Time microsecondClockValue - t0).
>>      -       [Time utcMicrosecondClock >= (t0 + 30000)] whileFalse.
>>      -       t3 := self highResClock - t3 * 1000 // (Time utcMicrosecondClock - t0).
>>
>>              "Take the median of the 3 estimates as the best"
>>              ^ t1 <= t2
>>                      ifTrue: [t2 <= t3
>>                                      ifTrue: [t2]
>>                                      ifFalse: [t1 <= t3
>>                                                      ifTrue: [t3]
>>                                                      ifFalse: [t1]]]
>>                      ifFalse: [t1 <= t3
>>                                      ifTrue: [t1]
>>                                      ifFalse: [t2 <= t3
>>                                                      ifTrue: [t3]
>>                                                      ifFalse: [t2]]]!
>>
>>      Item was added:
>>      + ----- Method: Time class>>microsecondClockValue (in category 'general inquiries') -----
>>      + microsecondClockValue
>>      +       "Answer the value of the microsecond clock, the number of microseconds elapsed since 1/1/1901 @ 00:00:00."
>
> Wrong date there.
>
>>      +       ^ self utcMicrosecondClock - MicrosecondsBetweenPosixEpochAndSqueakEpoch!
>>
>>      Item was changed:
>>        ----- Method: Time class>>microsecondsToRun: (in category 'general inquiries') -----
>>        microsecondsToRun: timedBlock
>>              "Answer the number of microseconds timedBlock takes to return its value."
>>
>>              | startUsecs |
>>              (self useHighResClockForTiming and: [self highResClock ~= 0])
>>                      ifTrue: [       ^(self nanosecondsToRunHighRes: timedBlock) + 500 // 1000].
>>      +       startUsecs := self microsecondClockValue.
>>      -       startUsecs := self utcMicrosecondClock.
>>              timedBlock value.
>>      +       ^self microsecondClockValue - startUsecs!
>>      -       ^self utcMicrosecondClock - startUsecs!
>
> Again, decreases the measurement's accuracy.
>
>>
>>      Item was changed:
>>        ----- Method: Time class>>millisecondClockValue (in category 'general inquiries') -----
>>        millisecondClockValue
>>              "Answer the value of the millisecond clock."
>>      +       ^self microsecondClockValue // 1000!
>>      -
>>      -       ^self utcMicrosecondClock // 1000!
>
> After loading, this change would break code relying on previous results of this method.
>
>>
>>      Item was changed:
>>        ----- Method: Time class>>nanosecondsToRun: (in category 'general inquiries') -----
>>        nanosecondsToRun: timedBlock
>>              "Answer the number of nanoseconds timedBlock takes to return its value.
>>              Use high resolution clock if available and preferred."
>>
>>              | startUsecs |
>>              (self useHighResClockForTiming and: [self highResClock ~= 0])
>>                      ifTrue: [       ^(self nanosecondsToRunHighRes: timedBlock)].
>>              "Fallback to microseconds clock"
>>      +       startUsecs := self microsecondClockValue.
>>      -       startUsecs := self utcMicrosecondClock.
>>              timedBlock value.
>>      +       ^self microsecondClockValue - startUsecs * 1000!
>>      -       ^self utcMicrosecondClock - startUsecs * 1000!
>
> Accuracy again.
>
>>
>>      Item was removed:
>>      - ----- Method: Time class>>posixUtcMicrosecondClock (in category 'clock') -----
>>      - posixUtcMicrosecondClock
>>      -       "Answer the UTC microseconds since the POSIX epoch (January 1st 1970 00:00:00 UTC)."
>>      -
>>      -       ^self utcMicrosecondClock - MicrosecondsBetweenPosixEpochAndSqueakEpoch!
>>
>>      Item was changed:
>>      + ----- Method: Time class>>utcMicrosecondClock (in category 'private') -----
>>      - ----- Method: Time class>>utcMicrosecondClock (in category 'clock') -----
>
> Does this mean that I have to reimplement the primitive in my external packages from now on?
>
>
> Levente
>
>>        utcMicrosecondClock
>>              "Answer the UTC microseconds since the Smalltalk epoch (January 1st 1901, the start of the 20th century).
>>               The value is derived from the Posix epoch with a constant offset corresponding to elapsed microseconds
>>               between the two epochs according to RFC 868."
>>              <primitive: 240>
>>              ^0!
>