[Mono-dev] Single thread scheduler for Threading.Timers patch

[Jonathan Gilbert]

At 11:04 PM 13/06/2006 -0700, Rafael Ferreira wrote:
>Howdy, 
>
>The attached patch changes the current Threading.Timer class to use a
>single thread scheduler instead of the current 1 thread per timer logic.
>I also spent a lot of time working on the Timer unit tests so they more
>consistently pass as well as fixing the "NotWorking" tests. 
>
>Some key features include:
>
>* A single thread handles firing all timer jobs thus allowing a much
>greater number of Timers to be defined - Fixing bug #65734
>* Timer scheduler is only started after the first System.Threading.Timer
>is created (lazy init)
>* Timer scheduler thread dies if there are no more timer jobs in its Job
>queue (early termination)
>* Scheduler can spit out debug info by exporting the MONO_TIMER_DEBUG
>environment variable

One thing that gets me about the design is the way that timer events are
marshalled to a different thread (in the thread pool) in order to be fired.
Obviously, you don't want synchronous callbacks from a single thread for
all timers, but perhaps a timer thread pool, each of which handling a
subset of the timers, would be a viable alternate design. With a limit of,
say, 100 scheduler threads, up to 100 timers could be created without any
chance of interference, and after that point, the threads would be reused
instead of overloading the system with thousands of threads. The threads
could even switch from synchronous callbacks when they are handling only a
single timer to asynchronous thread pool callbacks when their
responsibilities increase.

If we accept the single-thread thread-pool-callback design, though, I have
the following comments on the implementation:

1. A heap structure is very easy to maintain, and would be a significantly
more efficient way to find the next event to be fired. You have comments
that acknowledge the deficiency of looping over the entire set every time,
and I think this would be the logical next step in this.

2. Using Thread.Abort to signal the thread is fundamentally flawed. A user
very quickly adding & removing timers would eventually cause a
ThreadAbortException to fire inside the 'catch' handler, killing off the
scheduler thread and disabling all timers. A better approach would be to
use Monitor.Wait on a synchronization object in the scheduler thread with
the correct time-out, and then Monitor.Pulse to awaken the thread for an
update. The scheduler can use DateTime.Now comparisons to determine, when
Monitor.Wait returns, how long it actually waited and whether it should
actually fire the event at top of the heap, and the return value of
Monitor.Wait will indicate whether it was interrupted and thus should check
the queue of timer additions/deletions before proceeding.

If you'd like, I can try writing an alternate implementation along these
lines, but it will have to wait until after work for me today.

Jonathan Gilbert