United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-5053997 Swing Timer cannot handle multiple timers effectively
JDK-5053997 : Swing Timer cannot handle multiple timers effectively

Details
Type:
Bug
Submit Date:
2004-05-27
Status:
Closed
Updated Date:
2010-07-09
Project Name:
JDK
Resolved Date:
2011-03-08
Component:
client-libs
OS:
windows_nt,linux,generic,windows_xp,windows_2000
Sub-Component:
javax.swing
CPU:
x86,generic
Priority:
P5
Resolution:
Fixed
Affected Versions:
1.4.0,1.4.2,5.0,6
Fixed Versions:

Related Reports
Duplicate:
Duplicate:
Relates:
Relates:

Sub Tasks

Description
The Swing Timer code performs a wait() function after firing any single timer event.  This effectively causes that thread to sleep for the minimum timing resolution of that system (typically 10-15 ms on Windows, 20-50 ms on
some Unix systems).  if you assume a system clock resolution of 10 ms, this
means that only 100 timers can fire per second, maximum.  If an application has
requested several timers with small delays (say 10 ms), then the timers will not get serviced enough due to the wait() call and the application cannot count on the rate of the timers.

This is the case even when the timers are not actually performing any significant work; the thread sleeping caused by the wait() sets a minimum delay amount based on the number of timers being serviced; it has nothing to do with the work performed by any one timer.

I've attached a simple test case which demonstrates the problem: TimerTest.
This test creates serveral timers (20 by default) with the same delay time
(10 ms by default) and runs them all for some set period of time (3 seconds
by default).  It then compares and prints out the requested delay time along with the actual delay time (calculated by dividing the total time that the
timer was active by the number of times that timer fired).

On my win2k system, I got the following results:
java TimerTest -numTimers 1
	Requested Delay, Actual Delay = 10, 11
java TimerTest -numTimers 2
	Requested Delay, Actual Delay = 10, 19
java TimerTest -numTimers 10
	Requested Delay, Actual Delay = 10, 96

On my Solaris system (ultra60, Solaris 8), I got:
java TimerTest -numTimers 1
	Requested Delay, Actual Delay = 10, 19
java TimerTest -numTimers 2
	Requested Delay, Actual Delay = 10, 39
java TimerTest -numTimers 10
	Requested Delay, Actual Delay = 10, 188
Contributed by java.net member leouser

A DESCRIPTION OF THE FIX :
BUG ID: 5053997 Swing Timer cannot handle multple timers effectively.
FILES AFFECTED: javax.swing.TimerQueue
JDK VERSION
jdk-6-rc-bin-b64-linux-i586-15_dec_2005.bin

Discusion(embeded in test case as well):
/**
 * BUG ID: 5053997 Swing Timer cannot handle multple timers effectively.
 * I had to look at this one at a small time scale to really get at what
 * it was saying.  Executing the timers, each one is separated by a call
 * to wait(1).  This call appears to be platform dependent upon how long
 * a task is supposed to wait.  On my platform the 10 millisecond wait
 * turns into 100 ms.  Ive done some radical surgery on the TimerQueue
 * and have reimplemented it with a ScheduledExecutorService.  This
 * has had the effect of cutting the wait period by about 90% on these
 * small times.  Instead of seeing 100ms wait periods I was seeing 10 to
 * 20 ms wait periods---> which appear to be very close to the mark.
 *
 * Reimplementing the TimerQueue in terms of the ScheduledExecutorService
 * simplifies the code significantly.  No longer is the class managing
 * all the Timers in its linked list for the management is done in the
 * SES.  In other words the specialty of the java.util.concurrent package
 * really shines here.
 *
 * TESTING STRATEGY:
 * First off this isn't a Junit style test case.  We are observing
 * timing behavior here, not does a == b and stuff.  So we must watch
 * and see.  What we see determines if we are hitting the mark.
 * 1. Execute the 5 timers at 10 MILLISECONDS, 10 times each.  See the
 * difference in timing between the old implementation and the new one.  Both
 * intial timings should be off, there appears to be some overhead associated
 * with getting things started up.  Once beyond the initial runs you can
 * see the time difference.
 * 2. Run a continuous loop that schedules Timers against a continously
 * running Timer.  This is an attempt to shake out a naughty timer that
 * is stoping other timers from getting scheduled.  From testing it on
 * my platform, I did not see such lock out.
 * #1 is executed by: java TestTimer
 * #2 is executed by: java TestTimer type what you want here.
 *
 * WHAT ELSE DOES THIS MEAN?
 * It may be possible to enhance the Timer so that it will repeat itself
 * based on the different time resolutions offered by TimeUnit.  something
 * like setResolution(TimeUnit tu).  I won't be helped by this with my
 * current platform(s) but someone with a snazzy OS may benefit.
 *
 *
 * FILES AFFECTED: javax.swing.TimerQueue
 *
 * JDK VERSION
 * jdk-6-rc-bin-b64-linux-i586-15_dec_2005.bin
 *
 * test ran succesfully on a SUSE 7.3 Linux distribution
 *
 * Brian Harry
 * ###@###.###
 * Jan 24, 2006
 */

