From bf6401a8677938f34fa5d9a40e087f26634c8c50 Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Wed, 29 Nov 2023 17:08:06 -0800 Subject: [PATCH] A refined plan - Clean up and improve functional interfaces used in Plans. - Allow safely running TaskExecutor#sync* methods off-thread. - Formalize the concept of "main thread" in task executor. - Improve tests for main-thread plans. --- .../flywheel/api/task/TaskExecutor.java | 18 ++++-- .../flywheel/impl/task/FlwTaskExecutor.java | 3 +- .../impl/task/ParallelTaskExecutor.java | 63 ++++++++++++++----- .../impl/task/SerialTaskExecutor.java | 7 ++- .../impl/visualization/storage/Storage.java | 6 +- .../storage/VisualUpdatePlan.java | 2 +- .../flywheel/lib/task/ContextConsumer.java | 13 ---- .../flywheel/lib/task/ContextFunction.java | 13 ---- .../flywheel/lib/task/ContextRunnable.java | 16 ----- .../flywheel/lib/task/ContextSupplier.java | 17 ----- .../flywheel/lib/task/DynamicNestedPlan.java | 12 ++-- .../flywheel/lib/task/ForEachPlan.java | 24 ++++--- .../flywheel/lib/task/ForEachSlicePlan.java | 24 +++++-- .../flywheel/lib/task/IfElsePlan.java | 18 +++--- .../flywheel/lib/task/MapContextPlan.java | 15 +++-- .../flywheel/lib/task/NestedPlan.java | 3 +- .../flywheel/lib/task/SimplePlan.java | 15 +++-- .../flywheel/lib/task/SyncedPlan.java | 16 +++-- .../BooleanSupplierWithContext.java | 35 +++++++++++ .../task/functional/ConsumerWithContext.java | 34 ++++++++++ .../task/functional/RunnableWithContext.java | 35 +++++++++++ .../task/functional/SupplierWithContext.java | 38 +++++++++++ .../lib/task/functional/package-info.java | 8 +++ .../flywheel/lib/task/PlanExecutionTest.java | 48 +++++++++----- .../lib/task/PlanSimplificationTest.java | 3 +- 25 files changed, 340 insertions(+), 146 deletions(-) delete mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/ContextConsumer.java delete mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/ContextFunction.java delete mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/ContextRunnable.java delete mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/ContextSupplier.java create mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/functional/BooleanSupplierWithContext.java create mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/functional/ConsumerWithContext.java create mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/functional/RunnableWithContext.java create mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/functional/SupplierWithContext.java create mode 100644 src/main/java/com/jozufozu/flywheel/lib/task/functional/package-info.java diff --git a/src/main/java/com/jozufozu/flywheel/api/task/TaskExecutor.java b/src/main/java/com/jozufozu/flywheel/api/task/TaskExecutor.java index 6a67b8b51..92298d28e 100644 --- a/src/main/java/com/jozufozu/flywheel/api/task/TaskExecutor.java +++ b/src/main/java/com/jozufozu/flywheel/api/task/TaskExecutor.java @@ -28,6 +28,8 @@ public interface TaskExecutor extends Executor { * Wait for running tasks, so long as the given condition is met * ({@link BooleanSupplier#getAsBoolean()} returns {@code true}). *
+ * If this method is called on the + *
* This method is equivalent to {@code syncUntil(() -> !cond.getAsBoolean())}. * * @param cond The condition sync on. @@ -48,10 +50,18 @@ public interface TaskExecutor extends Executor { /** * Schedule a task to be run on the main thread. *
- * This method may be called from any thread, but the runnable will only - * be executed once somebody calls either {@link #syncPoint()} or - * {@link #syncUntil(BooleanSupplier)}. + * This method may be called from any thread (including the main thread), + * but the runnable will only be executed once somebody calls + * either {@link #syncPoint()} or {@link #syncUntil(BooleanSupplier)} + * on this task executor's main thread. * @param runnable The task to run. */ - void scheduleForSync(Runnable runnable); + void scheduleForMainThread(Runnable runnable); + + /** + * Check whether the current thread is this task executor's main thread. + * + * @return {@code true} if the current thread is the main thread. + */ + boolean isMainThread(); } diff --git a/src/main/java/com/jozufozu/flywheel/impl/task/FlwTaskExecutor.java b/src/main/java/com/jozufozu/flywheel/impl/task/FlwTaskExecutor.java index 785fac938..da404ab7d 100644 --- a/src/main/java/com/jozufozu/flywheel/impl/task/FlwTaskExecutor.java +++ b/src/main/java/com/jozufozu/flywheel/impl/task/FlwTaskExecutor.java @@ -4,6 +4,7 @@ import org.apache.commons.lang3.concurrent.AtomicSafeInitializer; import org.apache.commons.lang3.concurrent.ConcurrentUtils; import com.jozufozu.flywheel.api.task.TaskExecutor; +import com.mojang.blaze3d.systems.RenderSystem; public final class FlwTaskExecutor { public static final boolean USE_SERIAL_EXECUTOR = System.getProperty("flw.useSerialExecutor") != null; @@ -28,7 +29,7 @@ public final class FlwTaskExecutor { return SerialTaskExecutor.INSTANCE; } - ParallelTaskExecutor executor = new ParallelTaskExecutor("Flywheel"); + ParallelTaskExecutor executor = new ParallelTaskExecutor("Flywheel", RenderSystem::isOnRenderThread); executor.startWorkers(); return executor; } diff --git a/src/main/java/com/jozufozu/flywheel/impl/task/ParallelTaskExecutor.java b/src/main/java/com/jozufozu/flywheel/impl/task/ParallelTaskExecutor.java index 79122c8d7..b49014680 100644 --- a/src/main/java/com/jozufozu/flywheel/impl/task/ParallelTaskExecutor.java +++ b/src/main/java/com/jozufozu/flywheel/impl/task/ParallelTaskExecutor.java @@ -14,7 +14,6 @@ import org.slf4j.Logger; import com.jozufozu.flywheel.Flywheel; import com.jozufozu.flywheel.api.task.TaskExecutor; -import com.mojang.blaze3d.systems.RenderSystem; import com.mojang.logging.LogUtils; import net.minecraft.util.Mth; @@ -27,6 +26,8 @@ public class ParallelTaskExecutor implements TaskExecutor { private final String name; private final int threadCount; + private final BooleanSupplier mainThreadQuery; + /** * If set to false, the executor will shut down. */ @@ -38,8 +39,9 @@ public class ParallelTaskExecutor implements TaskExecutor { private final ThreadGroupNotifier taskNotifier = new ThreadGroupNotifier(); private final WaitGroup waitGroup = new WaitGroup(); - public ParallelTaskExecutor(String name) { + public ParallelTaskExecutor(String name, BooleanSupplier mainThreadQuery) { this.name = name; + this.mainThreadQuery = mainThreadQuery; threadCount = getOptimalThreadCount(); } @@ -116,16 +118,17 @@ public class ParallelTaskExecutor implements TaskExecutor { } @Override - public void scheduleForSync(Runnable runnable) { + public void scheduleForMainThread(Runnable runnable) { if (!running.get()) { throw new IllegalStateException("Executor is stopped"); } - if (RenderSystem.isOnRenderThread()) { - runnable.run(); - } else { - mainThreadQueue.add(runnable); - } + mainThreadQueue.add(runnable); + } + + @Override + public boolean isMainThread() { + return mainThreadQuery.getAsBoolean(); } /** @@ -133,16 +136,18 @@ public class ParallelTaskExecutor implements TaskExecutor { */ @Override public void syncPoint() { + boolean onMainThread = isMainThread(); while (true) { - if (syncOneTask()) { + if (syncOneTask(onMainThread)) { // Done! Nothing left to do. - break; + return; } } } @Override public boolean syncUntil(BooleanSupplier cond) { + boolean onMainThread = isMainThread(); while (true) { if (cond.getAsBoolean()) { // The condition is already true! @@ -150,7 +155,7 @@ public class ParallelTaskExecutor implements TaskExecutor { return true; } - if (syncOneTask()) { + if (syncOneTask(onMainThread)) { // Out of tasks entirely. // The condition may have flipped though so return its result. return cond.getAsBoolean(); @@ -161,6 +166,7 @@ public class ParallelTaskExecutor implements TaskExecutor { @Override public boolean syncWhile(BooleanSupplier cond) { + boolean onMainThread = isMainThread(); while (true) { if (!cond.getAsBoolean()) { // The condition is already false! @@ -168,7 +174,7 @@ public class ParallelTaskExecutor implements TaskExecutor { return true; } - if (syncOneTask()) { + if (syncOneTask(onMainThread)) { // Out of tasks entirely. // The condition may have flipped though so return its result. return !cond.getAsBoolean(); @@ -179,24 +185,49 @@ public class ParallelTaskExecutor implements TaskExecutor { /** * Attempt to process a single task. * + * @param mainThread Whether this is being called from the main thread or not. * @return {@code true} if the executor has nothing left to do. */ - private boolean syncOneTask() { + private boolean syncOneTask(boolean mainThread) { + return mainThread ? syncOneTaskMainThread() : syncOneTaskOffThread(); + } + + private boolean syncOneTaskMainThread() { Runnable task; if ((task = mainThreadQueue.poll()) != null) { // Prioritize main thread tasks. processMainThreadTask(task); + + // Check again next loop. + return false; } else if ((task = taskQueue.pollLast()) != null) { - // then work on tasks from the queue. + // Nothing in the mainThreadQueue, work on tasks from the normal queue. processTask(task); + + // Check again next loop. + return false; } else { - // then wait for the other threads to finish. + // Nothing right now, wait for the other threads to finish. boolean done = waitGroup.await(10_000); // If we timed-out tasks may have been added to the queue, so check again. // if they didn't, we're done. return done && mainThreadQueue.isEmpty(); } - return false; + } + + private boolean syncOneTaskOffThread() { + Runnable task; + if ((task = taskQueue.pollLast()) != null) { + // then work on tasks from the queue. + processTask(task); + // Check again next loop. + return false; + } else { + // Nothing right now, wait for the other threads to finish. + // If we timed-out tasks may have been added to the queue, so check again. + // if they didn't, we're done. + return waitGroup.await(10_000); + } } private void processTask(Runnable task) { diff --git a/src/main/java/com/jozufozu/flywheel/impl/task/SerialTaskExecutor.java b/src/main/java/com/jozufozu/flywheel/impl/task/SerialTaskExecutor.java index dfac16e21..93ae7af87 100644 --- a/src/main/java/com/jozufozu/flywheel/impl/task/SerialTaskExecutor.java +++ b/src/main/java/com/jozufozu/flywheel/impl/task/SerialTaskExecutor.java @@ -16,7 +16,7 @@ public class SerialTaskExecutor implements TaskExecutor { } @Override - public void scheduleForSync(Runnable runnable) { + public void scheduleForMainThread(Runnable runnable) { runnable.run(); } @@ -38,4 +38,9 @@ public class SerialTaskExecutor implements TaskExecutor { public int getThreadCount() { return 1; } + + @Override + public boolean isMainThread() { + return true; + } } diff --git a/src/main/java/com/jozufozu/flywheel/impl/visualization/storage/Storage.java b/src/main/java/com/jozufozu/flywheel/impl/visualization/storage/Storage.java index 11b5a16dd..b37ada43a 100644 --- a/src/main/java/com/jozufozu/flywheel/impl/visualization/storage/Storage.java +++ b/src/main/java/com/jozufozu/flywheel/impl/visualization/storage/Storage.java @@ -59,7 +59,7 @@ public abstract class Storage { tickableVisuals.remove(visual); dynamicVisuals.remove(visual); if (plannedVisuals.remove(visual)) { - framePlan.clear(); + framePlan.triggerReInitialize(); } visual.delete(); } @@ -103,8 +103,8 @@ public abstract class Storage { tickableVisuals.clear(); dynamicVisuals.clear(); plannedVisuals.clear(); - framePlan.clear(); - tickPlan.clear(); + framePlan.triggerReInitialize(); + tickPlan.triggerReInitialize(); visuals.values() .forEach(Visual::delete); visuals.clear(); diff --git a/src/main/java/com/jozufozu/flywheel/impl/visualization/storage/VisualUpdatePlan.java b/src/main/java/com/jozufozu/flywheel/impl/visualization/storage/VisualUpdatePlan.java index 531c1add0..ff27e0427 100644 --- a/src/main/java/com/jozufozu/flywheel/impl/visualization/storage/VisualUpdatePlan.java +++ b/src/main/java/com/jozufozu/flywheel/impl/visualization/storage/VisualUpdatePlan.java @@ -47,7 +47,7 @@ public class VisualUpdatePlan implements SimplyComposedPlan { return plan; } - public void clear() { + public void triggerReInitialize() { plan = UnitPlan.of(); initialized = false; } diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/ContextConsumer.java b/src/main/java/com/jozufozu/flywheel/lib/task/ContextConsumer.java deleted file mode 100644 index 598cb4ef1..000000000 --- a/src/main/java/com/jozufozu/flywheel/lib/task/ContextConsumer.java +++ /dev/null @@ -1,13 +0,0 @@ -package com.jozufozu.flywheel.lib.task; - -import com.jozufozu.flywheel.api.task.Plan; - -/** - * A consumer like interface for use with {@link Plan}s. - * - * @param The context type. - */ -@FunctionalInterface -public interface ContextConsumer { - void accept(C context); -} diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/ContextFunction.java b/src/main/java/com/jozufozu/flywheel/lib/task/ContextFunction.java deleted file mode 100644 index 98ce006a8..000000000 --- a/src/main/java/com/jozufozu/flywheel/lib/task/ContextFunction.java +++ /dev/null @@ -1,13 +0,0 @@ -package com.jozufozu.flywheel.lib.task; - -import com.jozufozu.flywheel.api.task.Plan; - -/** - * A function like interface for use with {@link Plan}s. - * @param The context type. - * @param The return type. - */ -@FunctionalInterface -public interface ContextFunction { - R apply(C context); -} diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/ContextRunnable.java b/src/main/java/com/jozufozu/flywheel/lib/task/ContextRunnable.java deleted file mode 100644 index 4ab130ba3..000000000 --- a/src/main/java/com/jozufozu/flywheel/lib/task/ContextRunnable.java +++ /dev/null @@ -1,16 +0,0 @@ -package com.jozufozu.flywheel.lib.task; - -/** - * A {@link ContextConsumer} that ignores the context object. - * - * @param The context type. - */ -@FunctionalInterface -public interface ContextRunnable extends ContextConsumer { - void run(); - - @Override - default void accept(C ignored) { - run(); - } -} diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/ContextSupplier.java b/src/main/java/com/jozufozu/flywheel/lib/task/ContextSupplier.java deleted file mode 100644 index 6ee3fbded..000000000 --- a/src/main/java/com/jozufozu/flywheel/lib/task/ContextSupplier.java +++ /dev/null @@ -1,17 +0,0 @@ -package com.jozufozu.flywheel.lib.task; - -/** - * A {@link ContextFunction} that ignores the context object. - * - * @param The context type. - * @param The return type. - */ -@FunctionalInterface -public interface ContextSupplier extends ContextFunction { - R get(); - - @Override - default R apply(C ignored) { - return get(); - } -} diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/DynamicNestedPlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/DynamicNestedPlan.java index 440276220..29dae88b3 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/DynamicNestedPlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/DynamicNestedPlan.java @@ -4,6 +4,7 @@ import java.util.Collection; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; +import com.jozufozu.flywheel.lib.task.functional.SupplierWithContext; /** * A plan that executes many other plans provided dynamically. @@ -11,18 +12,19 @@ import com.jozufozu.flywheel.api.task.TaskExecutor; * @param plans A function to get a collection of plans based on the context. * @param The type of the context object. */ -public record DynamicNestedPlan(ContextFunction>> plans) implements SimplyComposedPlan { - public static Plan of(ContextSupplier>> supplier) { +public record DynamicNestedPlan( + SupplierWithContext>> plans) implements SimplyComposedPlan { + public static Plan of(SupplierWithContext.Ignored>> supplier) { return new DynamicNestedPlan<>(supplier); } - public static Plan of(ContextFunction>> function) { - return new DynamicNestedPlan<>(function); + public static Plan of(SupplierWithContext>> supplier) { + return new DynamicNestedPlan<>(supplier); } @Override public void execute(TaskExecutor taskExecutor, C context, Runnable onCompletion) { - var plans = this.plans.apply(context); + var plans = this.plans.get(context); if (plans.isEmpty()) { onCompletion.run(); diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/ForEachPlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/ForEachPlan.java index 221322ad6..55b6bef46 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/ForEachPlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/ForEachPlan.java @@ -1,12 +1,11 @@ package com.jozufozu.flywheel.lib.task; import java.util.List; -import java.util.function.BiConsumer; -import java.util.function.Consumer; -import java.util.function.Supplier; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; +import com.jozufozu.flywheel.lib.task.functional.ConsumerWithContext; +import com.jozufozu.flywheel.lib.task.functional.SupplierWithContext; /** * A plan that executes code on each element of a provided list. @@ -18,17 +17,26 @@ import com.jozufozu.flywheel.api.task.TaskExecutor; * @param The type of the list elements. * @param The type of the context object. */ -public record ForEachPlan(Supplier> listSupplier, BiConsumer action) implements SimplyComposedPlan { - public static Plan of(Supplier> iterable, BiConsumer forEach) { +public record ForEachPlan(SupplierWithContext> listSupplier, + ConsumerWithContext action) implements SimplyComposedPlan { + public static Plan of(SupplierWithContext> iterable, ConsumerWithContext forEach) { return new ForEachPlan<>(iterable, forEach); } - public static Plan of(Supplier> iterable, Consumer forEach) { - return of(iterable, (t, c) -> forEach.accept(t)); + public static Plan of(SupplierWithContext> iterable, ConsumerWithContext.Ignored forEach) { + return new ForEachPlan<>(iterable, forEach); + } + + public static Plan of(SupplierWithContext.Ignored> iterable, ConsumerWithContext forEach) { + return new ForEachPlan<>(iterable, forEach); + } + + public static Plan of(SupplierWithContext.Ignored> iterable, ConsumerWithContext.Ignored forEach) { + return new ForEachPlan<>(iterable, forEach); } @Override public void execute(TaskExecutor taskExecutor, C context, Runnable onCompletion) { - taskExecutor.execute(() -> PlanUtil.distribute(taskExecutor, context, onCompletion, listSupplier.get(), action)); + taskExecutor.execute(() -> PlanUtil.distribute(taskExecutor, context, onCompletion, listSupplier.get(context), action)); } } diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/ForEachSlicePlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/ForEachSlicePlan.java index d090834e0..691aa080c 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/ForEachSlicePlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/ForEachSlicePlan.java @@ -1,11 +1,11 @@ package com.jozufozu.flywheel.lib.task; import java.util.List; -import java.util.function.BiConsumer; -import java.util.function.Supplier; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; +import com.jozufozu.flywheel.lib.task.functional.ConsumerWithContext; +import com.jozufozu.flywheel.lib.task.functional.SupplierWithContext; /** * A plan that executes code over many slices of a provided list. @@ -17,14 +17,26 @@ import com.jozufozu.flywheel.api.task.TaskExecutor; * @param The type of the list elements. * @param The type of the context object. */ -public record ForEachSlicePlan(Supplier> listSupplier, - BiConsumer, C> action) implements SimplyComposedPlan { - public static Plan of(Supplier> iterable, BiConsumer, C> forEach) { +public record ForEachSlicePlan(SupplierWithContext> listSupplier, + ConsumerWithContext, C> action) implements SimplyComposedPlan { + public static Plan of(SupplierWithContext> iterable, ConsumerWithContext, C> forEach) { + return new ForEachSlicePlan<>(iterable, forEach); + } + + public static Plan of(SupplierWithContext> iterable, ConsumerWithContext.Ignored, C> forEach) { + return new ForEachSlicePlan<>(iterable, forEach); + } + + public static Plan of(SupplierWithContext.Ignored> iterable, ConsumerWithContext, C> forEach) { + return new ForEachSlicePlan<>(iterable, forEach); + } + + public static Plan of(SupplierWithContext.Ignored> iterable, ConsumerWithContext.Ignored, C> forEach) { return new ForEachSlicePlan<>(iterable, forEach); } @Override public void execute(TaskExecutor taskExecutor, C context, Runnable onCompletion) { - taskExecutor.execute(() -> PlanUtil.distributeSlices(taskExecutor, context, onCompletion, listSupplier.get(), action)); + taskExecutor.execute(() -> PlanUtil.distributeSlices(taskExecutor, context, onCompletion, listSupplier.get(context), action)); } } diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/IfElsePlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/IfElsePlan.java index e801e32d0..0578525ab 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/IfElsePlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/IfElsePlan.java @@ -1,9 +1,8 @@ package com.jozufozu.flywheel.lib.task; -import java.util.function.Predicate; - import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; +import com.jozufozu.flywheel.lib.task.functional.BooleanSupplierWithContext; /** * Executes one plan or another, depending on a dynamically evaluated condition. @@ -12,14 +11,19 @@ import com.jozufozu.flywheel.api.task.TaskExecutor; * @param onFalse The plan to execute if the condition is false. * @param The type of the context object. */ -public record IfElsePlan(Predicate condition, Plan onTrue, Plan onFalse) implements SimplyComposedPlan { - public static Builder on(Predicate condition) { +public record IfElsePlan(BooleanSupplierWithContext condition, Plan onTrue, + Plan onFalse) implements SimplyComposedPlan { + public static Builder on(BooleanSupplierWithContext condition) { + return new Builder<>(condition); + } + + public static Builder on(BooleanSupplierWithContext.Ignored condition) { return new Builder<>(condition); } @Override public void execute(TaskExecutor taskExecutor, C context, Runnable onCompletion) { - if (condition.test(context)) { + if (condition.getAsBoolean(context)) { onTrue.execute(taskExecutor, context, onCompletion); } else { onFalse.execute(taskExecutor, context, onCompletion); @@ -40,11 +44,11 @@ public record IfElsePlan(Predicate condition, Plan onTrue, Plan onFa } public static class Builder { - private final Predicate condition; + private final BooleanSupplierWithContext condition; private Plan onTrue = UnitPlan.of(); private Plan onFalse = UnitPlan.of(); - public Builder(Predicate condition) { + public Builder(BooleanSupplierWithContext condition) { this.condition = condition; } diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/MapContextPlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/MapContextPlan.java index 442f1fd10..dfb7a1da7 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/MapContextPlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/MapContextPlan.java @@ -1,12 +1,15 @@ package com.jozufozu.flywheel.lib.task; -import java.util.function.Function; - import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; +import com.jozufozu.flywheel.lib.task.functional.SupplierWithContext; -public record MapContextPlan(Function map, Plan plan) implements SimplyComposedPlan { - public static Builder map(Function map) { +public record MapContextPlan(SupplierWithContext map, Plan plan) implements SimplyComposedPlan { + public static Builder map(SupplierWithContext map) { + return new Builder<>(map); + } + + public static Builder get(SupplierWithContext.Ignored map) { return new Builder<>(map); } @@ -28,9 +31,9 @@ public record MapContextPlan(Function map, Plan plan) implements } public static class Builder { - private final Function map; + private final SupplierWithContext map; - public Builder(Function map) { + public Builder(SupplierWithContext map) { this.map = map; } diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/NestedPlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/NestedPlan.java index a9a1111c3..845dccb6b 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/NestedPlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/NestedPlan.java @@ -7,6 +7,7 @@ import java.util.List; import com.google.common.collect.ImmutableList; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; +import com.jozufozu.flywheel.lib.task.functional.RunnableWithContext; public record NestedPlan(List> parallelPlans) implements SimplyComposedPlan { @SafeVarargs @@ -54,7 +55,7 @@ public record NestedPlan(List> parallelPlans) implements SimplyCompos .simplify(); } - var simplifiedTasks = new ArrayList>(); + var simplifiedTasks = new ArrayList>(); var simplifiedPlans = new ArrayList>(); var toVisit = new ArrayDeque<>(parallelPlans); while (!toVisit.isEmpty()) { diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/SimplePlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/SimplePlan.java index 21cff6b8c..eff105ec6 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/SimplePlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/SimplePlan.java @@ -5,19 +5,20 @@ import java.util.List; import com.google.common.collect.ImmutableList; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; +import com.jozufozu.flywheel.lib.task.functional.RunnableWithContext; -public record SimplePlan(List> parallelTasks) implements SimplyComposedPlan { +public record SimplePlan(List> parallelTasks) implements SimplyComposedPlan { @SafeVarargs - public static SimplePlan of(ContextRunnable... tasks) { + public static SimplePlan of(RunnableWithContext.Ignored... tasks) { return new SimplePlan<>(List.of(tasks)); } @SafeVarargs - public static SimplePlan of(ContextConsumer... tasks) { + public static SimplePlan of(RunnableWithContext... tasks) { return new SimplePlan<>(List.of(tasks)); } - public static SimplePlan of(List> tasks) { + public static SimplePlan of(List> tasks) { return new SimplePlan<>(tasks); } @@ -28,15 +29,13 @@ public record SimplePlan(List> parallelTasks) implements S return; } - taskExecutor.execute(() -> { - PlanUtil.distribute(taskExecutor, context, onCompletion, parallelTasks, ContextConsumer::accept); - }); + taskExecutor.execute(() -> PlanUtil.distribute(taskExecutor, context, onCompletion, parallelTasks, RunnableWithContext::run)); } @Override public Plan and(Plan plan) { if (plan instanceof SimplePlan simple) { - return of(ImmutableList.>builder() + return of(ImmutableList.>builder() .addAll(parallelTasks) .addAll(simple.parallelTasks) .build()); diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/SyncedPlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/SyncedPlan.java index d44d80a05..cb0867cb7 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/SyncedPlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/SyncedPlan.java @@ -2,20 +2,26 @@ package com.jozufozu.flywheel.lib.task; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.api.task.TaskExecutor; +import com.jozufozu.flywheel.lib.task.functional.RunnableWithContext; -public record SyncedPlan(ContextConsumer task) implements SimplyComposedPlan { - public static Plan of(ContextConsumer task) { +public record SyncedPlan(RunnableWithContext task) implements SimplyComposedPlan { + public static Plan of(RunnableWithContext task) { return new SyncedPlan<>(task); } - public static Plan of(ContextRunnable task) { + public static Plan of(RunnableWithContext.Ignored task) { return new SyncedPlan<>(task); } @Override public void execute(TaskExecutor taskExecutor, C context, Runnable onCompletion) { - taskExecutor.scheduleForSync(() -> { - task.accept(context); + if (taskExecutor.isMainThread()) { + task.run(context); + onCompletion.run(); + return; + } + taskExecutor.scheduleForMainThread(() -> { + task.run(context); onCompletion.run(); }); } diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/functional/BooleanSupplierWithContext.java b/src/main/java/com/jozufozu/flywheel/lib/task/functional/BooleanSupplierWithContext.java new file mode 100644 index 000000000..b933708d3 --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/lib/task/functional/BooleanSupplierWithContext.java @@ -0,0 +1,35 @@ +package com.jozufozu.flywheel.lib.task.functional; + +import java.util.function.BooleanSupplier; +import java.util.function.Predicate; + +/** + * A boolean supplier like interface for use with {@link com.jozufozu.flywheel.api.task.Plan Plans} and their contexts. + * + * @param The context type. + */ +@FunctionalInterface +public interface BooleanSupplierWithContext extends Predicate { + boolean getAsBoolean(C context); + + @Override + default boolean test(C c) { + return getAsBoolean(c); + } + + /** + * A {@link BooleanSupplierWithContext} that ignores the context object. + * + * @param The (ignored) context type. + */ + @FunctionalInterface + interface Ignored extends BooleanSupplierWithContext, BooleanSupplier { + @Override + boolean getAsBoolean(); + + @Override + default boolean getAsBoolean(C ignored) { + return getAsBoolean(); + } + } +} diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/functional/ConsumerWithContext.java b/src/main/java/com/jozufozu/flywheel/lib/task/functional/ConsumerWithContext.java new file mode 100644 index 000000000..ce4495262 --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/lib/task/functional/ConsumerWithContext.java @@ -0,0 +1,34 @@ +package com.jozufozu.flywheel.lib.task.functional; + +import java.util.function.BiConsumer; +import java.util.function.Consumer; + +/** + * A consumer like interface for use with {@link com.jozufozu.flywheel.api.task.Plan Plans} and their contexts. + *
+ * The subinterface {@link Ignored} is provided for consumers that do not need the context object. + * + * @param The type to actually consume. + * @param The context type. + */ +@FunctionalInterface +public interface ConsumerWithContext extends BiConsumer { + void accept(T t, C context); + + /** + * A {@link ConsumerWithContext} that ignores the context object. + * + * @param The type to actually consume. + * @param The (ignored) context type. + */ + @FunctionalInterface + interface Ignored extends ConsumerWithContext, Consumer { + @Override + void accept(T t); + + @Override + default void accept(T t, C ignored) { + accept(t); + } + } +} diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/functional/RunnableWithContext.java b/src/main/java/com/jozufozu/flywheel/lib/task/functional/RunnableWithContext.java new file mode 100644 index 000000000..7e4e2577b --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/lib/task/functional/RunnableWithContext.java @@ -0,0 +1,35 @@ +package com.jozufozu.flywheel.lib.task.functional; + +import java.util.function.Consumer; + +/** + * A runnable like interface for use with {@link com.jozufozu.flywheel.api.task.Plan Plans} and their contexts. + *
+ * The subinterface {@link Ignored} is provided for runnables that do not need the context object. + * @param The context type. + */ +@FunctionalInterface +public interface RunnableWithContext extends Consumer { + void run(C context); + + @Override + default void accept(C c) { + run(c); + } + + /** + * A {@link RunnableWithContext} that ignores the context object. + * + * @param The (ignored) context type. + */ + @FunctionalInterface + interface Ignored extends RunnableWithContext, Runnable { + @Override + void run(); + + @Override + default void run(C ignored) { + run(); + } + } +} diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/functional/SupplierWithContext.java b/src/main/java/com/jozufozu/flywheel/lib/task/functional/SupplierWithContext.java new file mode 100644 index 000000000..4178ccdb5 --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/lib/task/functional/SupplierWithContext.java @@ -0,0 +1,38 @@ +package com.jozufozu.flywheel.lib.task.functional; + +import java.util.function.Function; +import java.util.function.Supplier; + +/** + * A supplier like interface for use with {@link com.jozufozu.flywheel.api.task.Plan Plans} and their contexts. + *
+ * The subinterface {@link Ignored} is provided for suppliers that do not need the context object. + * @param The context type. + * @param The return type. + */ +@FunctionalInterface +public interface SupplierWithContext extends Function { + R get(C context); + + @Override + default R apply(C c) { + return get(c); + } + + /** + * A {@link SupplierWithContext} that ignores the context object. + * + * @param The (ignored) context type. + * @param The return type. + */ + @FunctionalInterface + interface Ignored extends SupplierWithContext, Supplier { + @Override + R get(); + + @Override + default R get(C ignored) { + return get(); + } + } +} diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/functional/package-info.java b/src/main/java/com/jozufozu/flywheel/lib/task/functional/package-info.java new file mode 100644 index 000000000..6551c466a --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/lib/task/functional/package-info.java @@ -0,0 +1,8 @@ +/** + * Functional interfaces accepting a context object for use with {@link com.jozufozu.flywheel.api.task.Plan Plans}. + *
+ * Each interface in this package has a subinterface that ignores the context object. Plans then call the parent + * interface, but do not need to create additional closure objects to translate when the consumer wishes to ignore + * the context object. + */ +package com.jozufozu.flywheel.lib.task.functional; diff --git a/src/test/java/com/jozufozu/flywheel/lib/task/PlanExecutionTest.java b/src/test/java/com/jozufozu/flywheel/lib/task/PlanExecutionTest.java index 2e1c764b8..884bbe974 100644 --- a/src/test/java/com/jozufozu/flywheel/lib/task/PlanExecutionTest.java +++ b/src/test/java/com/jozufozu/flywheel/lib/task/PlanExecutionTest.java @@ -5,9 +5,9 @@ import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -15,22 +15,26 @@ import org.junit.jupiter.params.provider.ValueSource; import com.jozufozu.flywheel.api.task.Plan; import com.jozufozu.flywheel.impl.task.ParallelTaskExecutor; +import com.jozufozu.flywheel.lib.task.functional.RunnableWithContext; import com.jozufozu.flywheel.lib.util.Unit; import it.unimi.dsi.fastutil.ints.IntArrayList; class PlanExecutionTest { - protected static final ParallelTaskExecutor EXECUTOR = new ParallelTaskExecutor("PlanTest"); + protected static ParallelTaskExecutor EXECUTOR; - @BeforeAll - public static void setUp() { + @BeforeEach + public void setUp() { + var currentThread = Thread.currentThread(); + EXECUTOR = new ParallelTaskExecutor("PlanTest", () -> currentThread == Thread.currentThread()); EXECUTOR.startWorkers(); } - @AfterAll - public static void tearDown() { + @AfterEach + public void tearDown() { EXECUTOR.stopWorkers(); + EXECUTOR = null; } @ParameterizedTest @@ -73,12 +77,12 @@ class PlanExecutionTest { var lock = new Object(); var sequence = new IntArrayList(8); - ContextRunnable addOne = () -> { + RunnableWithContext.Ignored addOne = () -> { synchronized (lock) { sequence.add(1); } }; - ContextRunnable addTwo = () -> { + RunnableWithContext.Ignored addTwo = () -> { synchronized (lock) { sequence.add(2); } @@ -185,12 +189,26 @@ class PlanExecutionTest { } @Test - void mainThreadPlan() { + void mainThreadPlanRunsImmediately() { var done = new AtomicBoolean(false); var plan = SyncedPlan.of(() -> done.set(true)); plan.execute(EXECUTOR, Unit.INSTANCE); + Assertions.assertTrue(done.get()); + } + + @Test + void mainThreadPlanIsNotCalledOffThread() { + var done = new AtomicBoolean(false); + + var plan = SyncedPlan.of(() -> { + done.set(true); + }); + + // call execute from within a worker thread + EXECUTOR.execute(() -> plan.execute(EXECUTOR, Unit.INSTANCE)); + Assertions.assertFalse(done.get()); EXECUTOR.syncPoint(); @@ -222,16 +240,18 @@ class PlanExecutionTest { var first = new NamedFlag("ready right away"); var second = new NamedFlag("ready after 2s"); - RaisePlan.raise(first) + var plan = RaisePlan.raise(first) .then(SimplePlan.of(() -> { + // sleep to add delay between raising the first flag and raising the second flag try { Thread.sleep(2000); } catch (InterruptedException e) { throw new RuntimeException(e); } })) - .then(RaisePlan.raise(second)) - .execute(EXECUTOR, Unit.INSTANCE); + .then(RaisePlan.raise(second)); + + EXECUTOR.execute(() -> plan.execute(EXECUTOR, Unit.INSTANCE)); Assertions.assertTrue(EXECUTOR.syncUntil(first::isRaised), "First flag should be raised since we submitted a plan that raises it."); @@ -260,7 +280,7 @@ class PlanExecutionTest { } public static void runAndWait(Plan plan) { - new TestBarrier(plan, Unit.INSTANCE).runAndWait(); + new TestBarrier<>(plan, Unit.INSTANCE).runAndWait(); } public static void runAndWait(Plan plan, C ctx) { diff --git a/src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java b/src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java index ea5501b14..44a54f4ce 100644 --- a/src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java +++ b/src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java @@ -4,11 +4,12 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import com.jozufozu.flywheel.api.task.Plan; +import com.jozufozu.flywheel.lib.task.functional.RunnableWithContext; import com.jozufozu.flywheel.lib.util.Unit; public class PlanSimplificationTest { - public static final ContextRunnable NOOP = () -> { + public static final RunnableWithContext.Ignored NOOP = () -> { }; public static final Plan SIMPLE = SimplePlan.of(NOOP);