GTNewHorizons / GT-New-Horizons-Modpack

A big progressive questing modpack for Minecraft 1.7.10 balanced around the mod GregTech.
https://www.gtnewhorizons.com/
Other
1.01k stars 305 forks source link

Very bad TPS lag from code relating to arcane enervation emitter (gadomancy) #6996

Closed KiloJoel closed 3 years ago

KiloJoel commented 3 years ago

Which modpack version are you using?

2.0.9.0QF4 #

What happened instead? (Attach screenshots if needed)

image This method is used to create an area that disables mobGriefing for entities near any arcane enervation emitter.

The problem is it's incredibly slow and is called every tick by every zombie, and some others, adding up to an unreasonably large tick time.

It also doesn't work properly, I've tested all the things affected by mobGriefing and the only one it actually prevents is creeper explosions destroying blocks (The enervation emitter only prevents non-special creepers from exploding). #

What do you suggest instead/what changes do you propose?

As far as I can tell, there's no way to change this to maintain functionality and remove the getStackTrace() call, so unless anyone else has any ideas, I propose the method be removed. This would mean it would no longer prevent creeper explosions from destroying blocks, but regular creepers would still not explode, and endermen would still not teleport.

KiloJoel commented 3 years ago

Whoops, I forgot to say, that code is in EventHandlerWorld.java in Gadomancy

repo-alt commented 3 years ago

Actually, I don't understand why it overrides "mob griefing" only for "EntityLivingBase.onUpdate" method

KiloJoel commented 3 years ago

Actually, I don't understand why it overrides "mob griefing" only for "EntityLivingBase.onUpdate" method

Yeah, that is weird since as far as I could tell, that's the only place mobgriefing checks come from

repo-alt commented 3 years ago

So, maybe it makes sense to just remove the callstack walking?

KiloJoel commented 3 years ago

Possibly, though I wouldn't be able to say whether that would cause any side effects or not. There may be some mod getting the mobgriefing rule that we don't know about.

repo-alt commented 3 years ago

I would think this is actually good. So in the area of effect of the emitter, mobgriefing is false to everybody

KiloJoel commented 3 years ago

Another issue currently is it's using the stacktrace to verify whether lastupdated is the object that called to check mobgriefing.

If the tracktrace checking was removed, a situation could happen such as: -Zombie is updated, lastupdated set to zombie -Wither skull is updated -Wither skull checks mobgriefing -Lastupdated is still zombie, so it checks whether zombie is within enervation emitter range to determine whether the skull can break blocks

boubou19 commented 3 years ago

might be easier to totally disable the enervation emitter as we can use the corporeal attractor to prevent mobs from escaping (including endermen) and use blocks with enough blast resistance to prevent explosion grieffing from vanilla creepers.

KiloJoel commented 3 years ago

might be easier to totally disable the enervation emitter as we can use the corporeal attractor to prevent mobs from escaping (including endermen) and use blocks with enough blast resistance to prevent explosion grieffing from vanilla creepers.

I have no objection to this, but I also don't see the need to disable more than is necessary. I can imagine someone might want to use it to more easily farm music discs for example. Also, are you sure corporeal attractors prevent endermen from teleporting? I can't find any info suggesting that is the case.

Uristqwerty commented 3 years ago

Instead of walking a stack trace, what about something like setting a thread-local variable while in either of the two functions it's scanning for? I'm not familiar with the difficulty of patching vanilla(?) functions, but it might be a decently-straightforward way to preserve current functionality.

draknyte1 commented 3 years ago

I'm not familiar with the difficulty of patching vanilla(?) functions

Difficult. Using ASM or Mixins to fix something should always be a last resort and there’s < 5 people in the GTNH capable of actually doing so.

Any other solution is preferential. Walking stacktraces is also horribly slow and inefficient.