Improve Performance of NetworkInventoryHandler (#1096)

Co-authored-by: Martin Robertz <dream-master@gmx.net>
Co-authored-by: hiroscho <hiroscho@users.noreply.github.com>

src/functionalTest/java/appeng/test/me/storage/NetworkInventoryHandlerFunctionalTest.java

@@ -0,0 +1,89 @@
+package appeng.test.me.storage;
+
+import static appeng.util.item.AEItemStackType.ITEM_STACK_TYPE;
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.io.File;
+
+import net.minecraft.server.MinecraftServer;
+import net.minecraft.world.World;
+import net.minecraft.world.WorldProviderSurface;
+import net.minecraft.world.WorldServer;
+import net.minecraft.world.WorldSettings;
+import net.minecraft.world.WorldSettings.GameType;
+import net.minecraft.world.WorldType;
+import net.minecraftforge.common.DimensionManager;
+
+import org.junit.jupiter.api.Test;
+
+import appeng.api.storage.data.IAEItemStack;
+import appeng.api.storage.data.IItemList;
+import appeng.me.cache.SecurityCache;
+import appeng.me.storage.NetworkInventoryHandler;
+import appeng.test.DummySaveHandler;
+import appeng.test.mockme.MockAESystem;
+import appeng.test.mockme.MockGrid;
+import appeng.test.mockme.MockGridNode;
+import appeng.util.item.ItemList;
+
+public class NetworkInventoryHandlerFunctionalTest {
+
+    private static World createDummyWorld() {
+        if (!DimensionManager.isDimensionRegistered(256)) {
+            DimensionManager.registerProviderType(256, WorldProviderSurface.class, false);
+            DimensionManager.registerDimension(256, 256);
+        }
+        return new WorldServer(
+                MinecraftServer.getServer(),
+                new DummySaveHandler(),
+                "DummyTestWorld",
+                256,
+                new WorldSettings(256, GameType.SURVIVAL, false, false, WorldType.DEFAULT),
+                MinecraftServer.getServer().theProfiler) {
+
+            @Override
+            public File getChunkSaveLocation() {
+                return new File("dummy-ignoreme");
+            }
+        };
+    }
+
+    @Test
+    public void testGetAvailableItems() {
+        // Setup
+        World dummyWorld = createDummyWorld();
+        MockAESystem mockSystem = new MockAESystem(dummyWorld);
+        MockGrid mockGrid = mockSystem.grid;
+        MockGridNode mockNode = mockGrid.rootNode;
+        SecurityCache mockSecurity = mockSystem.grid.getCache(SecurityCache.class);
+        NetworkInventoryHandler<IAEItemStack> handler = new NetworkInventoryHandler<>(ITEM_STACK_TYPE, mockSecurity);
+        IItemList<IAEItemStack> inputList = new ItemList();
+
+        // Execute
+        IItemList<IAEItemStack> result = handler.getAvailableItems(inputList, 1);
+
+        // Verify
+        assertNotNull(result, "Result should not be null");
+        assertSame(inputList, result, "Returned list should be the same instance as the input");
+        assertEquals(inputList.size(), result.size(), "Result size should match input size");
+    }
+
+    @Test
+    public void testGetAvailableItemsWithFilter() {
+        // Setup
+        World dummyWorld = createDummyWorld();
+        MockAESystem mockSystem = new MockAESystem(dummyWorld);
+        MockGrid mockGrid = mockSystem.grid;
+        MockGridNode mockNode = mockGrid.rootNode;
+        SecurityCache mockSecurity = mockSystem.grid.getCache(SecurityCache.class);
+        NetworkInventoryHandler<IAEItemStack> handler = new NetworkInventoryHandler<>(ITEM_STACK_TYPE, mockSecurity);
+        IItemList<IAEItemStack> inputList = new ItemList();
+
+        // Execute
+        IItemList<IAEItemStack> result = handler.getAvailableItems(inputList, 1);
+
+        // Verify
+        assertNotNull(result, "Result should not be null");
+        assertEquals(inputList.size(), result.size(), "Result size should match input size");
+    }
+}

src/main/java/appeng/api/storage/IMEInventory.java

@@ -17,6 +17,8 @@ import static appeng.util.item.AEFluidStackType.FLUID_STACK_TYPE;
 import static appeng.util.item.AEItemStackType.ITEM_STACK_TYPE;
 
 import java.util.Collection;
+import java.util.Optional;
+import java.util.function.Predicate;
 
 import javax.annotation.Nonnull;
 
@@ -28,6 +30,7 @@ import appeng.api.storage.data.IAEStack;
 import appeng.api.storage.data.IAEStackType;
 import appeng.api.storage.data.IItemList;
 import appeng.util.IterationCounter;
+import appeng.util.item.NetworkItemList;
 
 /**
  * AE's Equivalent to IInventory, used to reading contents, and manipulating contents of ME Inventories.
@@ -90,6 +93,38 @@ public interface IMEInventory<StackType extends IAEStack> {
         return ret;
     }
 
+    /**
+     * Optional filtered variant of {@link #getAvailableItems(IItemList, int)}.
+     *
+     * @param out       the IItemList the results will be written to
+     * @param iteration numeric id for this iteration, use {@link appeng.util.IterationCounter#fetchNewId()} to avoid
+     *                  conflicts
+     * @param filter    optional filter
+     * @return returns same list that was passed in, is passed out. Network-aware implementations may instead return a
+     *         filtered {@link NetworkItemList} to preserve per-network accounting during nested scans.
+     */
+    default IItemList<StackType> getAvailableItems(IItemList<StackType> out, int iteration,
+            Optional<Predicate<StackType>> filter) {
+        if (!filter.isPresent()) {
+            return getAvailableItems(out, iteration);
+        }
+
+        final Predicate<StackType> predicate = filter.get();
+        final IItemList<StackType> allItems = (IItemList<StackType>) getStackType().createPrimitiveList();
+        final IItemList<StackType> availableItems = getAvailableItems(allItems, iteration);
+
+        if (availableItems instanceof NetworkItemList) {
+            return ((NetworkItemList<StackType>) availableItems).createFilteredView(predicate);
+        }
+
+        for (final StackType item : availableItems) {
+            if (predicate.test(item)) {
+                out.add(item);
+            }
+        }
+        return out;
+    }
+
     /**
      * Request a report of how many of a single item type are available in storage. It falls back to
      * {@link IMEInventory#getAvailableItems} if a more optimized implementation is not present.
@@ -120,7 +155,10 @@ public interface IMEInventory<StackType extends IAEStack> {
     @SuppressWarnings("unchecked") // changing the generic StackType to be correct here is too much of a breaking API
                                    // change
     default StackType getAvailableItem(@Nonnull StackType request, int iteration) {
-        return getAvailableItems((IItemList<StackType>) getStackType().createList(), iteration).findPrecise(request);
+        return getAvailableItems(
+                (IItemList<StackType>) getStackType().createList(),
+                iteration,
+                Optional.of(s -> s.isSameType(request))).findPrecise(request);
     }
 
     /**

src/main/java/appeng/me/cache/CraftingGridCache.java

@@ -23,6 +23,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.UUID;
@@ -31,6 +32,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.ThreadFactory;
+import java.util.function.Predicate;
 import java.util.stream.StreamSupport;
 
 import javax.annotation.Nonnull;
@@ -455,18 +457,28 @@ public class CraftingGridCache
 
     @Override
     public IItemList<IAEStack> getAvailableItems(final IItemList<IAEStack> out, int iteration) {
+        return getAvailableItems(out, iteration, Optional.empty());
+    }
+
+    @Override
+    @SuppressWarnings({ "rawtypes", "unchecked" })
+    public IItemList<IAEStack> getAvailableItems(final IItemList<IAEStack> out, int iteration,
+            Optional<Predicate<IAEStack>> filter) {
+        final IAEStackType<?> outType = out.getStackType();
+        final Predicate<IAEStack> predicate = filter.orElse(null);
+
         // add craftable items!
         for (final IAEStack<?> stack : this.craftableItems.keySet()) {
-            IAEStackType<?> type = out.getStackType();
-            if (type == null || type == stack.getStackType()) {
-                out.addCrafting(stack);
+            if ((outType == null || outType == stack.getStackType())
+                    && (predicate == null || predicate.test((IAEStack) stack))) {
+                out.addCrafting((IAEStack) stack);
             }
         }
 
         for (final IAEStack<?> stack : emitableItems) {
-            IAEStackType<?> type = out.getStackType();
-            if (type == null || type == stack.getStackType()) {
-                out.addCrafting(stack);
+            if ((outType == null || outType == stack.getStackType())
+                    && (predicate == null || predicate.test((IAEStack) stack))) {
+                out.addCrafting((IAEStack) stack);
             }
         }
 

src/main/java/appeng/me/storage/MEInventoryHandler.java

@@ -10,6 +10,7 @@
 
 package appeng.me.storage;
 
+import java.util.Optional;
 import java.util.function.Predicate;
 
 import javax.annotation.Nonnull;
@@ -122,6 +123,11 @@ public class MEInventoryHandler<T extends IAEStack<T>> implements IMEInventoryHa
 
     @Override
     public IItemList<T> getAvailableItems(final IItemList<T> out, int iteration) {
+        return this.getAvailableItems(out, iteration, Optional.empty());
+    }
+
+    @Override
+    public IItemList<T> getAvailableItems(final IItemList<T> out, int iteration, Optional<Predicate<T>> preFilter) {
         if (!this.hasReadAccess && !isVisible()) {
             return out;
         }
@@ -129,9 +135,12 @@ public class MEInventoryHandler<T extends IAEStack<T>> implements IMEInventoryHa
         if (out instanceof ItemFilterList) return getAvailableItemsFilter(out, iteration);
 
         if (this.isExtractFilterActive() && !this.myExtractPartitionList.isEmpty()) {
-            return this.filterAvailableItems(out, iteration);
+            Predicate<T> extractFilter = this.getExtractFilterCondition();
+            Predicate<T> effectiveFilter = preFilter.map(extractFilter::and).orElse(extractFilter);
+            return this.internal.getAvailableItems(out, iteration, Optional.of(effectiveFilter));
         } else {
-            return this.internal.getAvailableItems(out, iteration);
+            return !preFilter.isPresent() ? this.internal.getAvailableItems(out, iteration)
+                    : this.internal.getAvailableItems(out, iteration, preFilter);
         }
     }
 
@@ -141,18 +150,6 @@ public class MEInventoryHandler<T extends IAEStack<T>> implements IMEInventoryHa
         return bool;
     }
 
-    protected IItemList<T> filterAvailableItems(IItemList<T> out, int iteration) {
-        final IItemList<T> allAvailableItems = this.internal
-                .getAvailableItems((IItemList<T>) this.internal.getStackType().createList(), iteration);
-        Predicate<T> filterCondition = this.getExtractFilterCondition();
-        for (T item : allAvailableItems) {
-            if (filterCondition.test(item)) {
-                out.add(item);
-            }
-        }
-        return out;
-    }
-
     protected IItemList<T> getAvailableItemsFilter(IItemList<T> out, int iteration) {
         if (!getPartitionList().isEmpty() && getWhitelist() == IncludeExclude.WHITELIST) {
             for (T is : myPartitionList.getItems()) {

src/main/java/appeng/me/storage/NetworkInventoryHandler.java

@@ -17,6 +17,8 @@ import java.util.Collection;
 import java.util.Comparator;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Optional;
+import java.util.function.Predicate;
 
 import javax.annotation.Nonnull;
 
@@ -303,7 +305,13 @@ public class NetworkInventoryHandler<T extends IAEStack<T>> implements IMENetwor
 
     @Override
     @SuppressWarnings("unchecked")
-    public IItemList<T> getAvailableItems(IItemList out, int iteration) {
+    public IItemList<T> getAvailableItems(IItemList<T> out, int iteration) {
+        return getAvailableItems(out, iteration, Optional.empty());
+    }
+
+    @SuppressWarnings("unchecked")
+    @Override
+    public IItemList<T> getAvailableItems(IItemList<T> out, int iteration, Optional<Predicate<T>> filter) {
         if (this.diveIteration(this, Actionable.SIMULATE, iteration)) {
             return this.iterationItems == null ? out : this.iterationItems;
         }
@@ -313,7 +321,7 @@ public class NetworkInventoryHandler<T extends IAEStack<T>> implements IMENetwor
 
         final NetworkItemList<T> networkItemList = new NetworkItemList<>(
                 this,
-                () -> (IItemList<T>) getStackType().createList());
+                () -> (IItemList<T>) getStackType().createPrimitiveList());
         this.iterationItems = networkItemList;
 
         final IItemList<T> currentNetworkItemList = isIgnoreCrafting
@@ -328,7 +336,7 @@ public class NetworkInventoryHandler<T extends IAEStack<T>> implements IMENetwor
                 continue; // ignore any attempts to read self
             }
             final IItemList<T> passedInList = getPrimitiveItemList();
-            final IItemList<T> passedOutList = inv.getAvailableItems(passedInList, iteration);
+            final IItemList<T> passedOutList = inv.getAvailableItems(passedInList, iteration, filter);
 
             if (externalNetworkInventory != null && passedOutList instanceof NetworkItemList) {
                 networkItemList.addNetworkItems(externalNetworkInventory, passedOutList);
@@ -418,7 +426,9 @@ public class NetworkInventoryHandler<T extends IAEStack<T>> implements IMENetwor
             }
         }
         if (readsFromOtherNetwork) {
-            final T stack = this.getAvailableItems(getPrimitiveItemList(), iteration).findPrecise(request);
+            final T stack = this
+                    .getAvailableItems(getPrimitiveItemList(), iteration, Optional.of(s -> s.isSameType(request)))
+                    .findPrecise(request);
             count = addStackCount(stack, count);
         } else {
             if (this.diveIteration(this, Actionable.SIMULATE, iteration)) {

src/main/java/appeng/me/storage/StorageBusInventoryHandler.java

@@ -1,5 +1,6 @@
 package appeng.me.storage;
 
+import java.util.Optional;
 import java.util.function.Predicate;
 
 import appeng.api.storage.IMEInventory;
@@ -19,38 +20,39 @@ public class StorageBusInventoryHandler<T extends IAEStack<T>> extends MEInvento
 
     @Override
     public IItemList<T> getAvailableItems(final IItemList<T> out, int iteration) {
+        return this.getAvailableItems(out, iteration, Optional.empty());
+    }
+
+    @Override
+    public IItemList<T> getAvailableItems(final IItemList<T> out, int iteration, Optional<Predicate<T>> preFilter) {
         if (!this.hasReadAccess && !isVisible()) {
             return out;
         }
 
         if (out instanceof ItemFilterList) return this.getAvailableItemsFilter(out, iteration);
 
+        Predicate<T> filterCondition = preFilter.orElse(null);
+
         if (this.isExtractFilterActive() && !this.getExtractPartitionList().isEmpty()) {
-            return this.filterAvailableItems(out, iteration);
-        } else {
-            return this.getAvailableItems(out, iteration, e -> true);
+            Predicate<T> extractFilter = this.getExtractFilterCondition();
+            filterCondition = filterCondition == null ? extractFilter : extractFilter.and(filterCondition);
         }
-    }
 
-    @Override
-    protected IItemList<T> filterAvailableItems(IItemList<T> out, int iteration) {
-        Predicate<T> filterCondition = this.getExtractFilterCondition();
-        return getAvailableItems(out, iteration, filterCondition);
-    }
+        final IItemList<T> availableItems = this.getInternal().getAvailableItems(
+                (IItemList<T>) this.getStackType().createList(),
+                iteration,
+                Optional.of(filterCondition));
 
-    @SuppressWarnings("unchecked")
-    private IItemList<T> getAvailableItems(IItemList<T> out, int iteration, Predicate<T> filterCondition) {
-        final IItemList<T> availableItems = this.getInternal()
-                .getAvailableItems((IItemList<T>) this.getStackType().createList(), iteration);
         if (availableItems instanceof NetworkItemList) {
-            NetworkItemList<T> networkItemList = new NetworkItemList<>((NetworkItemList<T>) availableItems);
-            networkItemList.addFilter(filterCondition);
-            return networkItemList;
+            // when we cross between networks on a NetworkInventoryHandler dive, we need to break the "out" contract to
+            // avoid modifying the passed in list which belongs to a different network (and would cause double-counting
+            // for triangle-shaped networks)
+            return availableItems;
         } else {
+            // for non-cross-network dives, we need to honor the "out" contract
+            // and put the results in the passed in list
             for (T items : availableItems) {
-                if (filterCondition.test(items)) {
-                    out.add(items);
-                }
+                out.add(items);
             }
             return out;
         }

src/main/java/appeng/util/item/NetworkItemList.java

@@ -169,6 +169,22 @@ public class NetworkItemList<T extends IAEStack> implements IItemList<T> {
         return buildFinalItemList(out);
     }
 
+    public NetworkItemList<T> createFilteredView(Predicate<T> filter) {
+        final NetworkItemList<T> copy = new NetworkItemList<>(this);
+
+        if (this.predicates.isEmpty()) {
+            copy.predicates.add(filter);
+        } else {
+            // if there were existing filters then we need to add the final filter
+            // as AND instead of OR
+            for (Predicate<T> existingFilter : this.predicates) {
+                copy.predicates.add(existingFilter.and(filter));
+            }
+        }
+
+        return copy;
+    }
+
     /**
      * Writes all available items to the provided list.
      *