PneumaticCraft: Repressurized

PneumaticCraft: Repressurized

43M Downloads

Serious impact to performance for servers

cpw opened this issue ยท 3 comments

commented

if (!world.isRemote) {
for (TileEntity te : world.loadedTileEntityList) {
if (te instanceof TileEntityUniversalSensor) {
((TileEntityUniversalSensor) te).onEvent(event);
}
}
}

Hi, this code is causing a very visible impact to the performance of a server running with pneumaticraft. Specifically this method call is about 13% of the time spent processing everything on a running server, with no pneumaticraft objects in world that I can tell.

I would suggest tracking interested parties for your events in another way - perhaps by making your own event class and pushing these sensor events through the event bus?

Iterating every tile entity in the world isn't what I would consider efficient. One doesn't dismantle the haystack every tick to find there's no needles.

commented

Okay, I think I've fixed it with 542c560

Instead of querying the whole TileEntity list we can simply keep an own list of the interested TE's around. Made the implementation generic, because I noticed we currently query the whole TE list also here, which could use this method:
https://github.com/TeamPneumatic/pnc-repressurized/blob/master/src/main/java/me/desht/pneumaticcraft/common/ai/DroneGoToChargingStation.java#L34-L48

and this uses chunks to limit the TE's queried, but that could be improved with this implementation as well:
https://github.com/TeamPneumatic/pnc-repressurized/blob/master/src/main/java/me/desht/pneumaticcraft/common/util/PneumaticCraftUtils.java#L560-L566

I tested to make sure not to cause any memory leaks, I think removing on TileEntity#invalidate, and WorldEvent.Unload should make sure not to leave any dead TE's around (when the server stops this also triggers the WorldEvent.Unload). Just to be sure: Is there anything I'm missing?

commented

Update: Used weak hashset to make sure no dead TE's are kept bb3b6cb

And applied this system for the use cases described in previous post.

commented

Yes, you're right. This will be addressed.