views:

342

answers:

11

I am wanting to create an internal messaging system that can tell me the duration of some code being called. I was thinking for ease of use, to make the SystemMessage class implement IDisposable.

I would set a time stamp during the SystemMessage's constructor and if the Dispose was called, I could figure out the duration.

The problem is that I do not want to have the object GC'ed. I want it to stay around as part of a MessageCollection.

Is there another construct in C# that can give me the usability of the Using Statement without stepping on the intended function of IDisposable.

Using (message = Collection.CreateNewMessage("FileDownlading"))
{
    // I wonder how long it is taking me to download this file in production?
    // Lets log it in a message and store for later pondering.
    WebClass.DownloadAFile("You Know This File Is Great.XML");
}
// we fell out of the using statement, message will figure out how long
// it actually took to run.
// This was clean and easy to implement, but so wrong?
+1  A: 

I don't think using is what you want here. Why not just have the constructor record the time, and then when DownloadAFile is called log the time delta? In your example, if there's an exception, it will log the time of the exception as though the file was downloaded then.

If you actually want the behavior to be like your example, just use a try / finally block, and do the logging in the finally. using is just syntactic sugar for a try / finally block and a call to Dispose.

The equivalent code to your example looks like this:

try 
{
    var message = Collection.CreateNewMessage("FileDownlading"); 
    //...
    WebClass.DownloadAFile("You Know This File Is Great.XML");
}
finally 
{
    //You can change this to do logging instead.
    message.Dispose(); 
}
RossFabricant
I am looking for the Syntactic Sugar here... Trying to make it as easy for use in many places as possible. If my choices are improperly use using or make the user call message.EndTime(), I will use the EndTime() method.
Keith Sirmons
+1  A: 

You can use a System.Diagnostics.Stopwatch inside DownloadAFile() to do the timing every time it's called.

Or, just add the stopwatch code around the call to DownloadAFile() (depending on how you want this to work).

Using IDisposable in this case would not be a good idea.

Jon B
I had a hard time finding a solid answer, but it's unclear if the StopWatch uses DateTime fields to store the elapsed time or if it uses a background thread to run a timer (similar to System.Timers.Timer). If it does use a thread then you face severe scalability issues due to the context switching required to keep each message's timer counting.
STW
@Yoooder: I can't imagine it using a background thread when all it really needs to do is calculate the difference between two tick counts. However, like you, I couldn't find solid documentation on this.
Jon B
This looks exactly what was asked for from what I read.
Justin Drury
I did some quick tests and sure enough it looks like StopWatch does not use threads, unlike a Timer. It makes sense I suppose as a timer needs to raise an event at specified intervals and can't wait on blocking--whereas a stopwatch just needs to know when it started and when it stopped.
STW
A: 

Not really. The closest you can come is something like this (which is what is happening under the hood of the using() statement anyway:)

var message = Collection.CreateNewMessage("FileDownloading")

try
{
    WebClass.DownloadAFile("You Know This File Is Great.XML");
}
finally
{
    message.HowLongHaveIBeenAlive();
}
mquander
This is what I want to happen, I just don't want to have to sprinkle the try finally parts throughout the code.
Keith Sirmons
A: 

The using statement is meant to dispose of objects once you are finished with them. The idea of holding on to an object after it has been disposed is not exactly intended. You might try and do something as simple as:

message = Collection.CreateNewMessage("FileDownlading");
DateTime dtStart = DateTime.Now;
WebClass.DownloadAFile("You Know This File Is Great.XML");
DateTime dtEnd = DateTime.Now;

// perform comparison here to see how long it took.

// dispose of DateTimes
dtStart = dtEnd = null;
Scott Anderson
+2  A: 

Are you looking for something akin to closures?

http://en.wikipedia.org/wiki/Closure_(computer_science)

You could fake something... like this...

private TimeSpan GetDuration(Action a)
        {
            var start = DateTime.Now;
            a.Invoke();
            var end = DateTime.Now;
            return end.Subtract(start);
        }

        public void something()
        {
            string message;
            var timeSpan = GetDuration(() => { message = "Hello"; } );
        }
Min
A: 

Originally IDisposable was intended as a way to introduce deterministic clean-up in C#, but I have seen authors and implementations use the using/Dispose feature for something along the lines of what you are talking about.