UNIFIED DIFF:
--- /home/nstuff/java6/jdk1.6.0/javax/swing/TimerQueue.java	Thu Dec 15 02:17:39 2005
+++ /home/javarefs/javax/swing/TimerQueue.java	Tue Jan 24 14:47:43 2006
@@ -12,25 +12,30 @@
 
 
 import java.util.*;
-
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
 
 
 /**
  * Internal class to manage all Timers using one thread.
- * TimerQueue manages a queue of Timers. The Timers are chained
- * together in a linked list sorted by the order in which they will expire.
+ * TimerQueue manages the Timers with a ScheduledExecutorService.
  *
  * @version 1.38 11/17/05
  * @author Dave Moore
  */
-class TimerQueue implements Runnable
+class TimerQueue
 {
     private static final Object sharedInstanceKey =
         new StringBuffer("TimerQueue.sharedInstanceKey");
     private static final Object expiredTimersKey =
         new StringBuffer("TimerQueue.expiredTimersKey");
 
-    Timer   firstTimer;
+    private static ScheduledExecutorService eservice = null;
+
+    private static Map<Timer, ScheduledFuture> timers = new WeakHashMap<Timer,ScheduledFuture>();
+
     boolean running;
 
     /* Lock object used in place of class object for synchronization.
@@ -71,13 +76,9 @@
         }
         else {
             java.security.AccessController.doPrivileged(
-                new java.security.PrivilegedAction() {
+                new java.security.PrivilegedAction<Object>() {
                 public Object run() {
-                    Thread timerThread = new Thread(TimerQueue.this,
-                                                    "TimerQueue");
-                    timerThread.setDaemon(true);
-                    timerThread.setPriority(Thread.NORM_PRIORITY);
-                    timerThread.start();
+                    eservice = Executors.newSingleThreadScheduledExecutor();
                     return null;
                 }
             });
@@ -87,185 +88,97 @@
 
 
     synchronized void stop() {
-        running = false;
+	running = false;
+        eservice.shutdownNow();
+        eservice = null;
         notify();
     }
 
+    static class TimerWrapper implements Runnable{
+        
+	Timer timer;
+        TimerWrapper(Timer timer){
+	    this.timer = timer;
+        }
+        
+        public void run(){
+            try {
+                timer.post();  // have timer post an event
+                if(timer.isRepeats()){
+                    ScheduledFuture sf = eservice.schedule(this, timer.getDelay(), TimeUnit.MILLISECONDS);
+                    timers.put(timer, sf);
+                    timer.running = true;
+                }
+                else{
+                    timers.remove(timer);
+                    timer.running = false;
+                }
+             }
+             catch (SecurityException e) {}
+        }
+    }
 
     synchronized void addTimer(Timer timer, long expirationTime) {
-        Timer previousTimer;
-        Timer nextTimer;
-
         // If the Timer is already in the queue, then ignore the add.
         if (timer.running) {
             return;
         }
 
-        previousTimer = null;
-        nextTimer = firstTimer;
-
-        // Insert the Timer into the linked list in the order they will
-        // expire.  If two timers expire at the same time, put the newer entry
-        // later so they expire in the order they came in.
-
-        while (nextTimer != null) {
-            if (nextTimer.expirationTime > expirationTime) break;
-
-            previousTimer = nextTimer;
-            nextTimer = nextTimer.nextTimer;
-        }
-
-        if (previousTimer == null) {
-            firstTimer = timer;
-        }
-        else {
-            previousTimer.nextTimer = timer;
-        }
-
+        TimerWrapper tw = new TimerWrapper(timer);
+        long twait = expirationTime - System.currentTimeMillis();
+        if(twait < 0L) twait = 0L;
+        ScheduledFuture sf = eservice.schedule(tw, twait, TimeUnit.MILLISECONDS);
+        timers.put(timer, sf);
         timer.expirationTime = expirationTime;
-        timer.nextTimer = nextTimer;
         timer.running = true;
         notify();
     }
 
 
     synchronized void removeTimer(Timer timer) {
-        Timer   previousTimer;
-        Timer   nextTimer;
-        boolean found;
 
         if (!timer.running) return;
-
-        previousTimer = null;
-        nextTimer = firstTimer;
-        found = false;
-
-        while (nextTimer != null) {
-            if (nextTimer == timer) {
-                found = true;
-                break;
-            }
-
-            previousTimer = nextTimer;
-            nextTimer = nextTimer.nextTimer;
-        }
-
-        if (!found) return;
-
-        if (previousTimer == null) {
-            firstTimer = timer.nextTimer;
+        if(timers.containsKey(timer)){
+            ScheduledFuture sf = timers.get(timer);
+            sf.cancel(false);
+            timers.remove(timer);
         }
-        else {
-            previousTimer.nextTimer = timer.nextTimer;
-        }
-
         timer.expirationTime = 0;
-        timer.nextTimer = null;
         timer.running = false;
-    }
+     }
 
 
     synchronized boolean containsTimer(Timer timer) {
         return timer.running;
     }
-
-
-    /**
-     * If there are a ton of timers, this method may never return.  It loops
-     * checking to see if the head of the Timer list has expired.  If it has,
-     * it posts the Timer and reschedules it if necessary.
-     */
-    synchronized long postExpiredTimers() {
-        Timer   timer;
-        long    currentTime;
-        long    timeToWait;
-
-        // The timeToWait we return should never be negative and only be zero
-        // when we have no Timers to wait for.
-
-        do {
-            timer = firstTimer;
-            if (timer == null) return 0;
-
-            currentTime = System.currentTimeMillis();
-            timeToWait = timer.expirationTime - currentTime;
-
-            if (timeToWait <= 0) {
-                try {
-                    timer.post();  // have timer post an event
-                }
-                catch (SecurityException e) {
-                }
-
-                // Remove the timer from the queue
-                removeTimer(timer);
-
-                // This tries to keep the interval uniform at
-                // the cost of drift.
-                if (timer.isRepeats()) {
-                    addTimer(timer, currentTime + timer.getDelay());
-                }
-
-                // Allow other threads to call addTimer() and removeTimer()
-                // even when we are posting Timers like mad.  Since the wait()
-                // releases the lock, be sure not to maintain any state
-                // between iterations of the loop.
-
-                try {
-                    wait(1);
-                }
-                catch (InterruptedException e) {
-                }
+ 
+    public synchronized String toString() {
+   
+        StringBuilder sb = new StringBuilder();
+        sb.append("TimerQueue (");
+	Set<Timer> tset = timers.keySet();
+	Comparator<Timer> c = new Comparator<Timer>(){
+
+            public int compare(Timer t1, Timer t2){
+                int d1 = t1.getDelay();
+                int d2 = t2.getDelay();
+                if(d1 == d2) return 0;
+                else if(d1 < d2) return -1;
+                else return 1;
             }
-        } while (timeToWait <= 0);
-
-        return timeToWait;
-    }
-
-
-    public synchronized void run() {
-        long timeToWait;
 
-        try {
-            while (running) {
-                timeToWait = postExpiredTimers();
-                try {
-                    wait(timeToWait);
-                }
-                catch (InterruptedException e) {
-                }
-            }
-        }
-        catch (ThreadDeath td) {
-            running = false;
-            // Mark all the timers we contain as not being queued.
-            Timer timer = firstTimer;
-            while (timer != null) {
-                timer.cancelEvent();
-                timer = timer.nextTimer;
+            public boolean equals(Object o){
+                if(o == null) return false;
+                if(o.getClass() == getClass()) return true;
+                else return false;
             }
-            SystemEventQueueUtilities.restartTimerQueueThread();
-            throw td;
-        }
-    }
-
-
-    public synchronized String toString() {
-        StringBuffer buf;
-        Timer nextTimer;
-
-        buf = new StringBuffer();
-        buf.append("TimerQueue (");
-
-        nextTimer = firstTimer;
-        while (nextTimer != null) {
-            buf.append(nextTimer.toString());
-
-            nextTimer = nextTimer.nextTimer;
-            if (nextTimer != null) buf.append(", ");
-        }
-
-        buf.append(")");
-        return buf.toString();
+        };
+        List<Timer> tlist = new ArrayList<Timer>(tset);
+        Collections.sort(tlist, c);
+        for(Timer t: tlist)
+	    sb.append(t).append( " ,");
+        sb.delete(sb.length() - 2, sb.length());
+        sb.append(")");
+        return sb.toString();
     }
 }



JUnit TESTCASE :
import javax.swing.*;
import java.awt.event.*;
import javax.swing.event.*;
import static java.lang.System.*;
import java.util.*;


/**
 * BUG ID: 5053997 Swing Timer cannot handle multple timers effectively.
 * I had to look at this one at a small time scale to really get at what
 * it was saying.  Executing the timers, each one is separated by a call
 * to wait(1).  This call appears to be platform dependent upon how long
 * a task is supposed to wait.  On my platform the 10 millisecond wait
 * turns into 100 ms.  Ive done some radical surgery on the TimerQueue
 * and have reimplemented it with a ScheduledExecutorService.  This
 * has had the effect of cutting the wait period by about 90% on these
 * small times.  Instead of seeing 100ms wait periods I was seeing 10 to
 * 20 ms wait periods---> which appear to be very close to the mark.
 *
 * Reimplementing the TimerQueue in terms of the ScheduledExecutorService
 * simplifies the code significantly.  No longer is the class managing
 * all the Timers in its linked list for the management is done in the
 * SES.  In other words the specialty of the java.util.concurrent package
 * really shines here.
 *
 * TESTING STRATEGY:
 * First off this isn't a Junit style test case.  We are observing
 * timing behavior here, not does a == b and stuff.  So we must watch
 * and see.  What we see determines if we are hitting the mark.
 * 1. Execute the 5 timers at 10 MILLISECONDS, 10 times each.  See the
 * difference in timing between the old implementation and the new one.  Both
 * intial timings should be off, there appears to be some overhead associated
 * with getting things started up.  Once beyond the initial runs you can
 * see the time difference.
 * 2. Run a continuous loop that schedules Timers against a continously
 * running Timer.  This is an attempt to shake out a naughty timer that
 * is stoping other timers from getting scheduled.  From testing it on
 * my platform, I did not see such lock out.
 * #1 is executed by: java TestTimer
 * #2 is executed by: java TestTimer type what you want here.
 *
 * WHAT ELSE DOES THIS MEAN?
 * It may be possible to enhance the Timer so that it will repeat itself
 * based on the different time resolutions offered by TimeUnit.  something
 * like setResolution(TimeUnit tu).  I won't be helped by this with my
 * current platform(s) but someone with a snazzy OS may benefit.
 *
 *
 * FILES AFFECTED: javax.swing.TimerQueue
 *
 * JDK VERSION
 * jdk-6-rc-bin-b64-linux-i586-15_dec_2005.bin
 *
 * test ran succesfully on a SUSE 7.3 Linux distribution
 *
 * Brian Harry
 * ###@###.###
 * Jan 24, 2006
 */
public class TestTimer implements Runnable{

    boolean mthreadtest=false;
    public TestTimer(){}
    public TestTimer(String ... args){
	mthreadtest=true;
    }

    public void run(){
	JFrame jf = new JFrame();
	jf.setVisible(true);
	if(mthreadtest){
            javax.swing.Timer t = new javax.swing.Timer(10, new ActionListener3());
            t.start();
            Runnable run = new Runnable(){
		    public void run(){
            while(true){
                try{
		    Thread.currentThread().sleep(1);
                }
		catch(InterruptedException ie){}
		javax.swing.Timer t2 = new javax.swing.Timer(10, new ActionListener3());
                t2.setRepeats(false);
                t2.start();
            }
		    }
		};
	    new Thread(run).start();
        }
        else{
	    List<javax.swing.Timer> timers = new ArrayList<javax.swing.Timer>(10);
	    for(int i = 0; i < 5; i++ )
		timers.add( new javax.swing.Timer( 10, new ActionListener2(10)));
	    for(javax.swing.Timer t: timers)
		t.start();
        }

    }

    static class ActionListener2 implements ActionListener{
         
        long time;
        long period;
        int executions = 0;
        ActionListener2(long period){
            time = System.currentTimeMillis();
            this.period = period;
        }

[ snip ... refer to attached file 633847.txt for the rest ]

                                    

Comments
SUGGESTED FIX

Currently, the Timer services the queue by firing a single event and then calling wait(1) to make sure that other threads have the chance to get serviced and that we don't go into some infinite loop of servicing a non-ending stream of timers.  

I think a better way of doing this would be to see what the current list of ready-to-fire events is when we loop around, fire all of those events, and then perform wait() when we have finished that list (even if more events have been added in the meantime).  This would enable the system to service all timers that should be fired (according to the time and Timer.delay), but would also allow other threads to get serviced in turn.
                                     
2004-09-25
WORK AROUND


Applications can perform more work on single timers to reduce the number of timers being queued (and thus the number of times that wait() is called)
                                     
2004-09-25
EVALUATION

Contribution-forum:https://jdk-collaboration.dev.java.net/servlets/ProjectForumMessageView?forumID=1463&messageID=11013
                                     
2006-01-24
EVALUATION

See also related bug 6452147 about Timer granularity.
                                     
2006-07-25
EVALUATION

There are multiple things which affect Timer performance.
1. The current synchronization is far from optimal. Everything is synchronized on TimerQueue instance.
   TimerQueue.postExpiredTimer uses artificial wait(1) to allow other thread to access TimerQueue. 
2. System.currentTimeMillis() might not provide best granularity for time measurement.

To resolve these issues I suggest to:
- synchronize on individual Timer instance instead of TimerQueue instance
- use DelayQueue to hold the Timers instead of custom linked list. DelayQueue does exactly what we need.
(note : DelayQueue is thread-safe)

javadoc for DelayQueue
/* An unbounded {@linkplain BlockingQueue blocking queue} of
 * <tt>Delayed</tt> elements, in which an element can only be taken
 * when its delay has expired.  The <em>head</em> of the queue is that
 * <tt>Delayed</tt> element whose delay expired furthest in the
 * past.  If no delay has expired there is no head and <tt>poll</tt>
 * will return <tt>null</tt>. Expiration occurs when an element's
 * <tt>getDelay(TimeUnit.NANOSECONDS)</tt> method returns a value less
 * than or equal to zero.
 */

- use System.nanoTime() which gives the best granularity for time measurement.
                                     
2006-09-12
EVALUATION

The fix for this bug introduced a regression: 6623943 [javax.swing.TimerQueue's thread occasionally fails to start]
                                     
2007-11-12



Hardware and Software, Engineered to Work Together