Bewitchment (Legacy)

Bewitchment (Legacy)

9M Downloads

Bewitchment performance heavy compared to other mods

LLjo opened this issue · 37 comments

commented

Expected Behavior

Not being as heavy as other mods i got in the modpack

Actual Behavior

performance heavy

Steps to Reproduce

not sure, just play maybe

Version of Minecraft, Bewitchment, Forge, Patchouli, and Baubles used

1.12.2, bewitchment-1.12.2-0.0.22.26.jar, Forge 14.23.5.2854

Screenshots encouraged

bewitchmemt

commented

Well, if it's on a single player world, make sure you have at leasr 6 gigabytes of RAM allocated. Also, bewitchment is a bit of a larger mod compared to most others, so that also could cause some lag.

commented

I got 32 GB ram. Also I doubt it's bigger than the betweenlands, Abyssalcraft, Ice and fire. Anyway it looks like it's checking the world for potential spawns, which means the mobs from the mod or what? If it's because of the mobs it has spawned then it's still pretty bad since I got a lot of other mobs with better AI than a normal zombie. It's not a singleplayer world, I pregened my world as well

commented

can confirm happens on my private modded server. That function can often be the 2nd worst or even worst performing function on the entire server (according to lag goggles)!

commented
Dungeon Mobs Reborn (by sunconure11)
Serene Seasons (by TheAdubbz)
Hwyla (by TehNut)
BetterNether (by paulevs)
Macaw's Roofs (by sketch_macaw)
Grimoire of Gaia (by Silentine_)
Cosmetic Armor Reworked (by LainMI)
Shadowfacts' Forgelin (by ShadowfactsDev)
Useless Sword (by Furti_Two)
Vampirism Integrations (by maxanier)
ContainerFix (by Tschipp)
IvToolkit (by Ivorforce)
GottschCore (by gottsch)
ArmorPlus (by sokratis12GR)
Reskillable (by lanse505)
FancyMenu (by Keksuccino)
Unenchanting (by Noobanidus)
🥝 Kiwi (by Snownee_)
Straw Golem (by fradige95)
Minecraft Comes Alive (MCA) (by WildBamaBoy)
Teams (by CommodoreThrawn)
Wall-Jump! [FORGE] (by genandnic)
Zoo & Wild Animals Rebuilt : ZAWA (by SoggyStache)
🍳 Cuisine (by Snownee_)
Placebo (by Shadows_of_Fire)
Ancient Warfare 2 (by Shadowmage4513)
iChunUtil (by ohaiiChun)
Back Tools (by ohaiiChun)
Mob Dismemberment (by ohaiiChun)
Custom NPCs (by Noppes_)
MixinBootstrap (by LX_Gaming)
Grimoire of Gaia: Redux (by cybercat5555)
Silent Lib (by SilentChaos512)
MmmMmmMmmMmm (by bonusboni)
Bookshelf (by DarkhaxDev)
Gravestone mod - Extended (by nightkosh)
Forbidden and Arcanus (by cesar_zorak)
Material Changer (by Lellson8)
Pam's HarvestCraft (by pamharvestcraft)
NetherEx (by LogicTechCorp)
Animus (by TeamDman)
TheDragonLib (by sokratis12GR)
Mo' Bends (by iwoplaza)
Surge (by DarkhaxDev)
Spartan Weaponry (by ObliviousSpartan)
Vampirism - Become a vampire! (by maxanier)
Phosphor (Forge) (by jellysquid_)
Unique Enchantments (by Speiger)
Ice and Fire: Dragons in a whole new light! (by alex1the1666)
AbyssalCraft (by Shinoow)
Carry On (by Tschipp)
Random Things (by Lumien231)
Chisel (by tterrag1098)
MysticalLib (by EpicSquid315)
Better Survival mod (by block_vader)
ConnectedTexturesMod (by tterrag1098)
CraftTweaker (by Jaredlll08)
Advent of Ascension (Nevermine) (by Scimiguy)
Bewitchment (by sunconure11)
YUNG's Better Caves (by YUNGNICKYOUNG)
Quark (by Vazkii)
AutoRegLib (by Vazkii)
Resynth - Growable Ores, Mob Drops and Resources (by Ki11er_wolf)
Ambience (Music Mod) (by Vazkii)
stimmedcow : NoMoreRecipeConflict (by GotoLink)
Mantle (by mDiyo)
Random Loot Mod (by milomaz1)
Inventory Tweaks (by JimeoWan)
Biomes O' Plenty (by Forstride)
Fish's Undead Rising (by fish0016054)
Improved Backpacks (by DreenDexTwitch)
Blood Magic (by WayofTime)
YUNG's Better Mineshafts (Forge) (by YUNGNICKYOUNG)
Just Enough Dimensions (by masady)
Enchantment Descriptions (by DarkhaxDev)
Rocky Tweaks (by Mohron)
Cooking for Blockheads (by BlayTheNinth)
Progressive Difficulty (by Derpatiel)
MTLib (by Jaredlll08)
CompatSkills (by lanse505)
DiscordSuite (by hrznstudio)
Corpse Complex (by TheIllusiveC4)
Recurrent Complex (by Ivorforce)
Anvil Patch - lawful (by Lumber_Wizard)
Blockcraftery (by EpicSquid315)
Just Enough Items (JEI) (by mezz)
Progressive Bosses (by InsaneGames_)
Baubles (by Azanor13)
Mutant Beasts (by Chumbanotz)
Mowzie's Mobs (by bobmowzie)
LibraryEx (by LogicTechCorp)
Rough Mobs Revamped (by p1ut0nium_94)
Spartan and Fire (by cbkovak)
Patchouli (by Vazkii)
TargetingAPI (by cbkovak)
Aquaculture 2 (by Shadowclaimer)
Dungeons2 (by gottsch)
Better Combat Rebirth (by SanAndreasP)
Lapis Stays in the Enchanting Table (by csb987)
OpenModsLib (by OpenMods)
OpenBlocks (by OpenMods)
LLibrary (by _ForgeUser11902522)
Dynamic Trees - Biomes O' Plenty Compat (by mangoose3039)
Blood Arsenal (by Arcaratus)
Chameleon (by Texelsaur)
Scape and Run: Parasites (by Dhanantry)
Guide-API (by TehNut)
MK: Ultra (by cbkovak)
MK: Ultra Compatibility (by cbkovak)
Hostile Worlds - Invasions (by Corosus)
CoroUtil (by Corosus)
Elenai Dodge (by ElenaiDev)
Random Enchants (by tfarecnim)
Spartan Shields (by ObliviousSpartan)
Cyclops Core (by kroeser)
End: Reborn (by elecatron)
Storage Drawers (by Texelsaur)
Chisels & Bits (by AlgorithmX2)
Dynamic Trees (by ferreusveritas)
Item Blacklist (by DoubleDoorDev)
Bookworm (by SoggyStache)
Electroblob's Wizardry (by Electroblob)
[ANGRY PIXEL] The Betweenlands (by oily_oli)
Ordinary Coins (by flametaichou)
JustEnoughIDs (by Runemoro)
CodeChicken Lib 1.8.+ (by covers1624)
JourneyMap (by techbrew)
EvilCraft (by kroeser)
Gravestone mod - Graves (by nightkosh)
Foam​Fix (by asiekierka)
Embers Rekindled (by BordListian)