Personally, I'm not too happy about it as it undermines the concept behind IDisposable, but since there's isn't an alternative it comes down to a matter of how orthodox you want to be. I can certainly see why you want to do it, but by doing so you make it harder to explain the purpose and the importance of the IDisposable interface.

Brian Rasmussen
The underlying issue of using Dispose() for resetting an object rather than preparing it for GC is that there is no other well known interface specifically for getting an object ready for GC. Also, the convention that calling methods on an object which has been disposed should through an "ObjectDisposedException" means that recycling a Disposed object is very non-standard.
STW
...although you could carefully implement the IDispose pattern and ignore the convention of the Exception. The issue is that future developers will need to know that Dispose needs to perform both purposes--and also consumers will need to know that even though it is disposed it is reusable.
STW
A: 

It could be argued that this is an abuse of the using construct, so I probably wouldn't implement a public class in a shared class library that uses IDisposable in this way.

But I've seen this kind of thing done and think it is OK if it stays internal to your application.

The problem is that I do not want to have the object GC'ed. I want it to stay around as part of a MessageCollection.

I don't understand this at all. IDisposable has nothing to do with GC, and your message will stay alive if it's referenced as an element of your MessageCollection.

Your message class might look something like the sample below. After Dispose is called, it remains alive and well. Most classes that implement IDisposable are unusable after Dispose is called, and their implementation therefore throws an ObjectDisposedException if members are accessed after calling Dispose. But this is by no means mandatory.

class Message : IDisposable
{
    private Stopwatch _stopwatch = Stopwatch.StartNew();
    private long _elapsedTicks;
    private string _message;

    public Message(string message)
    {
        _message = message;
    }

    public void Dispose()
    {
       _elapsedTicks = _stopwatch.ElapsedTicks;
       ... anything else including logging the message ...
    }

    ...
}
Joe
You are correct that IDisposable doesn't technically call GC directly, however most developers don't know that--and if you ask a developer when they call Dispose and what they expect it to do they will say "clean up an object and prepare it for GC". Technically you can use it to reset the state of the object, however this is a terrible practice since it is a very unexpected behavior.
STW
You are incorrect to say that Dispose will "clean up an object and prepare it for GC". The usual use of Dispose is to deterministically release unmanaged resources. The only thing that "prepares an object for GC" (if there is even such a thing) is the last reference to the object going out of scope.
Joe
I understand that, however the convention (and generally understood usage of Dispose) is to prepare an object for GC. Also there are numerous coding standards and guidelines dictating that after Dispose() is called any method call on the object should throw an ObjectDisposedException.While you can technically recycle a Disposed object it is a very unexpected, and advised-against, practice. Reserving Dispose() for its conventional purpose is MUCH safer and more approachable.
STW
I agree with you that this would be an abuse of IDisposable, hence my comment that I wouldn't use it in a public API. Neither am I advocating recycling disposed object. And whatever that straw man "most developers" thinks, disposing an object does not prepare it for GC.
Joe
+3  A: 

The problem is that I do not want to have the object GC'ed. I want it to stay around as part of a MessageCollection.

Calling Dispose doesn't cause the object to be GC'ed - that happens when the GC does a sweep and nothing is referencing it. If you're still referencing the object via MessageCollection, it'll stick around.

Dispose can keep it from being Finalized, but since you're not using Dispose to clean up resources, you won't have a Finalizer and you won't care.

So, really the only problem is the confusing semantics around having your calss implement IDisposable even though there's no resources to dispose of.

Personally, I don't see it as a problem. If consumers call Dispose, then fine - they get the time logging. If they don't, then they don't get itme stamps and the worst that happens is they get an FxCop violation.

It is, however, a bit unintuitive - so if this is for public use, I'd suggest offering a more discoverable alternative like:

// C# 3+ lambda syntax
Collection.CreateNewMessage("FileDownlading", () => {
    // I wonder how long it is taking me to download this file in production?    
    // Lets log it in a message and store for later pondering.    
    WebClass.DownloadAFile("You Know This File Is Great.XML");
});

// C# 2 anonymous delegate syntax
Collection.CreateNewMessage("FileDownlading", delegate() {
    // I wonder how long it is taking me to download this file in production?    
    // Lets log it in a message and store for later pondering.    
    WebClass.DownloadAFile("You Know This File Is Great.XML");
});

// Method
void CreateNewMessage(string name, Action action) {
   StopWatch sw = StopWatch.StartNew();
   try {
      action();
   } finally {
      Log("{0} took {1}ms", name, sw.ElapsedMilliseconds);
   }
}

