Hello,

my friends and I recently stared an All The Mods 6 server and with like 5 players online We have been getting very low tps, around 8 constantly. I ran Spark to try and diagnose the issue and after reducing mob spawns the next largest cause of lag seems to be a lot of enchantment related event seem to fire, from different mods like Cyclic, but through the forge API they all seem to call net.minecraft.enchantment.EnchantmentHelper.getEnchantments() which in turn always calls hellfirepvp.astralsorcery.common.util.ASMHookEndpoint.applyNewEnchantmentLevels() thise goes on to call hellfirepvp.astralsorcery.common.enchantment.amulet.AmuletEnchantmentHelper.getPlayerHavingTool() within which it seems like the ItemStack copy() method is called frequently enough to account for a lot of the tick time.

While a single enchantment event does not take too much this adds up to like 50% of the tick time or more, as seen in the profiler output.

This is my understanding of the issue, feel free to correct me if I am wrong and if I should report this issue elsewhere.

Other relevant information:

Mod version: 1.13.9
Forge version: forge-1.16.5-36.0.46

Spark profile: https://spark.lucko.me/esZKznLRVF

CPU: 2x Intel Xeon X5660 6-core (8-cores for the server)
RAM: 10GB (spark reports only ~2-3GB used)
OS: Debian, inside an LXC container in Proxmox

Java version:

openjdk version "1.8.0_282"
OpenJDK Runtime Environment (AdoptOpenJDK)(build 1.8.0_282-b08)
OpenJDK 64-Bit Server VM (AdoptOpenJDK)(build 25.282-b08, mixed mode)

Java flags: (almost default as provided by the modpack)

  javaArgs:
    - "-d64"
    - "-server"
    - "-XX:+AggressiveOpts"
    - "-XX:ParallelGCThreads=8"
    - "-XX:+UseConcMarkSweepGC"
    - "-XX:+UnlockExperimentalVMOptions"
    - "-XX:+UseParNewGC"
    - "-XX:+ExplicitGCInvokesConcurrent"
    - "-XX:MaxGCPauseMillis=15"
    - "-XX:GCPauseIntervalMillis=50"
    - "-XX:+UseFastAccessorMethods"
    - "-XX:+OptimizeStringConcat"
    - "-XX:NewSize=84m"
    - "-XX:+UseAdaptiveGCBoundary"
    - "-XX:NewRatio=3"
    - "-Dfml.readTimeout=300"
    - "-Dfml.queryResult=confirm"

Ok, so my analysis turned out correct, the ItemStack copy()was really taking a huge toll on the tps, just by refactoring the code to no use the copy() method the TPS went from 6-7 up to 18-20. This is a huuuge improvement. We are still having some unrelated issues with mobs, ignore that on the profiler, but most of the processing from AS has been eliminated.

I must warn that I am not very well familiar with making Forge mods, but I do make server plugins for Spigot so I have deep knowledge of Java. I tried my best to look into the compare method and setDamage() to ensure the new code is functionally equal, but I urge you to either reimplement this yourself, or double check my code. This is the reason I decided not to do a proper Pull Request for this. I am also keeping the issue open for the time being.

This is the new profiler logs, after recompiling the latest source code on 1.16-indev branch with the following change.

https://spark.lucko.me/oo6hHHtb6f

The following are my changes, with the original code snippet commented out. This is in AmuletEnchantmentHelper, lines 110:

        /*ItemStack cmpThis = anyTool.copy();
        cmpThis.setDamage(0);

        //Check if the player actually wears/carries the tool
        boolean foundTool = false;
        for (EquipmentSlotType slot : EquipmentSlotType.values()) {
            ItemStack stack = player.getItemStackFromSlot(slot).copy();
            stack.setDamage(0);
            if (ItemComparator.compare(stack, cmpThis, ItemComparator.Clause.Sets.ITEMSTACK_STRICT)) {
                foundTool = true;
                break;
            }
        }
        if (!foundTool) return null;*/

        int originalDamage = anyTool.getDamage(); //Save the original damage on the tool

        //Check if the player actually wears/carries the tool
        boolean foundTool = false;
        for (EquipmentSlotType slot : EquipmentSlotType.values()) {
            ItemStack stack = player.getItemStackFromSlot(slot);
            anyTool.setDamage(stack.getDamage()); //Make sure the damages are equal before comparing
            if (ItemComparator.compare(stack, anyTool, ItemComparator.Clause.Sets.ITEMSTACK_STRICT)) {
                foundTool = true;
                break;
            }
        }
        anyTool.setDamage(originalDamage); //Reset original damage on the tool