perf(crafting): reduce repeated dispatch overhead (#1671)

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<IAEStack<?>> getExpandedCondensedInputs(ICraftingPatternDetails details,
+                CraftingGridCache cache) {
+            condensedExpansions++;
+            return super.getExpandedCondensedInputs(details, cache);
+        }
+
+        @Override
+        protected List<IAEStack<?>> 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;
+        }
+    }
+}

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<IAEStack<?>> 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<IAEStack<?>> 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<IAEStack<?>> 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<IAEStack<?>> 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;
                         }