This mod is too CPU-heavy
alfuken opened this issue ยท 8 comments
In a modpack with over 200 mods Betweenlands event handlers still consumes almost 30% of tick time. That's a huge number.
I'd suggest following optimisations to be applied:
-
In https://github.com/Angry-Pixel/The-Betweenlands/blob/1.12-dev/src/main/java/thebetweenlands/common/herblore/elixir/PotionRootBound.java#L64
Instead of iterating 20 times per second, you could check for the effect every 5 ticks, and change the status effect on/off. Also, since it's only 1/4 of a second, does it even makes sense to have this effect cool-off at all?... Maybe it makes more sense to just remove the cool-off? Or at least change it to 1/2s so you could perform the check 2 times per second, not 20. -
in https://github.com/Angry-Pixel/The-Betweenlands/blob/1.12-dev/src/main/java/thebetweenlands/common/block/plant/BlockWeedwoodBush.java#L218
that's a LOT of calculation just for a simple step effect. Maybe it'll be better to implement this withonEntityCollidedWithBlock
? Honestly, I'd think twice if it's really necessary to have this feature at all. -
https://github.com/Angry-Pixel/The-Betweenlands/blob/1.12-dev/src/main/java/thebetweenlands/common/handler/ItemEquipmentHandler.java
Uhm... try Baubles, maybe?..
Also, can safely be run once per second or even two, not every single tick. -
https://github.com/Angry-Pixel/The-Betweenlands/blob/1.12-dev/src/main/java/thebetweenlands/common/handler/PlayerPortalHandler.java
Another good candidate foronEntityCollidedWithBlock
. If that's not an option, this check can also be throttled down to 2 times per second, not 20. -
https://github.com/Angry-Pixel/The-Betweenlands/blob/1.12-dev/src/main/java/thebetweenlands/common/handler/PuppetHandler.java#L182
That's a huge method. Can it be split into few smaller ones, just for the sake of readability? Like, AI assignments, for example. Anyway, is every tick is necessary here?
All in all, just try to refrain from running callbacks EVERY tick. That's it.
Checked with LagGoggles mod...
- When you hit
else
condition inupdateClientEntity
, for 5 ticks the only thing you do is decrease counter, and after 5 ticks have passed, you remove the tag. Now, this is what I've had in mind:
int ticker = 0;
@SideOnly(Side.CLIENT)
private static void updateClientEntity(EntityLivingBase entity) {
ticker++;
if (ticker < 5) return; else ticker = 0;
if(entity instanceof AbstractClientPlayer) {
if(entity.getActivePotionEffect(ElixirEffectRegistry.ROOT_BOUND) != null) {
entity.addTag("thebetweenlands.rootBound")
// now, for the next 5 ticks entity will be tagged with "thebetweenlands.rootBound", untill ticker counts to 5
} else {
if (entity.getTags().contains("thebetweenlands.rootBound")) {
entity.removeTag("thebetweenlands.rootBound");
}
}
}
}
and since tags is just a HashSet, it's pretty fast.
-
My guess is that's because you want to render the effect while player is moving through the bushes. But you can use
onEntityCollision
to add a tag "inBushes", and only perform the full check if tag is set, and then at some point just remove it when player is no longer in the bushes. -
Yeah, that'd be good idea. Still, do all ticking items has to be updated every tick? Or some of them can be checked less often? Also, when player them picks up, you can add them to some watch list, and tick them from the list instead of going through the whole inventory every tick. Alternatively, you can rebuild this list from inventory every 20 ticks, and then again tick the items from the direct list to skip the inventory scan.
-
See net.minecraft.block.BlockPortal#onEntityCollidedWithBlock how it's implemented in vanilla, i think it makes sense to go the same way. Or not?
Where's that 30% number from? 30% of total tick time seems quite unrealistic to me.
Anyways, thanks for the suggestions...
- Yes, that needs to run every tick because setting motion to 0 in the potion's performEffect doesn't do much. I guess I can move updateClientEntity to ClientTickEvent though.
- IIRC there was a reason I didn't put it in onEntityCollision, but I'll check again.
- Needs to run every tick to update equipped items, just like when you have an item in your inventory. But I can add a boolean to the entity NBT when an entity has any equipment, so it can skip getting the capability and inventories for entities without any equipment.
- Will change it to PlayerTickEvent.
- Irrelevant to this topic. Yeah, could do that.
Uhm... try Baubles, maybe?..
Uhm.. no?.. Because Baubles doesn't have certain things we need.
I've profiled Crackpack 3 (~200 mods) and this is the result:
May not be 100% accurate due to sampling rate, but I get a similar result with LagGoggles.
That's nowhere near 30%. If your result really is correct then there's probably some bug.
- When you hit else condition in updateCl...
That would make practically no difference because the entity instanceof AbstractClientPlayer
is only true for the one single client player, so using ClientTickEvent makes more sense. Also what you suggested would not work because the timer needs to stay for 5 ticks after the effect has worn off, but by doing it like that it may only stay for anywhere between 1-5 ticks after effect has worn off :P
- My guess is that's because you want to re
It works fine in onEntityCollision with some slight tweaks.
- Yeah, that'd be good idea. Still, do all ticking items has ...
Further optimizations than what I suggested seem pointless to me since a vast majority of mobs will not have any equipment and the equipment capability and ticking code (apart from the boolean check) will only be called for a small fraction of mobs.
- See net.minecraft.block.BlockPortal#onEntityCollidedWithBlock how it's impl...
It needs to reset the portal timer somehow when the entity is no longer colliding. That can't be done in onEntityCollision and needs to be done on entity tick, since there's no onEntityLeaveCollision or something like that. Using PlayerTickEvent instead will reduce the overhead of this to pretty much none already.
Oh, what's that screenshot is made with? But yeah, those a drastically different results. Curious...
- you could add a tag on collision, just like in a suggestion โ2 ;) collide with portal - start checks every tick - eventually during tick check it shows you-re out of portal - remove tag - no longer checks every tick
Thanks for the comments, tho. Was nice chatting with you. :)