that's the mod list. using forge server Forge 14.23.5.2854.
happens more often when travelling i believe, that is becomes more performance heavy but over all it's the top heaviest mod except for mods that add a bunch of creatures.

It's harder to see without any other mods since then everything looks fine, doesnt matter what mod. Just giving you a heads up in case you can improve it. I enjoy the mod but had to remove it.

commented

I want you to run a profiler against MC and your pack. There are a number of them you can use, just look into google really. Also, a slight note, but you are out of date for BW. Update that first.

commented

Yeah i know, but the updates didn't really say anything I would think could improve it, but I can try and do a test

commented

I’m going to need more detail on this. I can’t resolve anything with this alone.

commented

Can't really test it that well without having some people online and since we removed the mod it's kinda too late. Might be fixed in the later versions, don't know. Maybe someone else can test this better unless it's just me and Storm that noticed this problem. Anyway keep up the good work, like the mod very much!

commented

The PotentialSpawns event lag is visible in dedicated server setup too. It's scaling with number of players, but it's not constant (i.e. it depends on what parts of the map are loaded, see below).

Here's a profiling example on medium sized modpack, dedicated server with Sponge and Biomes'o Plenty:
image
Full pack is here: https://www.curseforge.com/minecraft/modpacks/fsg-arcania-craft/files/3020551
Notably, there's 635 Mineshafts in our overworld during that 300 s profiling.

From a quick check on the source code, notably here:

