Ben Coman wrote:
I know everyone is busy and working in their own domains not having the
time to review a lot of other people's issues. However I am on my own,
not part of a team where I can walk over and tap someone on the shoulder
to ask for review of an issue I've resolved. It is a bit discouraging
not to be able to proceed for lack of review. I'd be glad for any
feedback including "its crap, do it a different way". So consider this
a virtual should tap.
I have...
* refactored the two calls to #forMilliseconds: out to a single call
* replaced call to #wait with #waitOr:
* added #waitOr:
Here is the diff...
WorldState >> interCyclePause: milliSecs
| currentTime wait |
self serverMode
ifFalse: [
(lastCycleTime notNil and: [CanSurrenderToOS ~~ false]) ifTrue: [
currentTime := Time millisecondClockValue.
wait := lastCycleTime + milliSecs - currentTime.
+ "wait is the time remaining until milliSecs has passed
+ since lastCycle. If wait is zero or less, no need to Delay"
+ "If wait is greater than milliSeconds then bypass Delay,
+ by setting wait to zero."
- (wait > 0 and: [ wait <= milliSecs ] ) ifTrue: [
- (Delay forMilliseconds: wait) wait ] ] ]
+ wait > milliSecs ifTrue: [ wait := 0 ] ] ] "btw, wait>milliSecs can
only occur when clock rolls over. Eliminate after integrating issue
14353 ?"
- ifTrue: [ (Delay forMilliseconds: 50) wait ].
+ ifTrue: [ wait := 50 ].
+ wait > 0 ifTrue: [
+ (Delay forMilliseconds: wait) waitOr: [
+ self inform: 'WorldState>>interCyclePause
failed.' ] ].
lastCycleTime := Time millisecondClockValue.
CanSurrenderToOS := true.
"--------------------------"
+Delay >> waitOr: anErrorBlock
+ self schedule.
+ [ beingWaitedOn
+ ifTrue: [ delaySemaphore wait ]
+ ifFalse:[ anErrorBlock value ]
+ ] ifCurtailed:[self unschedule].
+"Only the high priority timer event loop (TEL) signals
/delaySempahore/. If the
+ TEL is not running, a delay will block forever (for example the UI
will lock up).
+ /beingWaitedOn/ is only set to true from the TEL (via
+ DelayScheduler>>scheduleDelay: and Delay>>timingPriorityWaitedOn:) so
test
+ this to determine if TEL is running. Avoid waiting when the TEL is
not running."
"--------------------------"
This is for issue 14669 "Delay refactoring (part 2a) - avoid UI locking
up when timer event loop is stopped" that is required to facilitate
integration of issue 14353 "Delay refactoring (part 2) - change from
milliseconds to microseconds.
cheers -ben
https://pharo.fogbugz.com/default.asp?14669
https://pharo.fogbugz.com/default.asp?14353
I should add, the test procedure would be:
1. Open Tools > Process Browser
Observe it shows "(80) Delay Scheduling Process:
DelayScheduler>>runTimerEventLoop"
2. Merge slice 14669
3. In Workspace do "Delay stopTimerEventLoop"
Observe many informs stream up screen. Now the UI might be a bit slow
bit is still responsive. Previously the UI would have locked up. This
is the point of this change - having a chance to correct the fault.
4. Update the list in Tools > Process Browser.
Observe process "(80...)" is gone.
5. In Workspace do "Delay startTimerEventLoop"
Observe the informs have stopped.
cheers -ben
