Improved vexillology

- Separate concept of Flags from TaskExecutor.
- Instead, allow TaskExecutor to sync until, or while a given condition
  is met.
- Flags directly store their state as an AtomicBoolean.
- Switch `executor.syncOn(flag)` to `executor.syncUntil(flag::isRaised)`
- Remove tests made redundant by improved interface.
This commit is contained in:
Jozufozu 2023-11-20 11:16:32 -08:00
parent 78e959d1a9
commit 78975e8cad
14 changed files with 163 additions and 165 deletions

View file

@ -1,12 +0,0 @@
package com.jozufozu.flywheel.api.task;
/**
* Marker interface for flags that can be raised and lowered in a {@link TaskExecutor}.
* <br>
* <strong>Warning:</strong> flags will only be considered equal by reference.
* This is to allow multiple instances of the same high level structures to exist at once.
* <br>
* Keep this in mind when using records as flags.
*/
public interface Flag {
}

View file

@ -1,52 +1,40 @@
package com.jozufozu.flywheel.api.task;
import java.util.concurrent.Executor;
import java.util.function.BooleanSupplier;
public interface TaskExecutor extends Executor {
/**
* Wait for <em>all</em> running tasks to finish.
* <br>
* This is useful as a nuclear option, but most of the time you should
* try to use {@link Flag flags} and {@link #syncTo(Flag) syncTo}.
* try to use {@link #syncUntil(BooleanSupplier) syncUntil}.
*/
void syncPoint();
/**
* Wait for running tasks, until the given Flag is {@link #raise raised}.
* Wait for running tasks, until the given condition is met
* ({@link BooleanSupplier#getAsBoolean()} returns {@code true}).
* <br>
* The flag will remain raised until {@link #lower lowered} manually.
* This method is equivalent to {@code syncWhile(() -> !cond.getAsBoolean())}.
*
* @param flag The flag to wait for.
* @return {@code true} if the flag was encountered. May return false if
* this executor runs out of tasks before the flag was raised.
* @param cond The condition to wait for.
* @return {@code true} if the condition is met. {@code false} if
* this executor runs out of tasks before the condition is met.
*/
boolean syncTo(Flag flag);
boolean syncUntil(BooleanSupplier cond);
/**
* Raise a flag indicating a key point in execution.
* Wait for running tasks, so long as the given condition is met
* ({@link BooleanSupplier#getAsBoolean()} returns {@code true}).
* <br>
* If the flag was already raised, this method does nothing.
* This method is equivalent to {@code syncUntil(() -> !cond.getAsBoolean())}.
*
* @param flag The flag to raise.
* @param cond The condition sync on.
* @return {@code true} if the condition is no longer met. {@code false} if
* this executor runs out of tasks while the condition is still met.
*/
void raise(Flag flag);
/**
* Lower a flag that may have been previously raised.
* <br>
* If the flag was never raised, this method does nothing.
*
* @param flag The flag to lower.
*/
void lower(Flag flag);
/**
* Check if a flag is raised without waiting for it.
*
* @param flag The flag to check.
* @return {@code true} if the flag is raised.
*/
boolean isRaised(Flag flag);
boolean syncWhile(BooleanSupplier cond);
/**
* Check for the number of threads this executor uses.
@ -62,7 +50,7 @@ public interface TaskExecutor extends Executor {
* <br>
* This method may be called from any thread, but the runnable will only
* be executed once somebody calls either {@link #syncPoint()} or
* {@link #syncTo(Flag)}.
* {@link #syncUntil(BooleanSupplier)}.
* @param runnable The task to run.
*/
void scheduleForSync(Runnable runnable);

View file

@ -7,8 +7,8 @@ import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import com.jozufozu.flywheel.api.event.RenderStage;
import com.jozufozu.flywheel.api.task.Flag;
import com.jozufozu.flywheel.api.task.TaskExecutor;
import com.jozufozu.flywheel.lib.task.Flag;
import com.jozufozu.flywheel.lib.task.SimplyComposedPlan;
import com.jozufozu.flywheel.lib.task.StageFlag;
import com.jozufozu.flywheel.lib.task.Synchronizer;
@ -37,14 +37,14 @@ public class BatchedStagePlan implements SimplyComposedPlan<BatchContext> {
@Override
public void execute(TaskExecutor taskExecutor, BatchContext context, Runnable onCompletion) {
if (isEmpty()) {
taskExecutor.raise(flag);
flag.raise();
onCompletion.run();
return;
}
taskExecutor.execute(() -> {
var sync = new Synchronizer(bufferPlans.size(), () -> {
taskExecutor.raise(flag);
flag.raise();
onCompletion.run();
});

View file

@ -13,11 +13,11 @@ import com.jozufozu.flywheel.api.instance.InstanceType;
import com.jozufozu.flywheel.api.instance.Instancer;
import com.jozufozu.flywheel.api.model.Mesh;
import com.jozufozu.flywheel.api.model.Model;
import com.jozufozu.flywheel.api.task.Flag;
import com.jozufozu.flywheel.api.task.Plan;
import com.jozufozu.flywheel.api.task.TaskExecutor;
import com.jozufozu.flywheel.backend.engine.AbstractEngine;
import com.jozufozu.flywheel.backend.engine.InstancerKey;
import com.jozufozu.flywheel.lib.task.Flag;
import com.jozufozu.flywheel.lib.task.NamedFlag;
import com.jozufozu.flywheel.lib.task.SimplyComposedPlan;
import com.jozufozu.flywheel.lib.task.Synchronizer;
@ -57,7 +57,7 @@ public class BatchingEngine extends AbstractEngine implements SimplyComposedPlan
flush();
// Now it's safe to read stage plans in #renderStage.
taskExecutor.raise(flushFlag);
flushFlag.raise();
BatchContext ctx = BatchContext.create(context, renderOrigin);
@ -76,9 +76,9 @@ public class BatchingEngine extends AbstractEngine implements SimplyComposedPlan
@Override
public void renderStage(TaskExecutor executor, RenderContext context, RenderStage stage) {
executor.syncTo(flushFlag);
executor.syncUntil(flushFlag::isRaised);
if (stage.isLast()) {
executor.lower(flushFlag);
flushFlag.lower();
}
var stagePlan = stagePlans.get(stage);
@ -88,8 +88,8 @@ public class BatchingEngine extends AbstractEngine implements SimplyComposedPlan
return;
}
executor.syncTo(stagePlan.flag);
executor.lower(stagePlan.flag);
executor.syncUntil(stagePlan.flag::isRaised);
stagePlan.flag.lower();
drawTracker.draw(stage);
}

View file

@ -8,12 +8,12 @@ import com.jozufozu.flywheel.api.instance.Instance;
import com.jozufozu.flywheel.api.instance.InstanceType;
import com.jozufozu.flywheel.api.instance.Instancer;
import com.jozufozu.flywheel.api.model.Model;
import com.jozufozu.flywheel.api.task.Flag;
import com.jozufozu.flywheel.api.task.Plan;
import com.jozufozu.flywheel.api.task.TaskExecutor;
import com.jozufozu.flywheel.backend.engine.AbstractEngine;
import com.jozufozu.flywheel.gl.GlStateTracker;
import com.jozufozu.flywheel.gl.GlTextureUnit;
import com.jozufozu.flywheel.lib.task.Flag;
import com.jozufozu.flywheel.lib.task.NamedFlag;
import com.jozufozu.flywheel.lib.task.RaisePlan;
import com.jozufozu.flywheel.lib.task.SyncedPlan;
@ -49,7 +49,7 @@ public class IndirectEngine extends AbstractEngine {
@Override
public void renderStage(TaskExecutor executor, RenderContext context, RenderStage stage) {
if (drawManager.hasStage(stage)) {
executor.syncTo(flushFlag);
executor.syncUntil(flushFlag::isRaised);
try (var restoreState = GlStateTracker.getRestoreState()) {
setup();
@ -63,8 +63,8 @@ public class IndirectEngine extends AbstractEngine {
if (stage.isLast()) {
// Need to sync here to ensure this frame has everything executed
// in case we didn't have any stages to draw this frame.
executor.syncTo(flushFlag);
executor.lower(flushFlag);
executor.syncUntil(flushFlag::isRaised);
flushFlag.lower();
}
}

View file

@ -9,7 +9,6 @@ import com.jozufozu.flywheel.api.instance.Instance;
import com.jozufozu.flywheel.api.instance.InstanceType;
import com.jozufozu.flywheel.api.instance.Instancer;
import com.jozufozu.flywheel.api.model.Model;
import com.jozufozu.flywheel.api.task.Flag;
import com.jozufozu.flywheel.api.task.Plan;
import com.jozufozu.flywheel.api.task.TaskExecutor;
import com.jozufozu.flywheel.backend.compile.InstancingPrograms;
@ -18,6 +17,7 @@ import com.jozufozu.flywheel.backend.engine.UniformBuffer;
import com.jozufozu.flywheel.gl.GlStateTracker;
import com.jozufozu.flywheel.gl.GlTextureUnit;
import com.jozufozu.flywheel.lib.material.MaterialIndices;
import com.jozufozu.flywheel.lib.task.Flag;
import com.jozufozu.flywheel.lib.task.NamedFlag;
import com.jozufozu.flywheel.lib.task.RaisePlan;
import com.jozufozu.flywheel.lib.task.SyncedPlan;
@ -58,7 +58,7 @@ public class InstancingEngine extends AbstractEngine {
var drawSet = drawManager.get(stage);
if (!drawSet.isEmpty()) {
executor.syncTo(flushFlag);
executor.syncUntil(flushFlag::isRaised);
try (var state = GlStateTracker.getRestoreState()) {
setup();
@ -70,8 +70,8 @@ public class InstancingEngine extends AbstractEngine {
if (stage.isLast()) {
// Need to sync here to ensure this frame has everything executed
// in case we didn't have any stages to draw this frame.
executor.syncTo(flushFlag);
executor.lower(flushFlag);
executor.syncUntil(flushFlag::isRaised);
flushFlag.lower();
}
}

View file

@ -1,25 +1,22 @@
package com.jozufozu.flywheel.impl.task;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Deque;
import java.util.List;
import java.util.Queue;
import java.util.Set;
import java.util.concurrent.ConcurrentLinkedDeque;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.BooleanSupplier;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import com.jozufozu.flywheel.Flywheel;
import com.jozufozu.flywheel.api.task.Flag;
import com.jozufozu.flywheel.api.task.TaskExecutor;
import com.mojang.blaze3d.systems.RenderSystem;
import com.mojang.logging.LogUtils;
import it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet;
import net.minecraft.util.Mth;
// https://github.com/CaffeineMC/sodium-fabric/blob/5d364ed5ba63f9067fcf72a078ca310bff4db3e9/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java
@ -38,9 +35,6 @@ public class ParallelTaskExecutor implements TaskExecutor {
private final List<WorkerThread> threads = new ArrayList<>();
private final Deque<Runnable> taskQueue = new ConcurrentLinkedDeque<>();
private final Queue<Runnable> mainThreadQueue = new ConcurrentLinkedQueue<>();
private final Set<Flag> flags = Collections.synchronizedSet(new ReferenceOpenHashSet<>());
private final ThreadGroupNotifier taskNotifier = new ThreadGroupNotifier();
private final WaitGroup waitGroup = new WaitGroup();
@ -148,18 +142,36 @@ public class ParallelTaskExecutor implements TaskExecutor {
}
@Override
public boolean syncTo(Flag flag) {
public boolean syncUntil(BooleanSupplier cond) {
while (true) {
if (isRaised(flag)) {
// The flag is already raised!
if (cond.getAsBoolean()) {
// The condition is already true!
// Early return with true to indicate.
return true;
}
if (syncOneTask()) {
// Out of tasks entirely.
// The flag may have been raised though so return the result of isRaised.
return isRaised(flag);
// The condition may have flipped though so return its result.
return cond.getAsBoolean();
}
}
}
@Override
public boolean syncWhile(BooleanSupplier cond) {
while (true) {
if (!cond.getAsBoolean()) {
// The condition is already false!
// Early return with true to indicate.
return true;
}
if (syncOneTask()) {
// Out of tasks entirely.
// The condition may have flipped though so return its result.
return !cond.getAsBoolean();
}
}
}
@ -187,21 +199,6 @@ public class ParallelTaskExecutor implements TaskExecutor {
return false;
}
@Override
public void raise(Flag flag) {
flags.add(flag);
}
@Override
public void lower(Flag flag) {
flags.remove(flag);
}
@Override
public boolean isRaised(Flag flag) {
return flags.contains(flag);
}
private void processTask(Runnable task) {
try {
task.run();

View file

@ -1,17 +1,12 @@
package com.jozufozu.flywheel.impl.task;
import java.util.Set;
import java.util.function.BooleanSupplier;
import com.jozufozu.flywheel.api.task.Flag;
import com.jozufozu.flywheel.api.task.TaskExecutor;
import it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet;
public class SerialTaskExecutor implements TaskExecutor {
public static final SerialTaskExecutor INSTANCE = new SerialTaskExecutor();
private final Set<Flag> flags = new ReferenceOpenHashSet<>();
private SerialTaskExecutor() {
}
@ -30,23 +25,13 @@ public class SerialTaskExecutor implements TaskExecutor {
}
@Override
public boolean syncTo(Flag flag) {
return isRaised(flag);
public boolean syncUntil(BooleanSupplier cond) {
return cond.getAsBoolean();
}
@Override
public void raise(Flag flag) {
flags.add(flag);
}
@Override
public void lower(Flag flag) {
flags.remove(flag);
}
@Override
public boolean isRaised(Flag flag) {
return flags.contains(flag);
public boolean syncWhile(BooleanSupplier cond) {
return !cond.getAsBoolean();
}
@Override

View file

@ -7,7 +7,6 @@ import com.jozufozu.flywheel.api.backend.BackendManager;
import com.jozufozu.flywheel.api.backend.Engine;
import com.jozufozu.flywheel.api.event.RenderContext;
import com.jozufozu.flywheel.api.event.RenderStage;
import com.jozufozu.flywheel.api.task.Flag;
import com.jozufozu.flywheel.api.task.Plan;
import com.jozufozu.flywheel.api.task.TaskExecutor;
import com.jozufozu.flywheel.api.visual.DynamicVisual;
@ -22,6 +21,7 @@ import com.jozufozu.flywheel.impl.visualization.manager.BlockEntityVisualManager
import com.jozufozu.flywheel.impl.visualization.manager.EffectVisualManager;
import com.jozufozu.flywheel.impl.visualization.manager.EntityVisualManager;
import com.jozufozu.flywheel.lib.math.MatrixUtil;
import com.jozufozu.flywheel.lib.task.Flag;
import com.jozufozu.flywheel.lib.task.NamedFlag;
import com.jozufozu.flywheel.lib.task.NestedPlan;
import com.jozufozu.flywheel.lib.task.RaisePlan;
@ -151,11 +151,11 @@ public class VisualizationManagerImpl implements VisualizationManager {
*/
public void tick(double cameraX, double cameraY, double cameraZ) {
// Make sure we're done with any prior frame or tick to avoid racing.
taskExecutor.syncTo(frameFlag);
taskExecutor.lower(frameFlag);
taskExecutor.syncUntil(frameFlag::isRaised);
frameFlag.lower();
taskExecutor.syncTo(tickFlag);
taskExecutor.lower(tickFlag);
taskExecutor.syncUntil(tickFlag::isRaised);
tickFlag.lower();
tickPlan.execute(taskExecutor, new TickContext(cameraX, cameraY, cameraZ));
}
@ -171,7 +171,7 @@ public class VisualizationManagerImpl implements VisualizationManager {
public void beginFrame(RenderContext context) {
// Make sure we're done with the last tick.
// Note we don't lower here because many frames may happen per tick.
taskExecutor.syncTo(tickFlag);
taskExecutor.syncUntil(tickFlag::isRaised);
framePlan.execute(taskExecutor, context);
}

View file

@ -0,0 +1,50 @@
package com.jozufozu.flywheel.lib.task;
import java.util.concurrent.atomic.AtomicBoolean;
/**
* A flag that can be raised and lowered in a thread-safe fashion.
* <br>
* Useful when combined with {@link RaisePlan} and {@link com.jozufozu.flywheel.api.task.TaskExecutor#syncUntil TaskExecutor.syncUntil}.
*/
public class Flag {
private final AtomicBoolean raised = new AtomicBoolean(false);
/**
* Raise this flag indicating a key point in execution.
* <br>
* If the flag was already raised, this method does nothing.
*/
public void raise() {
raised.set(true);
}
/**
* Lower this flag that may have been previously raised.
* <br>
* If the flag was never raised, this method does nothing.
*/
public void lower() {
raised.set(false);
}
/**
* Check if this flag is raised.
*
* @return {@code true} if the flag is raised.
*/
public boolean isRaised() {
return raised.get();
}
/**
* Check if this flag is lowered.
*
* @return {@code true} if the flag is lowered.
*/
public boolean isLowered() {
return !isRaised();
}
}

View file

@ -1,11 +1,21 @@
package com.jozufozu.flywheel.lib.task;
import com.jozufozu.flywheel.api.task.Flag;
/**
* A flag with an arbitrary name.
*
* @param name The name of the flag, mainly for debugging purposes.
*/
public record NamedFlag(String name) implements Flag {
public final class NamedFlag extends Flag {
private final String name;
/**
* @param name The name of the flag, mainly for debugging purposes.
*/
public NamedFlag(String name) {
this.name = name;
}
@Override
public String toString() {
return "NamedFlag[" + "name=" + name + ']';
}
}

View file

@ -1,6 +1,5 @@
package com.jozufozu.flywheel.lib.task;
import com.jozufozu.flywheel.api.task.Flag;
import com.jozufozu.flywheel.api.task.TaskExecutor;
public record RaisePlan<C>(Flag flag) implements SimplyComposedPlan<C> {
@ -10,7 +9,7 @@ public record RaisePlan<C>(Flag flag) implements SimplyComposedPlan<C> {
@Override
public void execute(TaskExecutor taskExecutor, C context, Runnable onCompletion) {
taskExecutor.raise(flag);
flag.raise();
onCompletion.run();
}
}

View file

@ -1,12 +1,25 @@
package com.jozufozu.flywheel.lib.task;
import com.jozufozu.flywheel.api.event.RenderStage;
import com.jozufozu.flywheel.api.task.Flag;
/**
* A flag that is associated with a render stage.
* <br>
* Useful for synchronizing tasks for a specific render stage.
*/
public record StageFlag(RenderStage stage) implements Flag {
public final class StageFlag extends Flag {
private final RenderStage stage;
/**
* @param stage The render stage this flag is associated with.
*/
public StageFlag(RenderStage stage) {
this.stage = stage;
}
@Override
public String toString() {
return "StageFlag[" + "stage=" + stage + ']';
}
}

View file

@ -156,18 +156,18 @@ class PlanExecutionTest {
var first = new NamedFlag("ready right away");
var second = new NamedFlag("ready after we sync");
var sync = new Synchronizer(2, () -> EXECUTOR.raise(second));
var sync = new Synchronizer(2, second::raise);
RaisePlan.raise(first)
.execute(EXECUTOR, Unit.INSTANCE, sync);
Assertions.assertTrue(EXECUTOR.syncTo(first), "First flag should be raised since we submitted a plan that raises it");
Assertions.assertTrue(EXECUTOR.syncUntil(first::isRaised), "First flag should be raised since we submitted a plan that raises it");
Assertions.assertFalse(EXECUTOR.syncTo(second), "Second flag should not be raised yet.");
Assertions.assertFalse(EXECUTOR.syncUntil(second::isRaised), "Second flag should not be raised yet.");
sync.decrementAndEventuallyRun();
Assertions.assertTrue(EXECUTOR.syncTo(second), "Second flag should be raised since it was raised in sync.");
Assertions.assertTrue(EXECUTOR.syncUntil(second::isRaised), "Second flag should be raised since it was raised in sync.");
}
@Test
@ -186,58 +186,26 @@ class PlanExecutionTest {
.then(RaisePlan.raise(second))
.execute(EXECUTOR, Unit.INSTANCE);
Assertions.assertTrue(EXECUTOR.syncTo(first), "First flag should be raised since we submitted a plan that raises it.");
Assertions.assertTrue(EXECUTOR.syncUntil(first::isRaised), "First flag should be raised since we submitted a plan that raises it.");
Assertions.assertFalse(EXECUTOR.isRaised(second), "Second flag should not be raised immediately.");
Assertions.assertFalse(second.isRaised(), "Second flag should not be raised immediately.");
Assertions.assertTrue(EXECUTOR.syncTo(second), "Second flag should be raised since we were waiting for it.");
Assertions.assertTrue(EXECUTOR.syncUntil(second::isRaised), "Second flag should be raised since we were waiting for it.");
}
@Test
void syncToReturnsExpected() {
void syncUntilReturnsFlagValue() {
var flag = new NamedFlag("ready right away");
Assertions.assertFalse(EXECUTOR.syncTo(flag), "Flag should not be raised yet.");
Assertions.assertFalse(EXECUTOR.syncUntil(flag::isRaised), "Flag should not be raised yet.");
EXECUTOR.raise(flag);
flag.raise();
Assertions.assertTrue(EXECUTOR.syncTo(flag), "Flag should be raised since we raised it manually.");
Assertions.assertTrue(EXECUTOR.syncUntil(flag::isRaised), "Flag should be raised since we raised it manually.");
EXECUTOR.lower(flag);
flag.lower();
Assertions.assertFalse(EXECUTOR.syncTo(flag), "Flag should not be raised since we lowered it.");
}
@Test
void flagsAreReferenceEqual() {
var flagA = new NamedFlag("same");
var flagB = new NamedFlag("same");
Assertions.assertNotSame(flagA, flagB, "Flags should not be the same object.");
Assertions.assertEquals(flagA, flagB, "Flags should be equal.");
Assertions.assertFalse(EXECUTOR.isRaised(flagA), "Flag A should not be raised yet.");
Assertions.assertFalse(EXECUTOR.isRaised(flagB), "Flag B should not be raised yet.");
EXECUTOR.raise(flagA);
Assertions.assertTrue(EXECUTOR.isRaised(flagA), "Flag A should be raised since we raised it manually.");
Assertions.assertFalse(EXECUTOR.isRaised(flagB), "Flag B should not be raised yet.");
EXECUTOR.raise(flagB);
Assertions.assertTrue(EXECUTOR.isRaised(flagA), "Flag A should be raised since we raised it manually.");
Assertions.assertTrue(EXECUTOR.isRaised(flagB), "Flag B should be raised since we raised it manually.");
EXECUTOR.lower(flagA);
Assertions.assertFalse(EXECUTOR.isRaised(flagA), "Flag A should not be raised since we lowered it.");
Assertions.assertTrue(EXECUTOR.isRaised(flagB), "Flag B should be raised since we raised it manually.");
EXECUTOR.lower(flagB);
Assertions.assertFalse(EXECUTOR.isRaised(flagA), "Flag A should not be raised since we lowered it.");
Assertions.assertFalse(EXECUTOR.isRaised(flagB), "Flag B should not be raised since we lowered it.");
Assertions.assertFalse(EXECUTOR.syncUntil(flag::isRaised), "Flag should not be raised since we lowered it.");
}
private static void assertExpectedSequence(IntArrayList sequence, int... expected) {