public void onNetherEntitySpawnsCheck(WorldEvent.PotentialSpawns ev) {
, we can see multiple event handlers there that relies on the world.getChunkProvider().chunkGenerator.isInsideStructure() method.
The isInsideStructure() method is mostly used during world generation.
Internally, the base game MagGenStructure.getStructureAt() is only caching the structure start positions with no sorting. In other words, every time the game tries to spawn an entity, the mod is causing all structures from that dimensions to be reevaluated.
=> consider using your own structure check with a sorted tree, then caching the results rounded to the chunk?
=> consider using MagGenStructure.isPositionInStructure() to save some computations (less precise but much faster)?

Furthermore, those events are constantly extending the lists while any structure is loaded. This cause fast constant memory allocation.
=> consider postponing the structure/dimension check later on down the line?

From what I understood, it's easier to reproduce with map filled with features, using chunkloaders that keep a few villages, stronghold and mineshaft loaded.

commented

Hm. I have some ideas, but I kind of need to figure out how to do it.

commented

Got any examples of usage of getStructureAt?

I'm pushing out a new build that will make these spawns more situational, with the exception of the nether. And even then, weights are also lower.

commented

Let's try to go deeper in possible solutions for this laggy getStructureAt() method.
First, there's no common access to retrieve the MapGenMineshaft instance of a ChunkGenerator, so you would need to either:
1- scan the ChunkGenerator instance for a field of that type (this is painful and hacky)...
2- create your own instance of MapGenMineshaft (apparently, there's no hard dependency, but you'll need one per dimension, releasing it when closing the world, etc.).
3- replace the base game getStructureAt() method with a faster implementation of your own (this is a coremod but every mod would be benefit from it).
4- drastically reduce the need to call getStructureAt(). With Plain biome entity spawn list, in my modpack, Bewitchment total weight is 56 vs a total of 1056, so there's ~5% chance to try spawning a Bewitchment entity. You could replace WorldEvent.PotentialSpawns event with LivingSpawnEvent.CheckSpawn to cancel the spawn when outside a structure. You can even refine by saying 100% chance to keep entity inside a Minecraft and 33% chance outside.
Option 4 is a slight change in your spawn rules but it's potentially 10+ times faster than existing approach. It's also the lass hacky way to fix the issue.

commented

I guess number 4 could work, but I have been wanting to abandon maintenance mode fairly soon for 1.12.2 while the 1.16 port is worked on. There aren't many major bugs left.

With my sleep cycle pretty much trashed, while I try and get it back on track, I do not have the... best focus. I've been trying to offset at least one fix to the other offending mod, for instance, if you get my drift.

Perhaps send over a PR with your solution? I mean, you pointed this out after all, an issue I was unaware of.

Also not much motivation left to work on 1.12.2 anyways.

commented

Only condition for sending over the PR is that you should verify it does have the performance gains you mentioned.

commented

Decided to look at it myself. I think I might be able to pull it off. We shall see.

commented

Yeah okay I'm too tired to write this.

commented

I've made a prototype but for some reason the mod won't load in my debug space,

commented

Do you have any examples of number 4 in action, @LemADEC? I.e mods using spawns in that fashion for structures, for instance.

commented

I’ll merge and test it a bit.

commented

The new event is only called if the entity had a chance to spawn in the biome.
There's 2 aspects to consider :

  • Since EntityCambion has no biome set, it'll never spawn unless it's biome list gets updated. On a similar note, if a Stronghold is in a biome not listed for the EntityBlackDog, then the latest won't spawn in that structure.
  • since the mod no longer dynamically add spawn weight in structures, it should permanently be added in the EntityEntry. For example, Ghost had a natural weight of 6, and an added weight of 2 in structures. So the new natural weight should be 8.

Practically, my PR only gives a basic logic with some examples, it's up to you to adjust the weights depending on how you want your mobs to actually spawn, notably their chances to spawn outside a structure.

commented

Ah, I see.

commented

Yeah, now that I think on it, I don't think these fixes are gonna work at least, with how 1.12's builds are designed, and for a few reasons. At least, not number 4. Unless you have some fix of your own to resolve that.

  1. Some of the mobs in those events are meant to be exclusives to some structures. I.e cambions are only meant to spawn in a specific structure of their own, villages, and nether fortresses. Not all of these mobs are meant to have biome-centric spawns.
  2. Those that were not exclusive to structures also ended up stopped spawning, even if they had biome lists (i.e hellhounds for instance)
  3. Even when the weights and what not were upped properly in the config, still not a single spawn. Tried waiting for the spawn of say, ghosts, and none came after me.

Are there ways I can have this fire less?

commented
  1. the PR already gives you the opportunity to deny Cambion outside of structures.
  2. the PR gives you examples on how to handle outside structure with different ratios.
  3. I've only tested with dogs, it was working fine for me. I had Ghost spawning disabled as they were too spammy and also laggy.

What do you mean by 'fire less'?

commented

Beginning testing now. Where do you want me to look for spawn rate per biome? Just curious.

commented

As for the mod not loading, maybe try /gradlew setupDecompWorkspace idea?

Testing so far showed no spawns of any of these mobs in these structures. Granted, I went to nether fortresses, as they are the lowest hanging fruit for this.

Also noting suppression of spawns for these mobs outside of structures, despite the fact that at least a few are meant to spawn outside of structures as well, I just happened to allow them there.

commented

Fire the spawn events less. As in, the potentialspawns event. Perhaps if it was lower it could also reduce lag.

commented

That's the base game engine calling PotentialSpawns, there's no reducing it without a full rewrite of the mob spawning logic and causing issues with many other mods.

commented

Ah.

commented

As for ghost spawning, I have some potential ideas.

Also, when I tested in the workspace with your changes, I had run into the issue of the mobs not spawning. So for now, I reverted it. Even when I upped the weight, I saw no mobs from this mod.

commented

I'm gonna shuffle around spawns and what not. See how well a new config does when I push out the next build.

commented

OH. so, it really could potentially be you guys lagging my modpack on the server side. singleplayer my new pc can mostly take it after world is genned, but server side the server just chugs to no end...

and I had this really weird "lizard apocalypse" where everywhere I'd look in all my forested biomes they'd be just covered in little lizards cause I could see their health bars everywhere with neat and the lag would clear briefly when I killed them all with a command, but then they'd be replaced with maybe twice as many items of their drops... it got so bad I had to limit the amount of lizards maximum that could spawn with in control, which then made me think that could be cutting into the tps or processing power cause incontrol is constantly slapping lizards out of existence...

commented

What version are you using, in regards to the lizards?

commented

I've been toying with them as ambient mobs, but if it's breaking still, I can fix it.

commented
commented

So, are lizards overspawning or not?

commented

sorry, was afk.
But at the time it happened it was the version just before the spawn tweaks started: bewitchment-1.12.2-0.0.22.30. but I've since been keeping it updated, although just as a precaution since I only blacklisted the lizard spawns with incontrol, I left the piece in my json file, so I'm not sure if it's still occurring. But I could probably check if nothing had been done to it within those spawn fixes etc.

I do have increase mobs increasing the maximum amount of mobs that can be spawned which could be playing weirdly with it possibly.

edit a couple hours later: (yeeted increase mobs that just now after laggoggle profiling had beef with it.)

commented

Here are few other optimizations opportunities found from profiling and code reviews:

  • Ghost mobs are scanning more than 8k blocks per tick around it. Also most of those blockstates are recovered from the world 3 times. getBlockState() is slow, you want to cache it. I feel scanning only every 20 ticks with a single loop would be more than enough.
  • Silver and cold iron weakness checks are causing GC lag by constantly recreating string objects.
    The results of the related getSilverWeakness() & getColdIronWeakness() are float but they are compared to int, causing unnecessary conversion. It's usually not an issue, but those methods are called several times per tick and per entity.
  • The Altar scan logic is constantly checking if the DynamicTrees mod is loaded. That check is slow and it's result won't change since PreInit. Caching it in a property would kill 50 to 60% of the lag from that block. Also one of those call is useless as there's an empty then statement.
  • The altar scan isNatural() check is doing redundant checks. In general, you want to prioritize the most selective tests. For example, the majority of blocks around an altar are air, so we can do a fast check on that. Also, the negative checks like != GRASS, are TRUE in 96% cases (only 1 layer of grass in the whole box). Moving those negatic checks to the end of the condition expression would help. Same goes with isStatue(). Again, normally this doesn't change much, but those specific methods are called very frequently.
  • The Witches oven is constantly checking for furnace recipes (up to 3 times per tick with Botania ?). This is a slow lookup and would be better cached. On a side note, you want to skip the lookup when input is empty to save lag and exploits (right now you get free items from the oven depending on other mods installed).
  • The Witches cauldron is recreating the same AxisAlignedBB objects every tick (new AxisAlignedBB(getPos())) and every 5 ticks collectionZone.offset(getPos()).
    On a similar note, the ModConfig.misc.heatSources is a String[] but it's only used as a List which is recreated every 20 ticks. Depending on the size, a HashSet might be more efficient than scanning the list constantly. Consider caching those objects to save some GC lag.
  • The Witches cauldron is sending particle updates to all players in the dimension, which usually is the Overworld, so basically all players on the server. Those particles updates account for 90% of the Cauldron CPU load, after fixing the AxisALignedBB objects. Consider sending those less frequently and only to players who loaded the chunk?
  • The Witches cauldron seems to constantly force block updates through insertNextItem() > syncToClient().

Those should be fairly easy to fix, a more complicated one to consider is:

  • The altar is scanning constantly even with no players around. In other words, it's forcing chunks around to be loaded. Consider freezing the altar when no player is nearby, or chunks aren't loaded.