Repository navigation
Index candidate creative tabs before counting JEI items - #699
BonsUnleashed wants to merge 2 commits into
Conversation
|
The full-pack test caught an overly cautious fallback here: Mekanism's tabs inherit the standard item-list getters, but the original guard rejected every subclass. I've pushed a follow-up that accepts inherited getters while keeping the original loop for custom collection methods or unfamiliar sets. The revised native build used the index during an ordinary creative search. An untimed comparison matched every original count and all represented tabs across 36,142 ingredients. The complete operation, including index construction, took 1.54 seconds in the baseline and 0.42 seconds with the revision; allocation rose from 50.8 to 54.1 MiB. That's one fresh process per arm on the same machine, so I wouldn't treat it as a settled speedup estimate. The results and reproduction sources are here. The earlier measurements remain unchanged as evidence of the fallback, and the PR description now reflects the new check and its allocation cost. |
|
Ok, so there's something here since you got a 4x speedup. There is too much word salad here and the code change is rather large for what it sounds like it does. It would be nice if you could concisely and clearly explain the root cause of the performance issue. Then I can decide whether this is the right approach to solve it |
|
Short version:
The fix is to hash each stack once and only ask the tabs that hold something with that hash. The diff is large because of the fallbacks: tabs that return their own collection, sets that are not vanilla's type-and-tag set, empty stacks. Without them it is about ten lines: one map from stack to its tabs, built with the same hash strategy the tab sets use, then one lookup per JEI stack. If you'd rather have that version, say so and I'll swap it in. And fair enough on #697 and #698. Thanks for looking at the numbers. |
|
So really the issue you've flagged is that ItemStack.hashCode is expensive and not cached? If it was cached, we wouldn't have to bandage around that problem everywhere else, correct? |
|
Yes, repeated content hashing is a large part of the cost here. More precisely, it is the The complication is invalidation: stacks and their nested NBT can change in place. A persistent cache would need to catch those changes. This PR avoids that problem by keeping its index within one counting operation; it also skips most of the 8.8 million tab-membership checks. I have not tested a shared hash cache, so the measurements here do not establish that this index is the better approach. My previous explanation should have separated the repeated hashing from the number of lookups. |
|
It's a valid observation but we cannot add 120 lines of bespoke logic at every site that uses |
|
Understood. The added complexity is too much for this call site. Thanks for reviewing it. |
ModernFix's first JEI-backed creative search checks every JEI stack against every tab. This builds a temporary index of the tabs containing each item-stack hash, then runs the existing membership check only for possible matches.
Hash collisions still reach the original
containscall. The index requires vanilla's exact item set and type-and-tag strategy. Tabs may inherit the stock collection getters; subclasses declaring collection-returning methods keep the original loop. That check does not depend on mapped method names. Empty stacks retain the full check, and tab/stack order is preserved. Nothing is cached across searches or reloads.The equivalent change is prepared for Bons and Furious 1.0.30. This native revision passed compilation and reobfuscation, plus 321 helper cases in each of two mapping variants. In the actual loaded pack, the index handled 36,142 ingredients and produced exactly the original per-tab counts and 235 represented tabs. The original-loop comparison ran outside the timers.
In one matched client pair, the complete counting operation, including index construction, took 1.54 seconds before and 0.42 seconds after. Thread allocation increased from 50.8 to 54.1 MiB. This is one process per arm, so it is an exploratory result rather than a stable speedup estimate. Raw results, settings and reproduction sources include the limitations and the earlier fallback explanation.