Simple Java task scheduler

The task is pretty simple: implement a timer that executes tasks when the next event takes place. I created a task scheduler based on a priority queue.

• A new thread is created for TaskTimerRunnable and sleeps during run() until the next task.

• When a new task is added, it checks if the time for the new task is sooner than the soonest time in the priority queue and interrupt the timer thread to start the loop again.

• Each task is executed in a separate thread from a thread pool.

Is there any way to improve my code?

```
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.PriorityBlockingQueue;

public class TaskScheduler {

interface Task extends Runnable {
}

private static class TaskTime implements Comparable {
public long timestampMilis;
public Task task;

TaskTime(long timestamp, Task task) {
this.task = task;
this.timestampMilis = timestamp;
}

@Override
public int compareTo(TaskTime task) {
long diff = timestampMilis - task.timestampMilis;

if (diff > 0) return 1;

return diff == 0 ? 0 : -1;
}
}

public static class TimerQueue {
private BlockingQueue queue = new PriorityBlockingQueue<>();

public void addTask(long timestampMillis, Task task) {
queue.add(new TaskTime(timestampMillis, task));
}

public long getSonnestTimestampMillis() {
TaskTime taskTime = queue.peek();
if (taskTime == null) return 0;
return taskTime.timestampMilis;
}

private Task popSoonestTask() {
TaskTime taskTime = queue.poll();
if (taskTime == null) return null;
return taskTime.task;
}
}

public static class TaskTimerRunnable implements Runnable {
private static final int MAX_THREADS = 100;

Typos

private static class TaskTime implements Comparable {
    public long timestampMilis;
    // ...
}

timestampMilis should be timestampMillis, and it's missing the final modifier if you want it be publicly accessible in an immutable way.

public long getSonnestTimestampMillis() {
    TaskTime taskTime = queue.peek();
    if (taskTime == null) return 0;
    return taskTime.timestampMilis;
}

The method name should be getSoonestTimestampMillis(). Actually, if you think 'soonest' is a mouthful, you can also consider 'next'.

compareTo()

In addition to @TheCoffeeCup's answer, there's Long.compare(long, long) if you really prefer to return the range [-1, 1].

No more work

// inside TimerQueue
private Task popSoonestTask() {
    TaskTime taskTime = queue.poll();
    if (taskTime == null) return null;
    return taskTime.task;
}

// inside TaskTimerRunnable
if (soonestTimestampMillis <= currentTimestampMillis) {
    pool.submit(queue.popSoonestTask());
} else {
    // ...
}

The problem here is that the caller simply passes the potential null result to the pool, which will result in a NullPointerException. You should check for this first.

Rip Van Winkle

private void sleep(long millis) {
    try {
        if (millis <= 0) {
            Thread.sleep(Long.MAX_VALUE);
        } else {
            Thread.sleep(millis);
        }
    } catch (InterruptedException e) {
        return;
    }
}

I'm not sure if you really want your program to sleep() for \$2^{63} - 1\$ milliseconds if it receives a <= 0 value. What if your program is told to launch just one task one second ago?

Scheduling an alternative

Instead of using a regular fixed-size thread-pool-backed ExecutorService, why not consider ScheduledExecutorService? Even though its APIs deal with relative timings and you are trying to use absolute timings, its Javadoc describes a possible simple transformation with some caveats:


All schedule methods accept relative delays and periods as arguments, not absolute times or dates. It is a simple matter to transform an absolute time represented as a Date to the required form. For example, to schedule at a certain future date, you can use: schedule(task, date.getTime() - System.currentTimeMillis(), TimeUnit.MILLISECONDS). Beware however that expiration of a relative delay need not coincide with the current Date at which the task is enabled due to network time synchronization protocols, clock drift, or other factors.

StackExchange Code Review Q#117716, answer score: 2