Add primitiveMillisecondClockMask? (was: The Trunk: Kernel-ar.564.mcz)

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

Add primitiveMillisecondClockMask? (was: The Trunk: Kernel-ar.564.mcz)

David T. Lewis
On Thu, Apr 07, 2011 at 09:04:41AM +0000, [hidden email] wrote:

>
> Andreas Raab uploaded a new version of Kernel to project The Trunk:
> http://source.squeak.org/trunk/Kernel-ar.564.mcz
>
> ==================== Summary ====================
>
> Name: Kernel-ar.564
> Author: ar
> Time: 7 April 2011, 11:04:21.248 am
> UUID: 32ecb9c9-177a-ce45-9c1c-a3714c801929
> Ancestors: Kernel-nice.563
>
> Fix computation of Time>>milliseconds:since: which would compute incorrect deltas upon clock overflow.
>
> =============== Diff against Kernel-nice.563 ===============
>
> Item was added:
> + ----- Method: Time class>>millisecondClockMask (in category 'general inquiries') -----
> + millisecondClockMask
> + "The VM mask for the max. millisecond clock value.
> + This should be primitive but it ain't so it's copied inline from the Interpreter."
> + ^16r1FFFFFFF!
>

Agreed, the millisecondClockMask should be available from a primitive.
Should I make it so?

Dave


Reply | Threaded
Open this post in threaded view
|

Re: Add primitiveMillisecondClockMask? (was: The Trunk: Kernel-ar.564.mcz)

Eliot Miranda-2


On Wed, Apr 13, 2011 at 11:09 AM, David T. Lewis <[hidden email]> wrote:
On Thu, Apr 07, 2011 at 09:04:41AM +0000, [hidden email] wrote:
>
> Andreas Raab uploaded a new version of Kernel to project The Trunk:
> http://source.squeak.org/trunk/Kernel-ar.564.mcz
>
> ==================== Summary ====================
>
> Name: Kernel-ar.564
> Author: ar
> Time: 7 April 2011, 11:04:21.248 am
> UUID: 32ecb9c9-177a-ce45-9c1c-a3714c801929
> Ancestors: Kernel-nice.563
>
> Fix computation of Time>>milliseconds:since: which would compute incorrect deltas upon clock overflow.
>
> =============== Diff against Kernel-nice.563 ===============
>
> Item was added:
> + ----- Method: Time class>>millisecondClockMask (in category 'general inquiries') -----
> + millisecondClockMask
> +     "The VM mask for the max. millisecond clock value.
> +     This should be primitive but it ain't so it's copied inline from the Interpreter."
> +      ^16r1FFFFFFF!
>

Agreed, the millisecondClockMask should be available from a primitive.
Should I make it so?

Personally I would much rather see us move to the 64-bit millisecond clock I implemented for Cog. This renders the whole clock roll-over issue moot.

Eliot
 

Dave





Reply | Threaded
Open this post in threaded view
|

Re: Add primitiveMillisecondClockMask? (was: The Trunk: Kernel-ar.564.mcz)

David T. Lewis
On Wed, Apr 13, 2011 at 11:57:32AM -0700, Eliot Miranda wrote:

> On Wed, Apr 13, 2011 at 11:09 AM, David T. Lewis <[hidden email]>wrote:
>
> > On Thu, Apr 07, 2011 at 09:04:41AM +0000, [hidden email] wrote:
> > >
> > > Andreas Raab uploaded a new version of Kernel to project The Trunk:
> > > http://source.squeak.org/trunk/Kernel-ar.564.mcz
> > >
> > > ==================== Summary ====================
> > >
> > > Name: Kernel-ar.564
> > > Author: ar
> > > Time: 7 April 2011, 11:04:21.248 am
> > > UUID: 32ecb9c9-177a-ce45-9c1c-a3714c801929
> > > Ancestors: Kernel-nice.563
> > >
> > > Fix computation of Time>>milliseconds:since: which would compute
> > incorrect deltas upon clock overflow.
> > >
> > > =============== Diff against Kernel-nice.563 ===============
> > >
> > > Item was added:
> > > + ----- Method: Time class>>millisecondClockMask (in category 'general
> > inquiries') -----
> > > + millisecondClockMask
> > > +     "The VM mask for the max. millisecond clock value.
> > > +     This should be primitive but it ain't so it's copied inline from
> > the Interpreter."
> > > +      ^16r1FFFFFFF!
> > >
> >
> > Agreed, the millisecondClockMask should be available from a primitive.
> > Should I make it so?
> >
>
> Personally I would much rather see us move to the 64-bit millisecond clock I
> implemented for Cog. This renders the whole clock roll-over issue moot.
>

Understood, though my question was intended to be about the existing
primitiveMillisecondClock, which will presumably be retained for a
few more years. Andreas committed a fix for a rather nasty bug related
to a constant in the image being out of sync with a constant in the VM.
The method comment noted that "This should be primitive but it ain't"
so I was just offering to add the primitive and update Squeak trunk
to use it when available.

Dave


Reply | Threaded
Open this post in threaded view
|

Re: Add primitiveMillisecondClockMask? (was: The Trunk: Kernel-ar.564.mcz)

Colin Putney-3
On Wed, Apr 13, 2011 at 6:58 PM, David T. Lewis <[hidden email]> wrote:

> Understood, though my question was intended to be about the existing
> primitiveMillisecondClock, which will presumably be retained for a
> few more years. Andreas committed a fix for a rather nasty bug related
> to a constant in the image being out of sync with a constant in the VM.
> The method comment noted that "This should be primitive but it ain't"
> so I was just offering to add the primitive and update Squeak trunk
> to use it when available.

Why does the existing primitiveMillisecondClock have to be maintained?
Is there a lot of code that depends on it rolling over? No sarcasm
intended, it's an honest question.

Colin

Reply | Threaded
Open this post in threaded view
|

Re: Add primitiveMillisecondClockMask? (was: The Trunk: Kernel-ar.564.mcz)

David T. Lewis
On Wed, Apr 13, 2011 at 07:07:57PM -0700, Colin Putney wrote:

> On Wed, Apr 13, 2011 at 6:58 PM, David T. Lewis <[hidden email]> wrote:
>
> > Understood, though my question was intended to be about the existing
> > primitiveMillisecondClock, which will presumably be retained for a
> > few more years. Andreas committed a fix for a rather nasty bug related
> > to a constant in the image being out of sync with a constant in the VM.
> > The method comment noted that "This should be primitive but it ain't"
> > so I was just offering to add the primitive and update Squeak trunk
> > to use it when available.
>
> Why does the existing primitiveMillisecondClock have to be maintained?
> Is there a lot of code that depends on it rolling over? No sarcasm
> intended, it's an honest question.

It is used by Time class>>millisecondClockValue, so see senders of
#millisecondClockValue.

>From the point of view of the VM, the primitive needs to be maintained
because many existing images expect it to be there. From the point of
view of the image, a different and better implementation can easily
be adopted. My question was from the point of view of the VM, if we
assume that many images will expect primitiveMillisecondClock to be
there, should we also add primitiveMillisecondClockMask such that
those images can obtain the correct mask value if they choose to
do so (both the VM and the image changes are minor updates).

With respect to code that depends on the clock rollover, I expect
that the most serious concern would related to socket timeouts, which
might intermittently affect server applications, Seaside, etc.

To be clear, the patch that Andreas committed will fully address
the problem for all current VMs and images, so this discussion
has little or no practical impact. It's really just about tidying
up some loose ends just in case the the primitiveMillisecondClock
ends up getting used for a few years longer than any of us might
care to anticipate ;)

Dave


Reply | Threaded
Open this post in threaded view
|

Re: Add primitiveMillisecondClockMask? (was: The Trunk: Kernel-ar.564.mcz)

Igor Stasenko
In reply to this post by Eliot Miranda-2
On 13 April 2011 20:57, Eliot Miranda <[hidden email]> wrote:

>
>
> On Wed, Apr 13, 2011 at 11:09 AM, David T. Lewis <[hidden email]>
> wrote:
>>
>> On Thu, Apr 07, 2011 at 09:04:41AM +0000, [hidden email] wrote:
>> >
>> > Andreas Raab uploaded a new version of Kernel to project The Trunk:
>> > http://source.squeak.org/trunk/Kernel-ar.564.mcz
>> >
>> > ==================== Summary ====================
>> >
>> > Name: Kernel-ar.564
>> > Author: ar
>> > Time: 7 April 2011, 11:04:21.248 am
>> > UUID: 32ecb9c9-177a-ce45-9c1c-a3714c801929
>> > Ancestors: Kernel-nice.563
>> >
>> > Fix computation of Time>>milliseconds:since: which would compute
>> > incorrect deltas upon clock overflow.
>> >
>> > =============== Diff against Kernel-nice.563 ===============
>> >
>> > Item was added:
>> > + ----- Method: Time class>>millisecondClockMask (in category 'general
>> > inquiries') -----
>> > + millisecondClockMask
>> > +     "The VM mask for the max. millisecond clock value.
>> > +     This should be primitive but it ain't so it's copied inline from
>> > the Interpreter."
>> > +      ^16r1FFFFFFF!
>> >
>>
>> Agreed, the millisecondClockMask should be available from a primitive.
>> Should I make it so?
>
> Personally I would much rather see us move to the 64-bit millisecond clock I
> implemented for Cog. This renders the whole clock roll-over issue moot.

Except that if you change clock on your machine, Cog hangs, because it
doesn't takes into account
that clock counter could 'advance' backwards.
This could happen either when user changes system time, or if your
machine synchronizing clock via network.

I plagued by this issue, because i using dual boot MacOS/Windows on my
machine , and windows
writes UTC clock value to CMOS, while MacOS treats it as a local time,
so when i reboot under MacOS, my time is always 1 (or 2) hours in
advance.
Which is pretty annoying. And every time i changing the clock i need
to watch out if i don't have any images open.

A solution to that, i think, is to handle system event which tells
that user changed current time.
But i'm not sure if all platforms support such kind of event, and how
to listen for it.

> 2¢
> Eliot
>
>>
>> Dave



--
Best regards,
Igor Stasenko AKA sig.

Reply | Threaded
Open this post in threaded view
|

Re: [Vm-dev] Re: [squeak-dev] Add primitiveMillisecondClockMask? (was: The Trunk: Kernel-ar.564.mcz)

Bert Freudenberg
In reply to this post by David T. Lewis

On 14.04.2011, at 04:46, David T. Lewis wrote:

>
> On Wed, Apr 13, 2011 at 07:07:57PM -0700, Colin Putney wrote:
>> On Wed, Apr 13, 2011 at 6:58 PM, David T. Lewis <[hidden email]> wrote:
>>
>>> Understood, though my question was intended to be about the existing
>>> primitiveMillisecondClock, which will presumably be retained for a
>>> few more years. Andreas committed a fix for a rather nasty bug related
>>> to a constant in the image being out of sync with a constant in the VM.
>>> The method comment noted that "This should be primitive but it ain't"
>>> so I was just offering to add the primitive and update Squeak trunk
>>> to use it when available.
>>
>> Why does the existing primitiveMillisecondClock have to be maintained?
>> Is there a lot of code that depends on it rolling over? No sarcasm
>> intended, it's an honest question.
>
> It is used by Time class>>millisecondClockValue, so see senders of
> #millisecondClockValue.
>
>> From the point of view of the VM, the primitive needs to be maintained
> because many existing images expect it to be there. From the point of
> view of the image, a different and better implementation can easily
> be adopted. My question was from the point of view of the VM, if we
> assume that many images will expect primitiveMillisecondClock to be
> there, should we also add primitiveMillisecondClockMask such that
> those images can obtain the correct mask value if they choose to
> do so (both the VM and the image changes are minor updates).
>
> With respect to code that depends on the clock rollover, I expect
> that the most serious concern would related to socket timeouts, which
> might intermittently affect server applications, Seaside, etc.
>
> To be clear, the patch that Andreas committed will fully address
> the problem for all current VMs and images, so this discussion
> has little or no practical impact. It's really just about tidying
> up some loose ends just in case the the primitiveMillisecondClock
> ends up getting used for a few years longer than any of us might
> care to anticipate ;)
>
> Dave

I agree that it should have been made a primitive long ago. However, adding it now would only make sense if we anticipate that it will ever change. And since Eliot is advocating to switch over to a 64 bit clock anyway, what's the point of changing the old one now?

I'd still retain the old primitive for accessing millisecondClockValue because it certainly can be cheaper (no LargeInts). Or was there a proposal to actually change the old primitive, instead of adding the high-res one?

- Bert -