Delay class not fit for RoarVM

Hi:

I ran into a little trouble with the Delay class.
David thought he had fixed it already, but there was another problem to get Pharo running multicore.

The fundamental problem is, that Delay relies on scheduler implementation details.
More precisely, it uses a single class variable to communicate the delay it wants to schedule (self) to the handleTimerEvent loop.

This cannot work on RoarVM, at every point in time, there can be multiple processes which all want to schedule a Delay, and the guarantees for impossible interleavings the current implementation is requiring are not given anymore.

Below are two, well, dirty hacks, to fix the problem.
Better solutions are very welcome. I do not really like the busy wait, however, other solutions add more overhead. I could imaging a semaphore based synchronization, but with the current semaphore implementation it is kind of tricky to get the initialization right. The biggest problem is the actual writing of that code, since changing Delay is not the most safe operation...
Another solution could involve a synchronized/atomic queue to communicate the Delays to the handler loop.


Beyond the implementation, it would be also interesting to discuss how such fixes, that are specific to RoarVM should be handled. Do you want to support it?

Thanks
Stefan

PS: those fixes should also work for Squeak, but I haven't tested it yet.



!Delay methodsFor: 'private' stamp: 'StefanMarr 11/9/2010 23:38' prior: 47886371!
schedule
        "Schedule this delay"
       
        | delayDelivered |
        delayDelivered := false.
       
        beingWaitedOn ifTrue: [^self error: 'This Delay has already been scheduled.'].
        resumptionTime := Time millisecondClockValue + delayDuration.
       
        [AccessProtect critical: [
                ScheduledDelay ifNil: [
                        ScheduledDelay := self.
                        TimingSemaphore signal.
                        delayDelivered := true.
                ]
        ]. delayDelivered ] whileFalse.! !
!Delay methodsFor: 'private' stamp: 'StefanMarr 11/9/2010 23:39' prior: 47885691!
unschedule
        | delayDelivered |
        delayDelivered := false.
       
        [AccessProtect critical: [
                ScheduledDelay ifNil: [
                        FinishedDelay := self.
                        TimingSemaphore signal.
                        delayDelivered := true.
                ]
        ]. delayDelivered ] whileFalse.! !
!Delay class methodsFor: 'timer process' stamp: 'StefanMarr 11/9/2010 17:44' prior: 19672467!
handleTimerEvent
        "Handle a timer event; which can be either:
                - a schedule request (ScheduledDelay notNil)
                - an unschedule request (FinishedDelay notNil)
                - a timer signal (not explicitly specified)
        We check for timer expiry every time we get a signal."
        | nowTick nextTick |
        "Wait until there is work to do."
        TimingSemaphore wait.

        "Process any schedule requests"
        ScheduledDelay ifNotNil:[
                "Schedule the given delay"
                self scheduleDelay: ScheduledDelay.
                ScheduledDelay := nil.
        ].

        "Process any unschedule requests"
        FinishedDelay ifNotNil:[
                self unscheduleDelay: FinishedDelay.
                FinishedDelay := nil.
        ].

        "Check for clock wrap-around."
        nowTick := Time millisecondClockValue.
        nowTick < ActiveDelayStartTime ifTrue: [
                "clock wrapped"
                self saveResumptionTimes.
                self restoreResumptionTimes.
        ].
        ActiveDelayStartTime := nowTick.

        "Signal any expired delays"
        [ActiveDelay notNil and:[nowTick >= ActiveDelay resumptionTime]] whileTrue:[
                ActiveDelay signalWaitingProcess.
                SuspendedDelays isEmpty
                        ifTrue: [ActiveDelay := nil]
                        ifFalse:[ActiveDelay := SuspendedDelays removeFirst].
        ].

        "And signal when the next request is due. We sleep at most 1sec here
        as a soft busy-loop so that we don't accidentally miss signals."
        nextTick := nowTick + 1000.
        ActiveDelay ifNotNil:[nextTick := nextTick min: ActiveDelay resumptionTime].
        nextTick := nextTick min: SmallInteger maxVal.

        "Since we have processed all outstanding requests, reset the timing semaphore so
        that only new work will wake us up again. Do this RIGHT BEFORE setting the next
        wakeup call from the VM because it is only signaled once so we mustn't miss it."
     "No!!!! If running multicore, TimingSemaphore may have already been signalled!!!!
      Do not reset signals, because then we will miss it -- dmu 9/26/10"
     RVMPrimitives coreCount > 1 ifFalse: [TimingSemaphore initSignals].
     "Do not wait till the next Delay if Semaphore already signaled. -- dmu 9/26/10"
     TimingSemaphore isSignaled ifFalse: [
          Delay primSignal: TimingSemaphore atMilliseconds: nextTick.
     ].

        "This last test is necessary for the obscure case that the msecs clock rolls over
        after nowTick has been computed (unlikely but not impossible). In this case we'd
        wait for MillisecondClockMask msecs (roughly six days) or until another delay gets
        scheduled (which may not be any time soon). In any case, since handling the
        condition is easy, let's just deal with it"
        Time millisecondClockValue < nowTick ifTrue:[TimingSemaphore signal]. "retry"
! !
--
Stefan Marr
Software Languages Lab
Vrije Universiteit Brussel
Pleinlaan 2 / B-1050 Brussels / Belgium
http://soft.vub.ac.be/~smarr
Phone: +32 2 629 2974
Fax:   +32 2 629 3525