Can my use of System.Threading.Timer be more efficient

Question

I´m looking for some advice about making my code more efficient. What I would like to do is to have a System.Threading.Timer that runs some job every hour or so, the job is not going to be very heavy but I would like to have a code that does not take much of resources. I plan to run this code in a windows service.

This is what I have so far.

class Program
{
    private static Timer timer;

    static void Main(string[] args)
    {
        SetTimer();
    }

    static void SetTimer()
    {
        timer = new Timer(Write);

        var next = DateTime.Now.AddHours(1);

        var nextSync = (int)(next - DateTime.Now).TotalMilliseconds;

        timer.Change(nextSync, Timeout.Infinite);
    }

    static void Write(object data)
    {
        Console.WriteLine("foo");

        SetTimer(); //Call the SetTimer again for the next run.
    }
}

What do you guys think? Can I make my code more efficient?

All advice is much appreciated!

Every hour? I wouldn't even bother with a windows service, console app + scheduled task would suffice. — James, Apr 25 '13 at 17:27
Efficiency is the measure of work performed divided by resources consumed. How are you measuring work and how are you measuring resources? **If you are not measuring something then you do not know whether you are improving or not**. — Eric Lippert, Apr 25 '13 at 17:27
Any design that combines a timer with a Windows service is almost certainly misguided. See [here](http://stackoverflow.com/a/246839/366904) for more information. — Cody Gray, Apr 25 '13 at 17:33
@EricLippert - What I mean with efficiency here is the design and the using av Timers. — Lars., Apr 25 '13 at 17:36
@Lars and what Eric is getting at is if you don't have any sort of benchmark then how do you know if your change is *better*? This has got premature optimisation written all over it. — James, Apr 25 '13 at 17:41
@Cody: That statement is much too general. It may be misguided to build a service that has nothing but a timer, but there's nothing wrong with using timers inside services. (Classic example: a service processing client transactions that arrive over the network, and also building a summary report every hour). The key is to put the timer inside a service that already exists (which might be the Windows-provided "Task Scheduler" but could be any existing custom service). There's also a time-space tradeoff -- keeping state in memory saves recreating it, which could be expensive. — Ben Voigt, Apr 25 '13 at 17:52
I dont believe it´s wrong to run Timers in windows services, as long as they are used right, wich is what I was asking advice for. — Lars., Apr 25 '13 at 18:02

score 5 · Accepted Answer · edited Feb 12 '21 at 17:20

5

Several points:

You do not have to create a new timer every hour.
Setting the second parameter to infinite, makes you have to reload the timer manually. But... In this case, why should you?
You make a difficult calculation to create a timespan from one hours form now: now + 1 hour - now. This can solved easily.

Try this:

class Program
{
    private static Timer timer = new Timer(Write, null, TimeSpan.FromHours(1), TimeSpan.FromHours(1));

    static void Main(string[] args)
    {
    }

    static void Write(object data)
    {
        Console.WriteLine("foo");
    }
}

edited Feb 12 '21 at 17:20

Theodor Zoulias

15,834
3
19
54

answered Apr 25 '13 at 17:27

Martin Mulder

11,592
3
19
50

What do you think about threading issues? Do you think I should be concerned about running my code in threads? Or with AutoResetEvent? – Lars. Apr 25 '13 at 18:06
It depends. Are you using multiple threads for multiple tasks? And if so, are those thread sharing the same variables or resources? If your answer is yes, then you should worry about synchronizing access to your data or resources. Depending of the type of sharing, you should consider the correct solution of which AutoResetEvent could be one. – Martin Mulder Apr 25 '13 at 18:10

score 1 · Answer 2 · answered Apr 25 '13 at 17:26

1

This is not good, since you create and abandon a brand new timer each iteration. Move

timer = new Timer(Write);

into Main so that it only executes once, then SetTimer can reuse this single Timer object.

answered Apr 25 '13 at 17:26

Ben Voigt

260,885
36
380
671

I´m going to run this code in a windows service, so the creation of the timer would go in too the Start method. – Lars. Apr 25 '13 at 17:28
@Lars: Certainly, you have to put it into your real code. The code in your question had `Main` rather than `Start`. – Ben Voigt Apr 25 '13 at 17:28
@Lars as Ben said, regardless of what the outer most function is, the timer allocation should be there and not in the `SetTimer` method. – evanmcdonnal Apr 25 '13 at 17:36
What do you think about threading issues? Do you think I should be concerned about running my code in threads? Or with AutoResetEvent? – Lars. Apr 25 '13 at 18:06
1

@Lars: I avoid threads unless I see an actual benefit. I much prefer to code in an event-driven style, with a single thread running an event dispatch loop (just like the GUI uses), and short non-blocking action in response to each external event, and think about state and context switching explicitly. This makes it much easier to maintain invariants, and because you don't need locking, it's actually higher performance (except on CPU-bound computations). – Ben Voigt Apr 25 '13 at 18:30

score 0 · Answer 3 · answered Apr 25 '13 at 17:29

0

In WPF:

DispatcherTimer timer = new DispatcherTimer();

timer.Tick += timer_Tick;
timer.Interval = = new TimeSpan(1, 0, 0); //once an hour
timer.Start();

void timer_Tick(object sender, EventArgs e)
{
     //do your updates
}

answered Apr 25 '13 at 17:29

morishuz

2,052
4
19
21

Can my use of System.Threading.Timer be more efficient

3 Answers3