UnityTimer icon indicating copy to clipboard operation
UnityTimer copied to clipboard

Pool Timers ?

Open slender9168 opened this issue 5 years ago • 2 comments

Thanks for the incredible library. I wanted to double check if timers are pooled & if there are any plans to pool them ?

slender9168 avatar Mar 31 '19 16:03 slender9168

Hey thanks!

Currently the timers are not pooled. I thought about it for a bit and I think there are some situations that might make it really annoying to pool them. I've never tried pooling before so maybe I'm overestimating how tricky this situation is to address. Let's say you have code like this:

class MyScript : MonoBehaviour {
  private Timer myTimer;

  private void DoSomeTimedThing() {
    if (myTimer != null && !myTimer.isDone) {
      return;
    }
    myTimer = Timer.Register(...);
  } 
}

If between two calls of DoSomeTimedThing, myTimer finishes and gets reused by some other script, myTimer.isDone will return false and DoSomeTimedThing won't run even though it should.

There are a few ways I think to work around this but I'm not a fan of them:

  1. Force MyScript to understand that timers are pooled and clean up the myTimer state in their timer callback, so myTimer will always be null if the timer is not running. I think this puts too much responsibility on the user of the API.

  2. Create a new PoolRegister method or something of the sort that doesn't return a Timer object so this is no longer an issue. This could work, but seems janky to me.

  3. Make the MonoBehaviour.AttachTimer extension method the only way of creating pooled timers, so the Timer knows its owner and can't be reused until the owner is destroyed. This makes it too easy for a MonoBehavior with a long lifespan to monopolize the pool by creating a bunch of short timers.

I'll keep thinking about it, but if this is causing issues for your project it might be easiest for you to change your local copy of the library to use a pool instead (e.g. by not returning a Timer from Register or being careful in your timer callbacks to clean up so you can't read data from a reused Timer). Here are the lines that add and remove timers from the list. I could also create a fork of the library that uses a pool instead.

akbiggs avatar Mar 31 '19 18:03 akbiggs

This might be doable without API changes through a design like this:

  1. Timer becomes an interface with two implementations: TimerImpl (which is what Timer is today) and ProxyTimer. ProxyTimer simply delegates all its interface method calls to another Timer instance which can be changed by the TimerManager via a ProxyTimer::SetProxy method.

  2. When a ProxyTimer is done, TimerManager calls ProxyTimer::SetProxy with a deadTimer instance that always returns true for isDone and other methods.

But this would still require allocating ProxyTimer instances, and I think that defeats the point if the goal is to avoid having the GC run at all by using Timer. Also I'm not sure if methods like GetTimeElapsed could still work after the timer completes.

akbiggs avatar Mar 31 '19 20:03 akbiggs