Delays getting mangled across save

Well the timer stuff has certainly been messed about with a lot since I last had to dig into it, but it looks like it *ought* to work ok.

The only likely culprit I can spot is some issue with adjusting the resumption times after the restart, but that would require some problem with the millisecond time prim.


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Strange OpCodes: YVR: Branch to Vancouver

That has to be one of the oldest bugs! It's been there forever. I just committed that to trunk.

There is still a problem though. Squeak's delays (with Bob's fix) are based on image "running time". That is, if I have a 1 hour delay and snapshot the image after 45 minutes, on the next day it will expire 15 minutes after starting the image.

But arguably a delay should really expire by the same wall-clock time as when it was scheduled, even across snapshots. This is how it was handled in Smalltalk-80. Here's the ST80 code:

Delay>>preSnapshot

"convert from local millisecond clock to milliseconds since Jan. 1 1901"

pendingDelay _ resumptionTime - Time millisecondClockValue.

resumptionTime _ Time totalSeconds * 1000 + pendingDelay

Delay>>postSnapshot

"convert from milliseconds since Jan. 1 1901 to local millisecond clock"

pendingDelay _ resumptionTime - (Time totalSeconds * 1000).

pendingDelay _ pendingDelay max: 0.

resumptionTime _ Time millisecondClockValue + pendingDelay

Of course, we would have to use UTC nowadays but I still think re-implementing this would be a good idea.

In any case, the fix makes the Delay snapshotting behavior at least predictable :)

- Bert -

On 2013-08-06, at 21:57, Bob Arning <[hidden email]> wrote:

Well, this seems to fix it:

'From Squeak4.4 of 1 March 2013 [latest update: #12489] on 6 August 2013 at 3:39:29 pm'!

!Delay class methodsFor: 'snapshotting' stamp: 'raa 8/6/2013 15:22'!
restoreResumptionTimes
    "Private!! Restore the resumption times of all scheduled Delays after a snapshot or clock roll-over. This method should be called only while the AccessProtect semaphore is held."

    | newBaseTime |
    newBaseTime := Time millisecondClockValue.
    SuspendedDelays do: [:d | d adjustResumptionTimeOldBase: 0 newBase: newBaseTime].
    ActiveDelay == nil ifFalse: [
        ActiveDelay adjustResumptionTimeOldBase: 0 newBase: newBaseTime.
    ].
    ActiveDelayStartTime _ newBaseTime "<-----this"! !

Cheers,
Bob

On 8/6/13 3:27 PM, tim Rowledge wrote:

Well the timer stuff has certainly been messed about with a lot since I last had to dig into it, but it looks like it *ought* to work ok.

The only likely culprit I can spot is some issue with adjusting the resumption times after the restart, but that would require some problem with the millisecond time prim.


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Strange OpCodes: YVR: Branch to Vancouver

Everyone here is aware of the enormous amount of traffic on the Pharo
list about getting these kinds of Delay problems sorted out, right?

frank

On 7 August 2013 16:14, Bert Freudenberg [hidden email] wrote:

Yes, making Delays work accurately wrt wall-clock time would be more work
now that I think about it. This seems to be better handled in a higher-level
object.

- Bert -

On 2013-08-07, at 16:16, Bob Arning [hidden email] wrote:

I was thinking about that, but I suspect there are use cases for both image
time and real-world time. Maybe something like

(Delay until: someRealTime) wait

would meet the need the current Delay does not.

Cheers,
Bob

On 8/7/13 9:54 AM, Bert Freudenberg wrote:

That has to be one of the oldest bugs! It's been there forever. I just
committed that to trunk.

There is still a problem though. Squeak's delays (with Bob's fix) are based
on image "running time". That is, if I have a 1 hour delay and snapshot the
image after 45 minutes, on the next day it will expire 15 minutes after
starting the image.

But arguably a delay should really expire by the same wall-clock time as
when it was scheduled, even across snapshots. This is how it was handled in
Smalltalk-80. Here's the ST80 code:

Delay>>preSnapshot
"convert from local millisecond clock to milliseconds since Jan. 1 1901"
pendingDelay _ resumptionTime - Time millisecondClockValue.
resumptionTime _ Time totalSeconds * 1000 + pendingDelay

Delay>>postSnapshot
"convert from milliseconds since Jan. 1 1901 to local millisecond clock"
pendingDelay _ resumptionTime - (Time totalSeconds * 1000).
pendingDelay _ pendingDelay max: 0.
resumptionTime _ Time millisecondClockValue + pendingDelay

Of course, we would have to use UTC nowadays but I still think
re-implementing this would be a good idea.

In any case, the fix makes the Delay snapshotting behavior at least
predictable :)

- Bert -

On 2013-08-06, at 21:57, Bob Arning [hidden email] wrote:

Well, this seems to fix it:

'From Squeak4.4 of 1 March 2013 [latest update: #12489] on 6 August 2013 at
3:39:29 pm'!

!Delay class methodsFor: 'snapshotting' stamp: 'raa 8/6/2013 15:22'!
restoreResumptionTimes
    "Private!! Restore the resumption times of all scheduled Delays after a
