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

Suggestion: Actions Exposed as Async #64

Open
tomtzook opened this issue Nov 24, 2020 · 1 comment
Open

Suggestion: Actions Exposed as Async #64

tomtzook opened this issue Nov 24, 2020 · 1 comment

Comments

@tomtzook
Copy link
Member

tomtzook commented Nov 24, 2020

Actions under flashlib.core.scheduling are tasks which a specific run flow which are executed by the scheduler. For a long time, that execution happened on the robot main thread, with the use of a synchronous scheduler implementation. With the possible introduction of multithreading into future implementations of scheduler, it is time to consider updating the actions API to expose async semantics.

Although it is true that actions were sync in the sense that there was no multithreading involved; they are not actually sync, as in a call to start did not block until the action finished. Instead, the action was left to execute whenever after started.

action.start();  // action runs in the background
// more user code unrelated

Due to being async it is usually necessary to provide some way to interact with actions when needed. This does exist with some methods in Scheduler, like Scheduler.cancel and such. This is severely limiting however, as exposing certain additional functionalities will complicate the implementation of Scheduler, especially in a multithreaded environment. In addition, it either forces user to interact with the Scheduler directly at times, which is unwanted:

Action action = ...
Time runtime = RunningRobot.getControl().getScheduler().getRunTime(action);

or adds more methods to Action/ActionBase to hid that requirement, which fills the interface/class with wrapping code.

public interface Action {
...
    Time getRunTime();
...
}

public abstract class ActionBase implements Action {
...
    @Override
    public Time getRunTime() {
        return scheduler.getRunTime(this);
    }
...
}

Thus allowing:

Action action = ...
Time runtime action.getRunTime();

Further more. this kind of API is not desirable since it is not immutable for Action, as it can only be used if the action is running. And this could be amplified with different kinds of information that Action could be required to supply - such as a return value.

Proposal

With inspiration from Java's ExecutorService API, I propose to use a Future-like object (or actually use Future) to facilitate aysnc semantics:

public interface Scheduler {
...
    public Future start(Action action);
...
}

public interface Action {
...
    public Future start();
...
}

And usage:

Future future = action.start();
... // some other code here
Time time = future.getRunTime();
future.wait(); // wait for action

Since Future is a contained object representing a single Action, implementations of Scheduler should be able to find it easy to integrate it.

With the new API, Future can be packed with methods to query information about the execution of Action without clattering the Scheduler and Action interfaces.

This also opens the way for Actions to produce results (like Callable) and allows users to handle exceptions and perhaps more.

java.util.concurrent.Future vs Custom Future

After seeing the advantages of using a Future-like interface with the Actions API; we are left with choosing whether or not to use the java.util.concurrent.Future provided with the ExecutorService API from Java.

There are a number of advantages for using it:

  • Interface and implementations (like CompletableFuture) already exist, so less code to write.
  • Well established and used, so people are generally familiar with it.
  • Can be combined with other APIs with them needing to know about FlashLib, or using some adapters.

However, there are some disadvantages:

  • Limited to the strict API provided by Future, so adding custom methods might be problematic.
  • Throws a bunch of checked exceptions, which is something we generally want to avoid in FlashLib to make it more friendly to beginners.
  • Generic, and thus might require FlashLib to be generic.

All in all, it's hard to say which to choose. So as of yet, it is undecided.

@tomtzook tomtzook self-assigned this Nov 24, 2020
@tomtzook tomtzook changed the title Actions Exposed as Async Suggestion: Actions Exposed as Async Nov 24, 2020
@tomtzook
Copy link
Member Author

Due to Actions being stateful objects, there can only be one Future at any given moment. The ExecutorService API's Future object is meant to be temporary, which makes since because the tasks are single-use and supposed to be state-less.

For Actions, the Future can and probably should be linked to the Action object. This can be done by linking the Future to an internal scheduler state.

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

No branches or pull requests

1 participant