From c931e84b65b59e6f88a0e899ecea4ec2591657ac Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Wed, 28 Feb 2024 21:23:47 -0800 Subject: [PATCH] More simpler - Remove Plan#simplify and associated tests - Neat idea but ineffective in practice - Should reduce the user burden of implementing plans --- .../com/jozufozu/flywheel/api/task/Plan.java | 8 -- .../VisualizationManagerImpl.java | 6 +- .../flywheel/lib/task/BarrierPlan.java | 14 --- .../flywheel/lib/task/IfElsePlan.java | 13 -- .../flywheel/lib/task/MapContextPlan.java | 11 -- .../flywheel/lib/task/NestedPlan.java | 60 ---------- .../flywheel/lib/task/SimplePlan.java | 8 -- .../flywheel/lib/task/SimplyComposedPlan.java | 4 - .../jozufozu/flywheel/lib/task/UnitPlan.java | 4 - .../lib/task/PlanSimplificationTest.java | 113 ------------------ 10 files changed, 2 insertions(+), 239 deletions(-) delete mode 100644 src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java diff --git a/src/main/java/com/jozufozu/flywheel/api/task/Plan.java b/src/main/java/com/jozufozu/flywheel/api/task/Plan.java index b9f89d4f4..703b1a59f 100644 --- a/src/main/java/com/jozufozu/flywheel/api/task/Plan.java +++ b/src/main/java/com/jozufozu/flywheel/api/task/Plan.java @@ -38,12 +38,4 @@ public interface Plan { * @return The composed plan. */ Plan and(Plan plan); - - /** - * If possible, create a new plan that accomplishes everything - * this plan does but with a simpler execution schedule. - * - * @return A simplified plan, or this. - */ - Plan simplify(); } diff --git a/src/main/java/com/jozufozu/flywheel/impl/visualization/VisualizationManagerImpl.java b/src/main/java/com/jozufozu/flywheel/impl/visualization/VisualizationManagerImpl.java index c90389c6d..80bfa9851 100644 --- a/src/main/java/com/jozufozu/flywheel/impl/visualization/VisualizationManagerImpl.java +++ b/src/main/java/com/jozufozu/flywheel/impl/visualization/VisualizationManagerImpl.java @@ -95,8 +95,7 @@ public class VisualizationManagerImpl implements VisualizationManager { effects = new VisualManagerImpl<>(effectsStorage); tickPlan = NestedPlan.of(blockEntities.tickPlan(), entities.tickPlan(), effects.tickPlan()) - .then(RaisePlan.raise(tickFlag)) - .simplify(); + .then(RaisePlan.raise(tickFlag)); var recreate = SimplePlan.of(context -> blockEntitiesStorage.recreateAll(context.partialTick()), context -> entitiesStorage.recreateAll(context.partialTick()), @@ -111,8 +110,7 @@ public class VisualizationManagerImpl implements VisualizationManager { .plan() .then(RaisePlan.raise(frameVisualsFlag)) .then(engine.createFramePlan()) - .then(RaisePlan.raise(frameFlag)) - .simplify(); + .then(RaisePlan.raise(frameFlag)); if (level instanceof Level l) { LevelExtension.getAllLoadedEntities(l) diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/BarrierPlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/BarrierPlan.java index eb94f7f03..fde6956e8 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/BarrierPlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/BarrierPlan.java @@ -13,18 +13,4 @@ public record BarrierPlan(Plan first, Plan second) implements SimplyCom first.execute(taskExecutor, context, () -> second.execute(taskExecutor, context, onCompletion)); } - @Override - public Plan simplify() { - var first = this.first.simplify(); - var second = this.second.simplify(); - - if (first == UnitPlan.of()) { - return second; - } - if (second == UnitPlan.of()) { - return first; - } - - return new BarrierPlan<>(first, second); - } } 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 0578525ab..178353d42 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/IfElsePlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/IfElsePlan.java @@ -30,19 +30,6 @@ public record IfElsePlan(BooleanSupplierWithContext condition, Plan onT } } - @Override - public Plan simplify() { - var maybeSimplifiedTrue = onTrue.simplify(); - var maybeSimplifiedFalse = onFalse.simplify(); - - if (maybeSimplifiedTrue instanceof UnitPlan && maybeSimplifiedFalse instanceof UnitPlan) { - // The condition may have side effects that still need to be evaluated. - return SimplePlan.of(condition::test); - } - - return new IfElsePlan<>(condition, maybeSimplifiedTrue, maybeSimplifiedFalse); - } - public static class Builder { private final BooleanSupplierWithContext condition; private Plan onTrue = UnitPlan.of(); 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 dfb7a1da7..516f5bd53 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/MapContextPlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/MapContextPlan.java @@ -19,17 +19,6 @@ public record MapContextPlan(SupplierWithContext map, Plan plan) plan.execute(taskExecutor, newContext, onCompletion); } - @Override - public Plan simplify() { - var maybeSimplified = plan.simplify(); - - if (maybeSimplified instanceof UnitPlan) { - return UnitPlan.of(); - } - - return new MapContextPlan<>(map, maybeSimplified); - } - public static class Builder { private final SupplierWithContext 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 845dccb6b..a182f1591 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/NestedPlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/NestedPlan.java @@ -1,13 +1,10 @@ package com.jozufozu.flywheel.lib.task; -import java.util.ArrayDeque; -import java.util.ArrayList; 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 @@ -44,61 +41,4 @@ public record NestedPlan(List> parallelPlans) implements SimplyCompos .build()); } - @Override - public Plan simplify() { - if (parallelPlans.isEmpty()) { - return UnitPlan.of(); - } - - if (parallelPlans.size() == 1) { - return parallelPlans.get(0) - .simplify(); - } - - var simplifiedTasks = new ArrayList>(); - var simplifiedPlans = new ArrayList>(); - var toVisit = new ArrayDeque<>(parallelPlans); - while (!toVisit.isEmpty()) { - var plan = toVisit.pop() - .simplify(); - - if (plan == UnitPlan.of()) { - continue; - } - - if (plan instanceof SimplePlan simplePlan) { - // merge all simple plans into one - simplifiedTasks.addAll(simplePlan.parallelTasks()); - } else if (plan instanceof NestedPlan nestedPlan) { - // inline and re-visit nested plans - toVisit.addAll(nestedPlan.parallelPlans()); - } else { - // /shrug - simplifiedPlans.add(plan); - } - } - - if (simplifiedTasks.isEmpty() && simplifiedPlans.isEmpty()) { - // everything got simplified away - return UnitPlan.of(); - } - - if (simplifiedTasks.isEmpty()) { - // no simple plan to create - if (simplifiedPlans.size() == 1) { - // we only contained one complex plan, so we can just return that - return simplifiedPlans.get(0); - } - return new NestedPlan<>(simplifiedPlans); - } - - if (simplifiedPlans.isEmpty()) { - // we only contained simple plans, so we can just return one - return SimplePlan.of(simplifiedTasks); - } - - // we have both simple and complex plans, so we need to create a nested plan - simplifiedPlans.add(SimplePlan.of(simplifiedTasks)); - return new NestedPlan<>(simplifiedPlans); - } } 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 26abad7b4..5b0fbeead 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/SimplePlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/SimplePlan.java @@ -43,12 +43,4 @@ public record SimplePlan(List> parallelTasks) implemen return SimplyComposedPlan.super.and(plan); } - @Override - public Plan simplify() { - if (parallelTasks.isEmpty()) { - return UnitPlan.of(); - } - - return this; - } } diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/SimplyComposedPlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/SimplyComposedPlan.java index 3e5b2c47b..d39ac0277 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/SimplyComposedPlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/SimplyComposedPlan.java @@ -13,8 +13,4 @@ public interface SimplyComposedPlan extends Plan { return NestedPlan.of(this, plan); } - @Override - default Plan simplify() { - return this; - } } diff --git a/src/main/java/com/jozufozu/flywheel/lib/task/UnitPlan.java b/src/main/java/com/jozufozu/flywheel/lib/task/UnitPlan.java index 9b1215cff..e12402bc2 100644 --- a/src/main/java/com/jozufozu/flywheel/lib/task/UnitPlan.java +++ b/src/main/java/com/jozufozu/flywheel/lib/task/UnitPlan.java @@ -29,8 +29,4 @@ public class UnitPlan implements Plan { return plan; } - @Override - public Plan simplify() { - return this; - } } diff --git a/src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java b/src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java deleted file mode 100644 index 44a54f4ce..000000000 --- a/src/test/java/com/jozufozu/flywheel/lib/task/PlanSimplificationTest.java +++ /dev/null @@ -1,113 +0,0 @@ -package com.jozufozu.flywheel.lib.task; - -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 RunnableWithContext.Ignored NOOP = () -> { - }; - public static final Plan SIMPLE = SimplePlan.of(NOOP); - - @Test - void emptyPlans() { - var empty = NestedPlan.of(); - Assertions.assertEquals(empty.simplify(), UnitPlan.of()); - - var simpleEmpty = SimplePlan.of(); - Assertions.assertEquals(simpleEmpty.simplify(), UnitPlan.of()); - } - - @Test - void nestedSimplePlans() { - var twoSimple = NestedPlan.of(SimplePlan.of(NOOP, NOOP, NOOP), SIMPLE); - Assertions.assertEquals(twoSimple.simplify(), SimplePlan.of(NOOP, NOOP, NOOP, NOOP)); - - var threeSimple = NestedPlan.of(SIMPLE, SIMPLE, SIMPLE); - Assertions.assertEquals(threeSimple.simplify(), SimplePlan.of(NOOP, NOOP, NOOP)); - } - - @Test - void oneNestedPlan() { - var oneSimple = NestedPlan.of(SIMPLE); - - Assertions.assertEquals(oneSimple.simplify(), SIMPLE); - - var mainThreadNoop = new SyncedPlan<>(NOOP); - var oneMainThread = NestedPlan.of(mainThreadNoop); - - Assertions.assertEquals(oneMainThread.simplify(), mainThreadNoop); - - var barrier = new BarrierPlan<>(SIMPLE, SIMPLE); - var oneBarrier = NestedPlan.of(barrier); - - Assertions.assertEquals(oneBarrier.simplify(), barrier); - } - - @Test - void nestedNestedPlan() { - var outer = NestedPlan.of(SIMPLE); - var outermost = NestedPlan.of(outer); - - Assertions.assertEquals(outermost.simplify(), SIMPLE); - } - - @Test - void nestedUnitPlan() { - var onlyUnit = NestedPlan.of(UnitPlan.of(), UnitPlan.of(), UnitPlan.of()); - Assertions.assertEquals(onlyUnit.simplify(), UnitPlan.of()); - - var unitAndSimple = NestedPlan.of(UnitPlan.of(), UnitPlan.of(), SIMPLE); - Assertions.assertEquals(unitAndSimple.simplify(), SIMPLE); - } - - @Test - void complexNesting() { - var mainThreadNoop = SyncedPlan.of(() -> { - }); - - var nested = NestedPlan.of(mainThreadNoop, SIMPLE); - Assertions.assertEquals(nested.simplify(), nested); // cannot simplify - - var barrier = new BarrierPlan<>(SIMPLE, SIMPLE); - var complex = NestedPlan.of(barrier, nested); - Assertions.assertEquals(complex.simplify(), NestedPlan.of(barrier, mainThreadNoop, SIMPLE)); - } - - @Test - void nestedNoSimple() { - var mainThreadNoop = SyncedPlan.of(() -> { - }); - var barrier = new BarrierPlan<>(SIMPLE, SIMPLE); - var oneMainThread = NestedPlan.of(mainThreadNoop, NestedPlan.of(mainThreadNoop, barrier, barrier)); - - Assertions.assertEquals(oneMainThread.simplify(), NestedPlan.of(mainThreadNoop, mainThreadNoop, barrier, barrier)); - } - - @Test - void manyNestedButJustOneAfterSimplification() { - var barrier = new BarrierPlan<>(SIMPLE, SIMPLE); - var oneMainThread = NestedPlan.of(barrier, NestedPlan.of(UnitPlan.of(), UnitPlan.of())); - - Assertions.assertEquals(oneMainThread.simplify(), barrier); - } - - @Test - void barrierPlan() { - var doubleUnit = new BarrierPlan<>(UnitPlan.of(), UnitPlan.of()); - Assertions.assertEquals(doubleUnit.simplify(), UnitPlan.of()); - - var simpleThenUnit = new BarrierPlan<>(SIMPLE, UnitPlan.of()); - Assertions.assertEquals(simpleThenUnit.simplify(), SIMPLE); - - var unitThenSimple = new BarrierPlan<>(UnitPlan.of(), SIMPLE); - Assertions.assertEquals(unitThenSimple.simplify(), SIMPLE); - - var simpleThenSimple = new BarrierPlan<>(SIMPLE, SIMPLE); - Assertions.assertEquals(simpleThenSimple.simplify(), new BarrierPlan<>(SIMPLE, SIMPLE)); - } -}