From 22f6836925cb1c682045497d214773a3d04e60b2 Mon Sep 17 00:00:00 2001 From: Holger Hans Peter Freyther Date: Wed, 6 Jul 2011 16:56:09 +0200 Subject: [PATCH] timer: Try to avoid copying the entire timerlist all the timer Make sure that we only remove from one process, this way we can check if the list is empty without having a lock. The list is sorted so even if we get interrupted between the [condition] whileFalse: [res] and someone adds an older timer, it is okay to use this timer. --- Timer.st | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/Timer.st b/Timer.st index 438d2de..a5d3e93 100644 --- a/Timer.st +++ b/Timer.st @@ -56,7 +56,13 @@ Object subclass: Timer [ cancel [ - schedule removeTimer: self. + "Remember that the timer is gone." + schedule := nil. + ] + + isCanceled [ + + ^ schedule == nil. ] ] @@ -111,13 +117,6 @@ bit difficult to do this race free.'> ^ sched ] - removeTimer: aSched [ - - sem critical: [ - queue remove: aSched ifAbsent: []. - ]. - ] - runTimers [ @@ -131,24 +130,20 @@ bit difficult to do this race free.'> fireTimers: now [ - | copy | - - "Create a shallow copy of the data" - copy := sem critical: [queue copy]. "Now execute the timers. One way or another this is crazy. If we have a long blocking application or a deadlock the timer queue will get stuck. But if we run this in a new process a later process might be run before this process, changing the order of the timers." - copy do: [:each | - each timeout > now ifTrue: [^true]. - sem critical: [queue remove: each]. - [ - each fire - ] on: Error do: [:e | + "Only this process will remove items, this is why we can check isEmpty + without having the lock" + [queue isEmpty or: [queue first timeout > now]] whileFalse: [ | each | + each := sem critical: [queue removeFirst]. + each isCanceled ifFalse: [ + [each fire] on: Error do: [:e | e logException: 'Execution of timer failed: %1' % {e tag} area: #timer. - ]. - ]. + ]]. + ] ] ]