Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread-Safe TimedPhase #185

Open
archolewa opened this issue Mar 1, 2017 · 1 comment
Open

Thread-Safe TimedPhase #185

archolewa opened this issue Mar 1, 2017 · 1 comment

Comments

@archolewa
Copy link
Contributor

We should presumably make TimedPhase thread-safe (though starting and stopping a TimedPhase multiple times is probably abusing the concept). Unfortunately, making TimedPhase thread-safe is tricky because whether or not the timer is running becomes a much trickier concept in a world where any thread could start or stop any TimedPhase at any time.

Therefore, if we wish to make the TimedPhase thread-safe, we should probably also impose a restriction: ATimedPhase can only be started once, and stopped once. Then we provide some utility methods for creating a new TimedPhase with a different name, and the same duration as the original. So something like:

  TimedPhase newTimer(String suffix) {
    if (suffix.equals("") {
           throw new IllegalArgumentException();
   }
    return new TimedPhase(getName() ++ suffix, duration);
   }

Then, if people want to run a timer multiple times, they can do it through a relay style timing, where each timer still has a well-defined notion of started and stopped.

@cdeszaq
Copy link
Collaborator

cdeszaq commented Mar 10, 2017

I think making TimedPhase a 1-time-use thing is probably a great idea. There's definitely some more thinking to do around what the multi-start capabilities were allowing for, and seeing if it still makes sense (and if we can continue to support it), but the thread-safety is more important I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants