From 6bc63f5510433fc1a63ac6f30f311e06c0c2195e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 11:32:42 +0000 Subject: [PATCH 1/6] Include data components in the ItemStack hash getItemStackHashCode hashed only count and item. Equality compares data components, so the hash was strictly less discriminating than equality: every stack of one item landed in the same bucket of any hash-based ingredient collection. Collapsed collections normalise the count to 1 before using a stack as a key, so the count term is constant there and the hash degenerated to a function of the item alone. With enough component variants per item the buckets treeify and every lookup turns into a tree walk doing full data component comparisons, which is what made large Integrated Dynamics storage networks scale quadratically. Profiling a 50k stack storage terminal open showed 61% of server thread samples inside treeified HashMap buckets and 65% inside IngredientInstanceWrapper.equals. Including components takes that open from 4686 ms to 168 ms of server thread time. The exclusion comment dated from NBT tags, which were expensive to hash. Component maps are not, and vanilla hashes them the same way in ItemStack.hashItemAndComponents. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mxjin31W1Lmq5XK1CCe84v --- .../helper/ItemStackHelpersCommon.java | 11 +- .../helper/TestItemStackHelpersHashCode.java | 109 ++++++++++++++++++ 2 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java diff --git a/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java b/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java index b435594025d..db0cbbaf44a 100644 --- a/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java +++ b/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java @@ -109,9 +109,14 @@ public int getItemStackHashCode(ItemStack stack) { int result = 1; result = 37 * result + stack.getCount(); result = 37 * result + stack.getItem().hashCode(); - // Tags can be very large, and expensive to calculate, which is not needed for hashCodes. - // CompoundTag tagCompound = stack.getTag(); - // result = 37 * result + (tagCompound != null ? tagCompound.hashCode() : 0); + // Data components have to be part of the hash, because equality compares them. + // Leaving them out makes every stack of the same item hash alike, so hash-based + // ingredient collections collapse into one bucket per item and every lookup turns + // into a scan doing full component comparisons. This is what made large storage + // networks scale quadratically. The exclusion dates from NBT tags, which were + // expensive to hash; component maps are not. + // Mirrors ItemStack.hashItemAndComponents, which vanilla uses for the same purpose. + result = 37 * result + stack.getComponents().hashCode(); // Not factoring in capability compatibility. Doing so would require either reflection (slow) // or an access transformer, it's highly unlikely that it'd be the only difference between // many ItemStacks in practice, and occasional hash code collisions are okay. diff --git a/loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java b/loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java new file mode 100644 index 00000000000..71ac7aa8fc7 --- /dev/null +++ b/loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java @@ -0,0 +1,109 @@ +package org.cyclops.cyclopscore.helper; + +import net.minecraft.core.Holder; +import net.minecraft.core.MappedRegistry; +import net.minecraft.core.component.DataComponentMap; +import net.minecraft.core.component.DataComponents; +import net.minecraft.core.registries.BuiltInRegistries; +import net.minecraft.network.chat.Component; +import net.minecraft.world.item.Item; +import net.minecraft.world.item.ItemStack; +import org.cyclops.cyclopscore.inventory.ItemDummy; +import org.junit.jupiter.api.Test; + +import java.util.HashSet; +import java.util.Set; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; + +/** + * Unit tests for the ItemStack hash code of {@link ItemStackHelpersCommon}. + * + * The hash has to take data components into account. Ingredient collections key on it, and a + * component-blind hash puts every stack of the same item in one bucket, which turns their + * lookups into scans over full component comparisons. + * + * @author rubensworks + */ +public class TestItemStackHelpersHashCode { + + static { + ((MappedRegistry) BuiltInRegistries.ITEM).unfreeze(true); + } + + private static final Item ITEM1 = new ItemDummy(); + private static final Item ITEM2 = new ItemDummy(); + + static { + ((Holder.Reference) ITEM1.builtInRegistryHolder()).bindComponents(DataComponentMap.EMPTY); + ((Holder.Reference) ITEM2.builtInRegistryHolder()).bindComponents(DataComponentMap.EMPTY); + } + + private static int hash(ItemStack stack) { + return new ItemStackHelpersCommon() {}.getItemStackHashCode(stack); + } + + private static ItemStack named(Item item, int count, String name) { + ItemStack stack = new ItemStack(item, count); + stack.set(DataComponents.CUSTOM_NAME, Component.literal(name)); + return stack; + } + + @Test + public void testEqualStacksHashEqual() { + assertThat(hash(new ItemStack(ITEM1)), is(hash(new ItemStack(ITEM1)))); + assertThat(hash(new ItemStack(ITEM1, 7)), is(hash(new ItemStack(ITEM1, 7)))); + assertThat(hash(named(ITEM1, 3, "a")), is(hash(named(ITEM1, 3, "a")))); + assertThat(hash(ItemStack.EMPTY), is(hash(ItemStack.EMPTY))); + } + + @Test + public void testDifferentItemsHashDifferently() { + assertThat(hash(new ItemStack(ITEM1)), is(not(hash(new ItemStack(ITEM2))))); + } + + @Test + public void testDifferentCountsHashDifferently() { + assertThat(hash(new ItemStack(ITEM1, 1)), is(not(hash(new ItemStack(ITEM1, 2))))); + } + + /** + * The regression this guards: stacks of one item differing only by components used to share a hash. + */ + @Test + public void testComponentsAffectHash() { + assertThat(hash(named(ITEM1, 1, "a")), is(not(hash(named(ITEM1, 1, "b"))))); + assertThat(hash(new ItemStack(ITEM1)), is(not(hash(named(ITEM1, 1, "a"))))); + } + + /** + * A single differing hash could be luck. Over a sample of same-item stacks the hash has to + * spread, or hash-based collections degrade to linear scans. + */ + @Test + public void testComponentVariantsSpreadOverManyBuckets() { + int samples = 1000; + Set hashes = new HashSet<>(); + for (int i = 0; i < samples; i++) { + hashes.add(hash(named(ITEM1, 1, "variant " + i))); + } + assertThat("Component variants of one item have to produce distinct hashes", + hashes.size() > samples * 0.99, is(true)); + } + + /** + * The hash may never distinguish two stacks that count as equal, or lookups miss. + */ + @Test + public void testHashIsConsistentWithComponentEquality() { + for (int i = 0; i < 100; i++) { + ItemStack a = named(ITEM1, 1 + (i % 5), "variant " + i); + ItemStack b = named(ITEM1, 1 + (i % 5), "variant " + i); + assertThat(ItemStack.isSameItemSameComponents(a, b), is(true)); + assertThat(a.getCount(), is(b.getCount())); + assertThat(hash(a), is(hash(b))); + } + } +} From 8ad85f81eaabbf7943d5c65e520d93212e7f9a5b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 11:56:21 +0000 Subject: [PATCH 2/6] Instantiate the concrete helper in the hash test ItemStackHelpersCommon is abstract on more than getItemStackHashCode, so an anonymous subclass does not compile. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mxjin31W1Lmq5XK1CCe84v --- .../cyclopscore/helper/TestItemStackHelpersHashCode.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java b/loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java index 71ac7aa8fc7..f016fb81026 100644 --- a/loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java +++ b/loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java @@ -41,8 +41,10 @@ public class TestItemStackHelpersHashCode { ((Holder.Reference) ITEM2.builtInRegistryHolder()).bindComponents(DataComponentMap.EMPTY); } + private static final IItemStackHelpers HELPERS = new ItemStackHelpersNeoForge(); + private static int hash(ItemStack stack) { - return new ItemStackHelpersCommon() {}.getItemStackHashCode(stack); + return HELPERS.getItemStackHashCode(stack); } private static ItemStack named(Item item, int count, String name) { From 8fb381afbd5c5074a2d4acf7b88d0babe6cc9f7e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 13:30:03 +0000 Subject: [PATCH 3/6] Answer classified lookups directly when the classifier holds nothing A single-classified collection partitions its instances by a category type. When a query's match condition covers that category, every match has to share the query's classifier, so an absent classifier means an empty result. contains and iterator already returned one directly, but getAll, keySet, containsKey, countKey and count fell through to the unclassified path, which scans every instance to produce a result that was already known to be empty. That fallback made the scan the common case rather than the exception. An IntegratedDynamics storage index keeps one classified map per priority level, so looking up an item touches every level, and every level that does not happen to hold that item scanned all of its entries. Measured on the IntegratedDynamics index benchmarks, item-only lookups over 5000 instances spread across 200 positions and 4 priority levels: index_lookup_item 0.240831 ms/op before 0.001157 ms/op after Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mxjin31W1Lmq5XK1CCe84v --- .../IngredientCollectionSingleClassified.java | 18 +- .../IngredientMapSingleClassified.java | 64 +++--- .../TestSingleClassifiedAbsentClassifier.java | 191 ++++++++++++++++++ 3 files changed, 238 insertions(+), 35 deletions(-) create mode 100644 loader-neoforge/src/test/java/org/cyclops/cyclopscore/ingredient/collection/TestSingleClassifiedAbsentClassifier.java diff --git a/loader-neoforge/src/main/java/org/cyclops/cyclopscore/ingredient/collection/IngredientCollectionSingleClassified.java b/loader-neoforge/src/main/java/org/cyclops/cyclopscore/ingredient/collection/IngredientCollectionSingleClassified.java index 9aab85895d5..a492598169b 100644 --- a/loader-neoforge/src/main/java/org/cyclops/cyclopscore/ingredient/collection/IngredientCollectionSingleClassified.java +++ b/loader-neoforge/src/main/java/org/cyclops/cyclopscore/ingredient/collection/IngredientCollectionSingleClassified.java @@ -176,13 +176,17 @@ public int count(T instance, M matchCondition) { } if (appliesToClassifier(matchCondition)) { IIngredientCollectionMutable collection = this.classifiedCollections.get(getClassifier(instance)); - if (collection != null) { - if (Objects.equals(getCategoryType().getMatchCondition(), matchCondition)) { - return collection.size(); - } else { - M subMatchCondition = getComponent().getMatcher().withoutCondition(matchCondition, getCategoryType().getMatchCondition()); - return collection.count(instance, subMatchCondition); - } + if (collection == null) { + // The match condition requires the classifier to be equal, so an absent classifier + // means nothing can match. Falling through to the unclassified path here would scan + // every instance for a result that is known to be zero. + return 0; + } + if (Objects.equals(getCategoryType().getMatchCondition(), matchCondition)) { + return collection.size(); + } else { + M subMatchCondition = getComponent().getMatcher().withoutCondition(matchCondition, getCategoryType().getMatchCondition()); + return collection.count(instance, subMatchCondition); } } return super.count(instance, matchCondition); diff --git a/loader-neoforge/src/main/java/org/cyclops/cyclopscore/ingredient/collection/IngredientMapSingleClassified.java b/loader-neoforge/src/main/java/org/cyclops/cyclopscore/ingredient/collection/IngredientMapSingleClassified.java index 31e51a8f941..f82e643e7ef 100644 --- a/loader-neoforge/src/main/java/org/cyclops/cyclopscore/ingredient/collection/IngredientMapSingleClassified.java +++ b/loader-neoforge/src/main/java/org/cyclops/cyclopscore/ingredient/collection/IngredientMapSingleClassified.java @@ -8,6 +8,7 @@ import javax.annotation.Nullable; import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -121,13 +122,17 @@ public int size() { public boolean containsKey(T instance, M matchCondition) { if (appliesToClassifier(matchCondition)) { IIngredientMapMutable map = this.classifiedMaps.get(getClassifier(instance)); - if (map != null) { - if (Objects.equals(getCategoryType().getMatchCondition(), matchCondition)) { - return true; - } else { - M subMatchCondition = getComponent().getMatcher().withoutCondition(matchCondition, getCategoryType().getMatchCondition()); - return map.containsKey(instance, subMatchCondition); - } + if (map == null) { + // The match condition requires the classifier to be equal, so an absent classifier + // means nothing can match. Falling through to the unclassified path here would scan + // every key for a result that is known to be empty. + return false; + } + if (Objects.equals(getCategoryType().getMatchCondition(), matchCondition)) { + return true; + } else { + M subMatchCondition = getComponent().getMatcher().withoutCondition(matchCondition, getCategoryType().getMatchCondition()); + return map.containsKey(instance, subMatchCondition); } } return super.containsKey(instance, matchCondition); @@ -137,13 +142,14 @@ public boolean containsKey(T instance, M matchCondition) { public int countKey(T instance, M matchCondition) { if (appliesToClassifier(matchCondition)) { IIngredientMapMutable map = this.classifiedMaps.get(getClassifier(instance)); - if (map != null) { - if (Objects.equals(getCategoryType().getMatchCondition(), matchCondition)) { - return map.size(); - } else { - M subMatchCondition = getComponent().getMatcher().withoutCondition(matchCondition, getCategoryType().getMatchCondition()); - return map.countKey(instance, subMatchCondition); - } + if (map == null) { + return 0; + } + if (Objects.equals(getCategoryType().getMatchCondition(), matchCondition)) { + return map.size(); + } else { + M subMatchCondition = getComponent().getMatcher().withoutCondition(matchCondition, getCategoryType().getMatchCondition()); + return map.countKey(instance, subMatchCondition); } } return super.countKey(instance, matchCondition); @@ -192,13 +198,14 @@ public Collection values() { public Collection getAll(T key, M matchCondition) { if (appliesToClassifier(matchCondition)) { IIngredientMapMutable map = this.classifiedMaps.get(getClassifier(key)); - if (map != null) { - if (Objects.equals(getCategoryType().getMatchCondition(), matchCondition)) { - return map.values(); - } else { - M subMatchCondition = getComponent().getMatcher().withoutCondition(matchCondition, getCategoryType().getMatchCondition()); - return map.getAll(key, subMatchCondition); - } + if (map == null) { + return Collections.emptyList(); + } + if (Objects.equals(getCategoryType().getMatchCondition(), matchCondition)) { + return map.values(); + } else { + M subMatchCondition = getComponent().getMatcher().withoutCondition(matchCondition, getCategoryType().getMatchCondition()); + return map.getAll(key, subMatchCondition); } } return super.getAll(key, matchCondition); @@ -208,13 +215,14 @@ public Collection getAll(T key, M matchCondition) { public IngredientSet keySet(T key, M matchCondition) { if (appliesToClassifier(matchCondition)) { IIngredientMapMutable map = this.classifiedMaps.get(getClassifier(key)); - if (map != null) { - if (Objects.equals(getCategoryType().getMatchCondition(), matchCondition)) { - return map.keySet(); - } else { - M subMatchCondition = getComponent().getMatcher().withoutCondition(matchCondition, getCategoryType().getMatchCondition()); - return map.keySet(key, subMatchCondition); - } + if (map == null) { + return new IngredientHashSet<>(getComponent()); + } + if (Objects.equals(getCategoryType().getMatchCondition(), matchCondition)) { + return map.keySet(); + } else { + M subMatchCondition = getComponent().getMatcher().withoutCondition(matchCondition, getCategoryType().getMatchCondition()); + return map.keySet(key, subMatchCondition); } } return super.keySet(key, matchCondition); diff --git a/loader-neoforge/src/test/java/org/cyclops/cyclopscore/ingredient/collection/TestSingleClassifiedAbsentClassifier.java b/loader-neoforge/src/test/java/org/cyclops/cyclopscore/ingredient/collection/TestSingleClassifiedAbsentClassifier.java new file mode 100644 index 00000000000..bd5c11e431a --- /dev/null +++ b/loader-neoforge/src/test/java/org/cyclops/cyclopscore/ingredient/collection/TestSingleClassifiedAbsentClassifier.java @@ -0,0 +1,191 @@ +package org.cyclops.cyclopscore.ingredient.collection; + +import org.cyclops.cyclopscore.ingredient.ComplexStack; +import org.cyclops.cyclopscore.ingredient.IngredientComponentStubs; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.Collection; +import java.util.Iterator; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +/** + * Queries on a single-classified collection whose classifier holds nothing. + * + * A match condition that covers the category type can only match instances sharing the query's + * classifier, so an absent classifier means an empty result. These lookups must say so directly + * rather than falling back on a scan over every classifier, which is both slower and grows with + * the size of the whole collection instead of with the size of one classifier. + * + * @author rubensworks + */ +public class TestSingleClassifiedAbsentClassifier { + + private static final int GROUP = ComplexStack.Match.GROUP; + private static final int GROUP_META = ComplexStack.Match.GROUP | ComplexStack.Match.META; + + private static final ComplexStack A01 = new ComplexStack(ComplexStack.Group.A, 0, 1, null); + private static final ComplexStack A12 = new ComplexStack(ComplexStack.Group.A, 1, 2, null); + private static final ComplexStack B01 = new ComplexStack(ComplexStack.Group.B, 0, 1, null); + /** + * Nothing of this group is ever added. + */ + private static final ComplexStack C01 = new ComplexStack(ComplexStack.Group.C, 0, 1, null); + + /** + * Counts every time an inner collection is asked to iterate, so that a fallback scan over all + * classifiers becomes visible rather than only slow. + */ + private AtomicInteger innerIterations; + + private IngredientCollectionSingleClassified> collection; + private IngredientMapSingleClassified map; + + @BeforeEach + public void before() { + this.innerIterations = new AtomicInteger(); + + this.collection = new IngredientCollectionSingleClassified<>(IngredientComponentStubs.COMPLEX, + () -> new CountingSet(this.innerIterations), + IngredientComponentStubs.COMPLEX.getCategoryTypes().get(0)); + this.collection.add(A01); + this.collection.add(A12); + this.collection.add(B01); + + this.map = new IngredientMapSingleClassified<>(IngredientComponentStubs.COMPLEX, + () -> new CountingMap(this.innerIterations), + IngredientComponentStubs.COMPLEX.getCategoryTypes().get(0)); + this.map.put(A01, "a01"); + this.map.put(A12, "a12"); + this.map.put(B01, "b01"); + + this.innerIterations.set(0); + } + + @Test + public void testCollectionCountAbsentClassifier() { + assertThat(collection.count(C01, GROUP), is(0)); + assertThat(collection.count(C01, GROUP_META), is(0)); + assertThat(innerIterations.get(), is(0)); + } + + @Test + public void testCollectionCountPresentClassifier() { + assertThat(collection.count(A01, GROUP), is(2)); + assertThat(collection.count(B01, GROUP), is(1)); + assertThat(collection.count(A01, GROUP_META), is(1)); + } + + @Test + public void testMapCountKeyAbsentClassifier() { + assertThat(map.countKey(C01, GROUP), is(0)); + assertThat(map.countKey(C01, GROUP_META), is(0)); + assertThat(innerIterations.get(), is(0)); + } + + @Test + public void testMapCountKeyPresentClassifier() { + assertThat(map.countKey(A01, GROUP), is(2)); + assertThat(map.countKey(B01, GROUP), is(1)); + assertThat(map.countKey(A01, GROUP_META), is(1)); + } + + @Test + public void testMapContainsKeyAbsentClassifier() { + assertThat(map.containsKey(C01, GROUP), is(false)); + assertThat(map.containsKey(C01, GROUP_META), is(false)); + assertThat(innerIterations.get(), is(0)); + } + + @Test + public void testMapContainsKeyPresentClassifier() { + assertThat(map.containsKey(A01, GROUP), is(true)); + assertThat(map.containsKey(A01, GROUP_META), is(true)); + } + + @Test + public void testMapGetAllAbsentClassifier() { + assertThat(map.getAll(C01, GROUP).isEmpty(), is(true)); + assertThat(map.getAll(C01, GROUP_META).isEmpty(), is(true)); + assertThat(innerIterations.get(), is(0)); + } + + @Test + public void testMapGetAllPresentClassifier() { + assertThat(map.getAll(A01, GROUP).size(), is(2)); + assertThat(map.getAll(B01, GROUP).size(), is(1)); + assertThat(map.getAll(A01, GROUP_META).size(), is(1)); + } + + @Test + public void testMapKeySetAbsentClassifier() { + assertThat(map.keySet(C01, GROUP).isEmpty(), is(true)); + assertThat(map.keySet(C01, GROUP_META).isEmpty(), is(true)); + assertThat(innerIterations.get(), is(0)); + } + + @Test + public void testMapKeySetPresentClassifier() { + assertThat(map.keySet(A01, GROUP).size(), is(2)); + assertThat(map.keySet(B01, GROUP).size(), is(1)); + assertThat(map.keySet(A01, GROUP_META).size(), is(1)); + } + + /** + * A match condition that does not cover the category type can not be answered by the + * classifier, so it still has to visit the inner collections. + */ + @Test + public void testMatchConditionOutsideClassifierStillScans() { + assertThat(map.countKey(C01, ComplexStack.Match.META), is(2)); + assertThat(innerIterations.get() > 0, is(true)); + } + + private static class CountingSet extends IngredientHashSet { + + private final AtomicInteger counter; + + public CountingSet(AtomicInteger counter) { + super(IngredientComponentStubs.COMPLEX); + this.counter = counter; + } + + @Override + public Iterator iterator() { + this.counter.incrementAndGet(); + return super.iterator(); + } + } + + private static class CountingMap extends IngredientHashMap { + + private final AtomicInteger counter; + + public CountingMap(AtomicInteger counter) { + super(IngredientComponentStubs.COMPLEX); + this.counter = counter; + } + + @Override + public Iterator> iterator() { + this.counter.incrementAndGet(); + return super.iterator(); + } + + @Override + public IngredientSet keySet() { + this.counter.incrementAndGet(); + return super.keySet(); + } + + @Override + public Collection values() { + this.counter.incrementAndGet(); + return super.values(); + } + } +} From a9b2ebf45d2c787c4b1998b7d5837482cd6cb1ef Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 13:30:03 +0000 Subject: [PATCH 4/6] Cover plain stacks in the ItemStack hash tests Stacks carrying no component patch, and stacks whose only component was set back to its default, are the cases most of a storage network consists of. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mxjin31W1Lmq5XK1CCe84v --- .../helper/TestItemStackHelpersHashCode.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java b/loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java index f016fb81026..e7d7c9052da 100644 --- a/loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java +++ b/loader-neoforge/src/test/java/org/cyclops/cyclopscore/helper/TestItemStackHelpersHashCode.java @@ -95,6 +95,32 @@ public void testComponentVariantsSpreadOverManyBuckets() { hashes.size() > samples * 0.99, is(true)); } + /** + * Stacks carrying no component patch skip the component hash, so this pins that they still + * spread over items and counts, and still agree with equality. + */ + @Test + public void testPlainStacksSpreadOverItemsAndCounts() { + assertThat(hash(new ItemStack(ITEM1, 1)), is(not(hash(new ItemStack(ITEM2, 1))))); + assertThat(hash(new ItemStack(ITEM1, 1)), is(not(hash(new ItemStack(ITEM1, 2))))); + assertThat(hash(new ItemStack(ITEM1, 5)), is(hash(new ItemStack(ITEM1, 5)))); + } + + /** + * A stack whose only component is set back to its default carries no patch any more, so it is + * equal to the plain stack and has to hash like one. + */ + @Test + public void testComponentSetBackToDefaultHashesAsPlain() { + ItemStack plain = new ItemStack(ITEM1); + ItemStack restored = new ItemStack(ITEM1); + restored.set(DataComponents.CUSTOM_NAME, Component.literal("a")); + assertThat(hash(restored), is(not(hash(plain)))); + restored.set(DataComponents.CUSTOM_NAME, null); + assertThat(ItemStack.isSameItemSameComponents(plain, restored), is(true)); + assertThat(hash(restored), is(hash(plain))); + } + /** * The hash may never distinguish two stacks that count as equal, or lookups miss. */ From e43ecda8ba7c29200eaeb34a9a785604c1882c6f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 13:41:14 +0000 Subject: [PATCH 5/6] Hash data components only for stacks that carry a patch A component map hashes its prototype alongside its patch, and the prototype is the item's default components. The item is already in the hash, so hashing its defaults again distinguishes nothing while walking the whole default map. For a plain stack, which carries no patch at all, that walk was the entire cost of the hash, and plain stacks are what most of a storage network consists of. This stays consistent with equality because PatchedDataComponentMap keeps its patch sanitized: setting a component to its default removes it from the patch rather than storing it. Two stacks of one item therefore have equal components exactly when they have equal patches. Vanilla can only report an empty patch by building one, which is free for exactly the stacks this catches, so the common implementation does that and NeoForge overrides it with the direct check. Measured on the IntegratedDynamics index benchmarks, over 5000 plain stacks distinct by item and count: index_lookup_exact 0.002014 -> 0.000988 ms/op index_modification 0.001276 -> 0.000582 ms/op Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mxjin31W1Lmq5XK1CCe84v --- .../helper/ItemStackHelpersCommon.java | 29 +++++++++++++++++-- .../helper/ItemStackHelpersNeoForge.java | 6 ++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java b/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java index db0cbbaf44a..a01352b68dd 100644 --- a/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java +++ b/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java @@ -115,11 +115,36 @@ public int getItemStackHashCode(ItemStack stack) { // into a scan doing full component comparisons. This is what made large storage // networks scale quadratically. The exclusion dates from NBT tags, which were // expensive to hash; component maps are not. - // Mirrors ItemStack.hashItemAndComponents, which vanilla uses for the same purpose. - result = 37 * result + stack.getComponents().hashCode(); + // + // Only stacks carrying a patch need it. A component map hashes its prototype alongside its + // patch, and the prototype is the item's defaults, which the item hashed above already + // stands for. Hashing it again distinguishes nothing, and it is the dominant cost for the + // plain stacks that most of a storage network consists of. + // + // This stays consistent with equality because the patch is kept sanitized: setting a + // component to its default removes it from the patch rather than storing it, so two stacks + // of one item have equal components exactly when they have equal patches. + if (hasComponentPatch(stack)) { + result = 37 * result + stack.getComponents().hashCode(); + } // Not factoring in capability compatibility. Doing so would require either reflection (slow) // or an access transformer, it's highly unlikely that it'd be the only difference between // many ItemStacks in practice, and occasional hash code collisions are okay. return result; } + + /** + * If the given stack carries data components that differ from its item's defaults. + * + * Only those have to take part in {@link #getItemStackHashCode(ItemStack)}. + * + * @param stack A non-empty stack. + * @return If the stack has a non-empty component patch. + */ + protected boolean hasComponentPatch(ItemStack stack) { + // Vanilla exposes the patch only by building one. That costs nothing for the stacks this + // is here to catch, as an empty patch yields the shared empty instance. Loaders that can + // answer without building anything should override this. + return !stack.getComponentsPatch().isEmpty(); + } } diff --git a/loader-neoforge/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersNeoForge.java b/loader-neoforge/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersNeoForge.java index cdeab05161e..ec09c8e85da 100644 --- a/loader-neoforge/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersNeoForge.java +++ b/loader-neoforge/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersNeoForge.java @@ -10,4 +10,10 @@ public class ItemStackHelpersNeoForge extends ItemStackHelpersCommon { public ItemStack getCraftingRemainingItem(ItemStack itemStack) { return itemStack.getCraftingRemainder().create(); } + + @Override + protected boolean hasComponentPatch(ItemStack stack) { + // Answers without building the patch instance that the common implementation needs + return !stack.isComponentsPatchEmpty(); + } } From ac4cf529108765022267cace4be9e20c44b09cc6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 17:31:15 +0000 Subject: [PATCH 6/6] Trim the comments around the ItemStack hash to match 1.21 Matches how this landed on master-1.21-lts in #240. The reasoning belongs in the PR, not repeated in the file. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mxjin31W1Lmq5XK1CCe84v --- .../helper/ItemStackHelpersCommon.java | 18 ------------------ .../helper/ItemStackHelpersNeoForge.java | 1 - 2 files changed, 19 deletions(-) diff --git a/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java b/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java index a01352b68dd..13cceda9636 100644 --- a/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java +++ b/loader-common/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersCommon.java @@ -109,21 +109,6 @@ public int getItemStackHashCode(ItemStack stack) { int result = 1; result = 37 * result + stack.getCount(); result = 37 * result + stack.getItem().hashCode(); - // Data components have to be part of the hash, because equality compares them. - // Leaving them out makes every stack of the same item hash alike, so hash-based - // ingredient collections collapse into one bucket per item and every lookup turns - // into a scan doing full component comparisons. This is what made large storage - // networks scale quadratically. The exclusion dates from NBT tags, which were - // expensive to hash; component maps are not. - // - // Only stacks carrying a patch need it. A component map hashes its prototype alongside its - // patch, and the prototype is the item's defaults, which the item hashed above already - // stands for. Hashing it again distinguishes nothing, and it is the dominant cost for the - // plain stacks that most of a storage network consists of. - // - // This stays consistent with equality because the patch is kept sanitized: setting a - // component to its default removes it from the patch rather than storing it, so two stacks - // of one item have equal components exactly when they have equal patches. if (hasComponentPatch(stack)) { result = 37 * result + stack.getComponents().hashCode(); } @@ -142,9 +127,6 @@ public int getItemStackHashCode(ItemStack stack) { * @return If the stack has a non-empty component patch. */ protected boolean hasComponentPatch(ItemStack stack) { - // Vanilla exposes the patch only by building one. That costs nothing for the stacks this - // is here to catch, as an empty patch yields the shared empty instance. Loaders that can - // answer without building anything should override this. return !stack.getComponentsPatch().isEmpty(); } } diff --git a/loader-neoforge/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersNeoForge.java b/loader-neoforge/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersNeoForge.java index ec09c8e85da..7f4127e705a 100644 --- a/loader-neoforge/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersNeoForge.java +++ b/loader-neoforge/src/main/java/org/cyclops/cyclopscore/helper/ItemStackHelpersNeoForge.java @@ -13,7 +13,6 @@ public ItemStack getCraftingRemainingItem(ItemStack itemStack) { @Override protected boolean hasComponentPatch(ItemStack stack) { - // Answers without building the patch instance that the common implementation needs return !stack.isComponentsPatchEmpty(); } }