Mod Loader

Fabric (actually Quilt but template didnt let me select me + see additional information)

Minecraft Version

1.19.2

Botania version

1.19.2-439-FABRIC

Modloader version

Quilt Loader 0.19.0-beta.13; QFAPI v4.0.0-beta.30 (FAPI 0.76.0 compatible)

Modpack info

https://github.com/HexxyCraft/modpack

The latest.log file

https://mclo.gs/HkNGE7M

Issue description

Both the mana enchanter and the elven gateway (especially the latter) have a lot higher performance cost than expected by the user, which stems from them checking their structure every tick.

The gateway additionally searches for pylons, making it hit especially hard. However these locations are not required to know unless items are traded or during startup. This means that those locations could be cached and only be updated when mana gets consumed. In Addition to that, the structure checking by the portal is completely unnecessary because destruction could be implemented similarly to a netherportal, where blockupdates propagate it. This would reduce it's cost immensely.
Spark profile for the gateway: https://spark.lucko.me/B5IYSSKTgQ?hl=1990

As for the enchanter, I dont have a lot of ideas other than to onlyy check when enchanting or to listen for block destructions(which minecraft might have a system for?)

Steps to reproduce

Build and initialize a mana enchanter or elven gateway and profile. Only really visible on a large scale such in large servers / in large farms.

Other information

Note that, while I only tested on the version provided, the code I refer to is still live in the xplat source set on 1.19.x. It should hold regardless of the plattform.
The main issue is that no player expects these blocks to have such a high cost when unused.

block updates only work for a portal because all the obsidian blocks touch the portal blocks, since these have an airgap we have to check every tick

Looking into the Alfheim portal recipe lookup suggestion:

• The block entity retrieves all recipes multiple times. Once whenever any item enters the portal, to determine whether to delete or store the item, then again every tick starting 4 ticks after the last item entered or an output was returned, until there are no items left in the internal list anymore.
• The internal list turns all items into single-item stacks for lookup purposes. That seems rather wasteful, but apparently is needed for the current recipe matching logic.
• Item verification iterates over all recipes, and each recipe iterates over all its inputs.
• Recipe resolution iterates over all recipes and attempts to match it against the complete item list. To do this it copies the recipe's item list into a temporary list that it iterates over while iterating over the stored items. On every match the matching ingredient is removed from the temporary list and the item is added to another temporary list. When the temporary ingredient list is empty, the temporary item list is returned as a match. Those items are then removed from the stored list (by object reference) and the output items are spawned.

I'm not sure what benefits converting the list to a container would bring, other than being able to use a built-in cache class. That class uses a matching method the portal recipes currently don't support, though. It might be worth taking inspiration from the OrechidManager, which preprocesses all recipes for things like grouping by input block type and calculating total weights. Similarly, a cache for the portal recipes could preprocess them for potential input items and other useful information that would aid matching of the available items against all the recipes. Maybe the stored items list could also be modified such that it only ever stores one stack per type and just pile items onto that stack. (This change could even be in-memory only and not necessarily visible in the serialized NBT data, if that helps with backward compatibility.)

I think there might be a couple of ways to improve performance of the Alfheim portal.

1. The block entity doesn't remember which way the portal is facing, so it needs to make a rotation-agnostic multiblock check each tick. Remembering the rotation could help skipping a needless verification attempt if the portal knows its multiblock matches the 90 degree rotation. (The same optimization probably also applies to the mana enchanter.)
2. The pylon locations are unlikely to change, and as mentioned above don't matter too much until mana needs to be consumed. (This is also somewhat similar to the Exoflame lag issue mentioned on Discord recently, where a large number of flowers will need to scan over a large area and see each other's block entities that need to be tested whether they are furnaces.)
   I propose caching known pylon locations. Each tick only those locations need to be checked for whether they still contain a pylon, and any other locations are slowly scanned over the course of several ticks (or even seconds) to discover potential newly added pylons. At certain critical moments, a complete scan could be forced, such as when the portal is first opened or if the portal would no longer be valid after there are not enough pylons anymore. At any other point, the worst that could happen is that a freshly added pylon doesn't provide mana for a short time. (This logic could even be abstracted so it can be reused for the Exoflame as well.)

Uh, github decided to hide the rest of my comment below three dots, thanks. Please expand to see more.

I'm looking into the pylon scan code and multiblock matching first, because that's the part I'm already somewhat familiar with. In addition to only checking for the expected orientation, I could imagine the idle enchanter checking its validity only once every few ticks, but that's about it. The Alfheim portal structure itself needs its checks every tick since it can't just use block update events to turn off, as explained above.
Unfortunately the AlfheimPortalBlockEntity's tick code seems to have grown into a bit of a spaghetti tree ready for harvest. For example, it may already know that its portal state is no longer valid, yet still accepts items in the same tick before closing and potentially even outputs a trading result. Attempting to resolve recipes also always performs a second full pylon scan each tick, even if none of the recipes are complete. Instead of trying to just optimize the existing code, a case could be made for rewriting it after setting up a number of game tests to ensure the rewrite behaves the same as the old code.

I think the pylon search optimizations are the most important one in this case (They add passive lag, while the recipe stuff lags when an item gets inside the portal)

@anonymous123-code Since you originally pointed out the performance impact, do you think the changes thus far are good enough to close this issue?

Going to assume this is sufficiently fixed now.