snapshot or clock roll-over. This method should be called only while the
AccessProtect semaphore is held."

    | newBaseTime |
    newBaseTime := Time millisecondClockValue.
    SuspendedDelays do: [:d | d adjustResumptionTimeOldBase: 0 newBase:
newBaseTime].
    ActiveDelay == nil ifFalse: [
        ActiveDelay adjustResumptionTimeOldBase: 0 newBase: newBaseTime.
    ].
    ActiveDelayStartTime _ newBaseTime "<-----this"! !

Cheers,
Bob

On 8/6/13 3:27 PM, tim Rowledge wrote:

Well the timer stuff has certainly been messed about with a lot since I last
had to dig into it, but it looks like it *ought* to work ok.

The only likely culprit I can spot is some issue with adjusting the
resumption times after the restart, but that would require some problem with
the millisecond time prim.


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Strange OpCodes: YVR: Branch to Vancouver

At any rate my point is that Delays across image saves have caused
Pharo people a reasonable amount of pain. If someone experiences pain,
it can be worthwhile learning from the lessons of others.

That is all.

frank

On 7 August 2013 17:13, Bert Freudenberg [hidden email] wrote:

... which is not what Igor was proposing there, and I agree with Eliot's objection to simply firing all delays at image startup time.

Switching to wall-clock for all delays would have pretty much the same effect, since the typical time between image snapshot and startup is larger than the typical delay time.

Delays being ultimately low-level objects I don't think wall-clock makes sense for them. You would need a calendar object for that, IMHO.

- Bert -

On 2013-08-07, at 17:52, Frank Shearar [hidden email] wrote:

Well, it's at least partially here:
http://forum.world.st/A-matter-with-Delays-td4671239.html People there
seem to be proposing what's in this thread: sometimes what you mean is
"15 minutes from now, wall clock time, do this", and other times you
want "15 minutes from now, 'logical time', do this". "Logical time"
means image running time.

Personally, I'm in the wall-clock-is-all-there-is camp, for what it's worth.

frank

On 7 August 2013 16:40, Bert Freudenberg [hidden email] wrote:

Err, no? I don't have time to follow all mailing lists, rather relying on kind people forwarding relevant discussions here :)

- Bert -

On 2013-08-07, at 17:36, Frank Shearar [hidden email] wrote:

Everyone here is aware of the enormous amount of traffic on the Pharo
list about getting these kinds of Delay problems sorted out, right?

frank

On 7 August 2013 16:14, Bert Freudenberg [hidden email] wrote:

Yes, making Delays work accurately wrt wall-clock time would be more work
now that I think about it. This seems to be better handled in a higher-level
object.

- Bert -

On 2013-08-07, at 16:16, Bob Arning [hidden email] wrote:

I was thinking about that, but I suspect there are use cases for both image
time and real-world time. Maybe something like

(Delay until: someRealTime) wait

would meet the need the current Delay does not.

Cheers,
Bob

On 8/7/13 9:54 AM, Bert Freudenberg wrote:

That has to be one of the oldest bugs! It's been there forever. I just
committed that to trunk.

There is still a problem though. Squeak's delays (with Bob's fix) are based
on image "running time". That is, if I have a 1 hour delay and snapshot the
image after 45 minutes, on the next day it will expire 15 minutes after
starting the image.

But arguably a delay should really expire by the same wall-clock time as
when it was scheduled, even across snapshots. This is how it was handled in
Smalltalk-80. Here's the ST80 code:

Delay>>preSnapshot
"convert from local millisecond clock to milliseconds since Jan. 1 1901"
pendingDelay _ resumptionTime - Time millisecondClockValue.
resumptionTime _ Time totalSeconds * 1000 + pendingDelay

Delay>>postSnapshot
"convert from milliseconds since Jan. 1 1901 to local millisecond clock"
pendingDelay _ resumptionTime - (Time totalSeconds * 1000).
pendingDelay _ pendingDelay max: 0.
resumptionTime _ Time millisecondClockValue + pendingDelay

Of course, we would have to use UTC nowadays but I still think
re-implementing this would be a good idea.

In any case, the fix makes the Delay snapshotting behavior at least
predictable :)

- Bert -

On 2013-08-06, at 21:57, Bob Arning [hidden email] wrote:

Well, this seems to fix it:

'From Squeak4.4 of 1 March 2013 [latest update: #12489] on 6 August 2013 at
3:39:29 pm'!

!Delay class methodsFor: 'snapshotting' stamp: 'raa 8/6/2013 15:22'!
restoreResumptionTimes
  "Private!! Restore the resumption times of all scheduled Delays after a
snapshot or clock roll-over. This method should be called only while the
AccessProtect semaphore is held."

  | newBaseTime |
  newBaseTime := Time millisecondClockValue.
  SuspendedDelays do: [:d | d adjustResumptionTimeOldBase: 0 newBase:
newBaseTime].
  ActiveDelay == nil ifFalse: [
      ActiveDelay adjustResumptionTimeOldBase: 0 newBase: newBaseTime.
  ].
  ActiveDelayStartTime _ newBaseTime "<-----this"! !

Cheers,
Bob

On 8/6/13 3:27 PM, tim Rowledge wrote:

Well the timer stuff has certainly been messed about with a lot since I last
had to dig into it, but it looks like it *ought* to work ok.

The only likely culprit I can spot is some issue with adjusting the
resumption times after the restart, but that would require some problem with
the millisecond time prim.


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Strange OpCodes: YVR: Branch to Vancouver