From e1290975a7b00b1c54290a131b7470f8db87e2b0 Mon Sep 17 00:00:00 2001 From: Worive <13164341+Worive@users.noreply.github.com> Date: Thu, 1 Oct 2026 12:12:03 +0200 Subject: [PATCH] perf(crafting): reduce repeated dispatch overhead (#1671) --- .../CraftingCPUClusterFunctionalTest.java | 302 ++++++++++++++++++ .../implementations/CraftingCPUCluster.java | 67 ++-- 2 files changed, 341 insertions(+), 28 deletions(-) create mode 100644 src/functionalTest/java/appeng/test/me/crafting/CraftingCPUClusterFunctionalTest.java diff --git a/src/functionalTest/java/appeng/test/me/crafting/CraftingCPUClusterFunctionalTest.java b/src/functionalTest/java/appeng/test/me/crafting/CraftingCPUClusterFunctionalTest.java new file mode 100644 index 000000000..5b5c8b136 --- /dev/null +++ b/src/functionalTest/java/appeng/test/me/crafting/CraftingCPUClusterFunctionalTest.java @@ -0,0 +1,302 @@ +package appeng.test.me.crafting; + +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.reflect.Proxy; +import java.util.List; + +import net.minecraft.init.Blocks; +import net.minecraft.inventory.InventoryCrafting; +import net.minecraft.item.ItemStack; +import net.minecraft.nbt.NBTTagCompound; +import net.minecraft.nbt.NBTTagList; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import appeng.api.AEApi; +import appeng.api.config.Actionable; +import appeng.api.networking.crafting.ICraftingGrid; +import appeng.api.networking.crafting.ICraftingMedium; +import appeng.api.networking.crafting.ICraftingPatternDetails; +import appeng.api.networking.energy.IEnergyGrid; +import appeng.api.storage.data.IAEItemStack; +import appeng.api.storage.data.IAEStack; +import appeng.api.util.WorldCoord; +import appeng.helpers.PatternHelper; +import appeng.me.cache.CraftingGridCache; +import appeng.me.cluster.implementations.CraftingCPUCluster; +import appeng.test.mockme.MockGrid; +import appeng.tile.crafting.TileCraftingTile; +import appeng.util.inv.MEInventoryCrafting; +import appeng.util.item.AEItemStack; + +public class CraftingCPUClusterFunctionalTest { + + private MockGrid grid; + private CraftingGridCache cache; + private CountingCPU cpu; + private CountingMedium medium; + private ICraftingPatternDetails pattern; + private IEnergyGrid energy; + private double energyUsed; + + @BeforeEach + void setUp() { + grid = new MockGrid(); + cache = grid.getCache(ICraftingGrid.class); + cpu = new CountingCPU(); + medium = new CountingMedium(); + pattern = processingPattern(); + cache.addCraftingOption(medium, pattern); + energy = (IEnergyGrid) Proxy.newProxyInstance( + IEnergyGrid.class.getClassLoader(), + new Class[] { IEnergyGrid.class }, + (proxy, method, args) -> { + if (!method.getName().equals("extractAEPower")) { + throw new UnsupportedOperationException(method.getName()); + } + final double amount = (Double) args[0]; + if (args[1] == Actionable.MODULATE) energyUsed += amount; + return amount; + }); + } + + @AfterEach + void tearDown() { + cpu.destroy(); + grid.rootNode.destroy(); + } + + @Test + void repeatedDispatchReusesInputsAndMarksDirtyOnce() { + cpu.prepare(pattern, 64, 128); + + tick(); + + assertAll( + () -> assertEquals(64, medium.pushes), + () -> assertEquals(512, medium.consumed), + () -> assertEquals(512, energyUsed), + () -> assertEquals(0, cpu.storedInputs()), + () -> assertEquals(512, cpu.expectedOutputs()), + () -> assertEquals(64, cpu.usedOperations()), + () -> assertEquals(1, cpu.condensedExpansions), + () -> assertEquals(1, cpu.slotExpansions), + () -> assertEquals(1, cpu.core.dirtyNotifications)); + for (IAEStack input : pattern.getAEInputs()) { + assertEquals(4, input.getStackSize(), "The accepting medium must not mutate cached pattern inputs"); + } + } + + @Test + void operationLimitFlushesDirtyAndLaterTicksResolveInputsAgain() { + cpu.prepare(pattern, 64, 4); + + tick(); + + assertAll( + () -> assertEquals(4, medium.pushes), + () -> assertEquals(480, cpu.storedInputs()), + () -> assertEquals(32, cpu.expectedOutputs()), + () -> assertEquals(1, cpu.condensedExpansions), + () -> assertEquals(1, cpu.slotExpansions), + () -> assertEquals(1, cpu.core.dirtyNotifications)); + + // The CPU's operation budget includes the previous three ticks. + for (int idleTick = 0; idleTick < 3; idleTick++) { + tick(); + } + assertEquals(4, medium.pushes); + assertEquals(1, cpu.core.dirtyNotifications, "Idle ticks must not dirty the CPU"); + tick(); + + assertAll( + () -> assertEquals(8, medium.pushes), + () -> assertEquals(448, cpu.storedInputs()), + () -> assertEquals(64, cpu.expectedOutputs()), + () -> assertEquals(2, cpu.condensedExpansions), + () -> assertEquals(2, cpu.slotExpansions), + () -> assertEquals(2, cpu.core.dirtyNotifications)); + } + + @Test + void fakeCraftingAlsoCoalescesDirtyNotifications() { + cpu.prepare(pattern, 64, 128); + cpu.enableFakeCrafting(64); + + tick(); + + assertAll( + () -> assertEquals(64, medium.pushes), + () -> assertEquals(0, cpu.storedInputs()), + () -> assertEquals(0, cpu.expectedOutputs()), + () -> assertTrue(cpu.completed()), + () -> assertEquals(1, cpu.condensedExpansions), + () -> assertEquals(1, cpu.slotExpansions), + () -> assertEquals(1, cpu.core.dirtyNotifications)); + } + + @Test + void rejectedDispatchReturnsInputsWithoutMarkingDirty() { + cpu.prepare(pattern, 64, 128); + medium.accept = false; + + tick(); + + assertAll( + () -> assertEquals(0, medium.pushes), + () -> assertEquals(512, cpu.storedInputs()), + () -> assertEquals(0, cpu.expectedOutputs()), + () -> assertEquals(0, cpu.usedOperations()), + () -> assertEquals(0, cpu.core.dirtyNotifications)); + } + + @Test + void dispatchFailureFlushesDirtyAndRestoresImmediateNotifications() { + cpu.prepare(pattern, 64, 128); + medium.failAfter = 1; + + assertThrows(IllegalStateException.class, this::tick); + + assertEquals(1, medium.pushes); + assertEquals(1, cpu.core.dirtyNotifications); + cpu.markDirty(); + assertEquals(2, cpu.core.dirtyNotifications, "Notifications outside the update must remain immediate"); + } + + private void tick() { + cpu.updateCraftingLogic(grid, energy, cache); + } + + private static ICraftingPatternDetails processingPattern() { + final ItemStack encoded = AEApi.instance().definitions().items().encodedPattern().maybeStack(1).get(); + final NBTTagCompound tag = new NBTTagCompound(); + final NBTTagList inputs = new NBTTagList(); + for (int slot = 0; slot < 2; slot++) { + inputs.appendTag(new ItemStack(Blocks.cobblestone, 4).writeToNBT(new NBTTagCompound())); + } + final NBTTagList outputs = new NBTTagList(); + outputs.appendTag(new ItemStack(Blocks.stone, 8).writeToNBT(new NBTTagCompound())); + tag.setTag("in", inputs); + tag.setTag("out", outputs); + encoded.setTagCompound(tag); + return new PatternHelper(encoded, null); + } + + private static final class CountingCPU extends CraftingCPUCluster { + + private final CountingCore core = new CountingCore(); + private int condensedExpansions; + private int slotExpansions; + + private CountingCPU() { + super(new WorldCoord(0, 0, 0), new WorldCoord(0, 0, 0)); + } + + private void prepare(ICraftingPatternDetails pattern, int crafts, int operations) { + isComplete = false; + accelerator = operations - 1; + tasks.put(pattern, new Progress(crafts)); + inventory.injectItems( + AEItemStack.create(new ItemStack(Blocks.cobblestone)).setStackSize(crafts * 8L), + Actionable.MODULATE); + } + + private void enableFakeCrafting(int crafts) { + finalOutput.init(AEItemStack.create(new ItemStack(Blocks.stone)).setStackSize(crafts * 8L)); + finalOutput.setFakeCrafting(); + } + + private long storedInputs() { + final IAEItemStack remaining = inventory.extractItems( + AEItemStack.create(new ItemStack(Blocks.cobblestone)).setStackSize(Long.MAX_VALUE), + Actionable.SIMULATE); + return remaining == null ? 0 : remaining.getStackSize(); + } + + private long expectedOutputs() { + final IAEStack expected = waitingFor.findPrecise(AEItemStack.create(new ItemStack(Blocks.stone))); + return expected == null ? 0 : expected.getStackSize(); + } + + private int usedOperations() { + return usedOps[0]; + } + + private boolean completed() { + return isComplete; + } + + @Override + protected TileCraftingTile getCore() { + return core; + } + + @Override + protected List> getExpandedCondensedInputs(ICraftingPatternDetails details, + CraftingGridCache cache) { + condensedExpansions++; + return super.getExpandedCondensedInputs(details, cache); + } + + @Override + protected List> getExpandedInputs(ICraftingPatternDetails details, CraftingGridCache cache) { + slotExpansions++; + return super.getExpandedInputs(details, cache); + } + } + + private static final class Progress extends CraftingCPUCluster.TaskProgress { + + private Progress(long crafts) { + value = crafts; + } + } + + private static final class CountingCore extends TileCraftingTile { + + private int dirtyNotifications; + + @Override + public boolean isActive() { + return true; + } + + @Override + public void markDirty() { + dirtyNotifications++; + } + } + + private static final class CountingMedium implements ICraftingMedium { + + private boolean accept = true; + private int failAfter = Integer.MAX_VALUE; + private int pushes; + private long consumed; + + @Override + public boolean pushPattern(ICraftingPatternDetails details, InventoryCrafting table) { + if (pushes == failAfter) throw new IllegalStateException("Test dispatch failure"); + if (!accept) return false; + for (int slot = 0; slot < table.getSizeInventory(); slot++) { + final IAEStack input = ((MEInventoryCrafting) table).getAEStackInSlot(slot); + consumed += input.getStackSize(); + // Interfaces consume the transferred stacks in place. + input.setStackSize(0); + } + pushes++; + return true; + } + + @Override + public boolean isBusy() { + return false; + } + } +} diff --git a/src/main/java/appeng/me/cluster/implementations/CraftingCPUCluster.java b/src/main/java/appeng/me/cluster/implementations/CraftingCPUCluster.java index 58f859a41..03c1ab080 100644 --- a/src/main/java/appeng/me/cluster/implementations/CraftingCPUCluster.java +++ b/src/main/java/appeng/me/cluster/implementations/CraftingCPUCluster.java @@ -172,6 +172,8 @@ public class CraftingCPUCluster implements IAECluster, ICraftingCPU { protected boolean isComplete = true; protected int remainingOperations; protected boolean somethingChanged; + private boolean deferMarkDirty; + private boolean markDirtyPending; protected long lastTime; protected long elapsedTime; @@ -540,7 +542,11 @@ public class CraftingCPUCluster implements IAECluster, ICraftingCPU { } public void markDirty() { - this.getCore().markDirty(); + if (this.deferMarkDirty) { + this.markDirtyPending = true; + } else { + this.getCore().markDirty(); + } } protected void postCraftingStatusChange(final IAEStack diff) { @@ -776,20 +782,31 @@ public class CraftingCPUCluster implements IAECluster, ICraftingCPU { this.workableTasks.clear(); this.workableTasks.putAll(this.tasks); this.knownBusyMediums.clear(); - if (this.remainingOperations > 0) { - do { - this.somethingChanged = false; - this.executeCrafting(eg, cc); - } while (this.somethingChanged && this.remainingOperations > 0); - } - this.usedOps[2] = this.usedOps[1]; - this.usedOps[1] = this.usedOps[0]; - this.usedOps[0] = started - this.remainingOperations; + // Notify the core once after all dispatches, including early returns and fake crafting completion. + final boolean wasDeferringMarkDirty = this.deferMarkDirty; + this.deferMarkDirty = true; + try { + if (this.remainingOperations > 0) { + do { + this.somethingChanged = false; + this.executeCrafting(eg, cc); + } while (this.somethingChanged && this.remainingOperations > 0); + } + this.usedOps[2] = this.usedOps[1]; + this.usedOps[1] = this.usedOps[0]; + this.usedOps[0] = started - this.remainingOperations; - this.knownBusyMediums.clear(); + this.knownBusyMediums.clear(); - if (this.remainingOperations > 0 && !this.somethingChanged) { - this.waiting = true; + if (this.remainingOperations > 0 && !this.somethingChanged) { + this.waiting = true; + } + } finally { + this.deferMarkDirty = wasDeferringMarkDirty; + if (!this.deferMarkDirty && this.markDirtyPending) { + this.markDirtyPending = false; + this.markDirty(); + } } } @@ -836,6 +853,8 @@ public class CraftingCPUCluster implements IAECluster, ICraftingCPU { mediumListCheck = parallelismProvider.getOrDefault(details, new ArrayList<>(mediumsList)); } + // Keep expansions local so later task visits can resolve updated tunnel patterns. + List> expandedInputs = null; doWhileCraftingLoop: do { MEInventoryCrafting craftingInventory = null; didPatternCraft = false; @@ -865,10 +884,12 @@ public class CraftingCPUCluster implements IAECluster, ICraftingCPU { double sum = 0; if (craftingInventory == null) { final boolean craftable = details.isCraftable(); - final List> expandedInputs = craftable ? Arrays.asList(details.getAEInputs()) - : getExpandedInputs(details, cc); if (expandedInputs == null) { - throw new IllegalStateException("Input-only pattern expansion failed"); + expandedInputs = craftable ? Arrays.asList(details.getAEInputs()) + : getExpandedInputs(details, cc); + if (expandedInputs == null) { + throw new IllegalStateException("Input-only pattern expansion failed"); + } } for (final IAEStack anInput : expandedInputs) { @@ -964,13 +985,7 @@ public class CraftingCPUCluster implements IAECluster, ICraftingCPU { // Smart blocking is fine sending the same recipe again. if (medium.getBlockingMode() == BlockingMode.BLOCKING) break; - final List> condensedInputsForRetry = getExpandedCondensedInputs( - details, - cc); - if (condensedInputsForRetry == null) { - throw new IllegalStateException("Input-only pattern expansion failed"); - } - if (!this.canCraft(details, condensedInputsForRetry)) { + if (!this.canCraft(details, condensedInputs)) { sr = ScheduledReason.NOT_ENOUGH_INGREDIENTS; break; } @@ -1030,11 +1045,7 @@ public class CraftingCPUCluster implements IAECluster, ICraftingCPU { // Smart blocking is fine sending the same recipe again. if (medium.getBlockingMode() == BlockingMode.BLOCKING) break; - final List> condensedInputsForRetry = getExpandedCondensedInputs(details, cc); - if (condensedInputsForRetry == null) { - throw new IllegalStateException("Input-only pattern expansion failed"); - } - if (!this.canCraft(details, condensedInputsForRetry)) { + if (!this.canCraft(details, condensedInputs)) { sr = ScheduledReason.NOT_ENOUGH_INGREDIENTS; break; }