which would run and time an Action delegate instead.

Mark Brackett
This looks promising. Does this syntax work in C# 2.0?
Keith Sirmons
That specific syntax would not (it's a lambda expression) but it's basically a delegate so you could do the equivalent in 2.0 easily enough.
GalacticCowboy
Get this error using the C#2 syntax: "Cannot convert anonymous method to type 'System.Delegate' because it is not a delegate type". Found this page: http://staceyw.spaces.live.com/blog/cns!F4A38E96E598161E!1042.entry
Keith Sirmons
A: 

The 'using' statement actually compiles down to a Try/Finally that expects the object to implement IDisposable--"using" is just a shortcut provided by the language.

For your purposes, especially since you want to recycle the object, I would consider writing your own interface. When you call a method to start timing just grab the value of datetime.now; and again when you stop the timer. Then subtract starttime from stoptime and you'll have the duration.

Avoid using a true Timer object on each class instance. Timer's use a ThreadPool thread, which means that each message will consume at least one thread--if too many messages are in the system your application will slow down as a result of the thread switching. Also, if the timer's aren't properly disposed they may not release their ThreadPool threads and you will essentially have a thread leak.

STW
A Stopwatch is more accurate than using DateTime. If you do want to use DateTime (e.g. .NET 1.x), prefer UtcNow to Now as it is not subject to jumps at the start/end of daylight savings time.
Joe
Do you know if a StopWatch uses a worker thread to time duration? I'm curious now and haven't been able to find an answer.
STW
A little research later and it looks like StopWatches do not use worker threads so you're correct that they should be ideal for this type of job.
STW
A: 

After reviewing your question again I don't see how the object in question is really a Message, unless it is more like a tracing message (for debugging assistance).

If you're looking for something more along those lines then here is a very rough approach using delegates. Essentially you create a delegate for each method you want to call and time, then pass the delegate and the method arguments off to a helper who is responsible for the actual calling of the method and timing its duration.

The obvious shortcoming in my example is that I sacrificed type-safe arguments since I wasn't sure if it was exactly what you're looking for or not. The other issue is that you would need to add a new delegate every time you want to call a method which has a method signature that you don't already have a delegate for:

using System;

namespace ConsoleApplication4
{
 class Program
 {
  static void Main(string[] args)
  {

   SomeCaller callerOne;
   YetAnotherCaller callerTwo;

   callerOne = new SomeCaller(SomeMethod);
   LogCallDuration(callerOne, new object[] { 15 });

   callerOne = new SomeCaller(SomeOtherMethod);
   LogCallDuration(callerOne, new object[] { 22 });

   callerTwo = new YetAnotherCaller(YetAnotherMethod);
   LogCallDuration(callerTwo, null);

   Console.ReadKey();
  }

  #region "Supporting Methods/Delegates"

  delegate void SomeCaller(int someArg);
  delegate void YetAnotherCaller();

  static void LogCallDuration(Delegate targetMethod, object[] args)
  {
   DateTime start = DateTime.UtcNow;
   targetMethod.DynamicInvoke(args);
   DateTime stop = DateTime.UtcNow;

   TimeSpan duration = stop - start;

   Console.WriteLine(string.Format("Method '{0}' took {1}ms to complete", targetMethod.Method.Name, duration.Milliseconds));

  }

  #endregion "Supporting Methods/Delegates"

  #region "Target methods, these don't have to be in your code"
  static void SomeMethod(int someArg)
  {
   // Do something that takes a little time
   System.Threading.Thread.Sleep(1 + someArg);
  }

  static void SomeOtherMethod(int someArg)
  {
   // Do something that takes a little time
   System.Threading.Thread.Sleep(320 - someArg);
  }

  static void YetAnotherMethod()
  {
   // Do something that takes a little time
   System.Threading.Thread.Sleep(150);
  }
  #endregion "Target methods"
 }
}
STW
+1  A: 

I have been looking at this lately and perhaps PostSharp can help you. It lets you decorate a method with an attribute that will be called when your method starts and when it stops.

http://www.postsharp.org/

I'm not sure it will do what you like, but it's worth investigation and it has that "syntactic sugar" you're craving!

Chris

Chris Dunaway
This looks like it might work for what I am trying to do. Thank you.
Keith Sirmons