-
Notifications
You must be signed in to change notification settings - Fork 8
Description
Describe the bug
When IControllable::setWorkingEnabled, AbstractRecipeLogic::setHasNotEnoughEnergy, or AbstractRecipeLogic::setActive is called, SyncedTileEntityBase::writeCustomData is called unconditionally. This function stores update data in an instance-specific ArrayList<UpdateEntry> updateEntries. Entries in this list are only removed when SyncedTileEntityBase::getUpdatePacket is called.
Via profiling, I found that either getUpdatePacket isn't being called when old state == new state, or it isn't being called frequently enough. As a result, when any of the three bugged functions are called fast enough (this can happen when other mods call these functions hundreds/thousands of times per second), the lists grow without bound. This impacts performance in a couple of ways:
- Tens of gigabytes of memory can be eaten in a matter of seconds (memory leak)
- When updates do get processed, the next garbage collection cycle takes a long time because a ton of memory is released (causes freezing/lag spikes)
- The JVM spends a lot of time managing array list memory, because the growth causes the underlying
ArrayListimplementation to copy memory as the internalelementDataarray is repeatedly resized (causing freezing/lag spikes)
There are several things that could be done to improve this (such as using a fixed-size queue instead), but there is one really simple solution: add a single conditional to the implementation of these functions that returns early if there is no change:
Example for setWorkingEnabled
@Override
public void setWorkingEnabled(boolean workingEnabled) {
if (this.workingEnabled == workingEnabled)
return;
this.workingEnabled = workingEnabled;
metaTileEntity.markDirty();
World world = metaTileEntity.getWorld();
if (world != null && !world.isRemote) {
writeCustomData(1, buf -> {
buf.writeBoolean(isActive);
buf.writeBoolean(workingEnabled);
});
}
}This should be a six LoC total change across three functions, and will prevent the problem from affecting GTNE itself or any addon mods. I built a patch for this into my addon mod and it immediately fixed the issue within my mod, but it still affects GTNE itself (as well as any other GTNE addons).
Versions
Forge: 1.12.2
GTCE: 1.18.8
Modpack: Nomi dev branch 4ca1459
Addons: GregTech Energistics master branch
Setup
Playing: Both
New world generated if applicable
Steps To Reproduce
- Build a factory, insert valid items into machines
- Let it run out of power (so machines have a valid recipe but not enough power to produce)
- Attach a profiler an watch
SyncedTileEntityBase$UpdateEntryinstances grow without bound, becausesetHasNotEnoughEnergywill be called every tick - Expand your factory to 1-2k machines and watch your computer melt
Expected behavior
Memory usage should be bounded regardless of how many times these functions are called with no actual state change
Screenshots
A few seconds later:
Additional context
Discussion on Discord here
