Citadel

Citadel

83M Downloads

[1.16.3] Huge memory leak causes client to crash

ViRb3 opened this issue ยท 10 comments

commented

Minecraft version: 1.16.3
Modpack: All the Mods 6 1.2.1
Includes mods:

  • citadel-1.4.1
  • ratlantis-1.0.0-1.16.3
  • rats-7.0.1-1.16.3
  • iceandfire-2.1.1-1.16.3

All my server's players have been experiencing this a lot lately. I profiled the game and found out that the culprit is in:

com.github.alexthe666.citadel.server.entity.EntityDataHandler
-->  registeredEntityData com.github.alexthe666.citadel.server.entity.WeakIdentityHashMap

It currently takes 609MB RAM, more than any other object in Minecraft's RAM. I have attached a reference graph below that I hope will help you. I am retaining the memory dumps, so feel free to ask for any more details and I will gladly assist you.

Heap walker - Reference Graph.pdf

commented

Yeap. I had a great server with a good game, but this problem arose and the truth did not find the root to give it a solution. Help me

commented

Is this still an issue? it seems to hog more memory every time we teleport or join a new world. Eventually leading to 100% allocated memory consumption and a game freeze.

commented

I haven't used any Citadel mod since posting this issue so I'm afraid I can't help.

commented

Looking at the code of EntityDataHandler, it seems like the map registeredEntityData is only put into, but never cleared.

The put method is called from here:

https://github.com/Alex-the-666/Citadel/blob/6dfa2496a106cac57ec10c12084d513ce6821180/src/main/java/com/github/alexthe666/citadel/server/CitadelServerEvents.java#L68-L90

Perhaps this is the problem?

commented

As @Alex-the-666 suggested on Discord, I set Track Entities=false in config/citadel-common.toml to work around this issue. Unfortunately, it did not solve the problem. After half a day of playing, the game starts spiking again, and here is the memory dump:

image

Could you please fix this problem? Or release up-to-date source for Citadel, so we could try to tackle it? Thanks

commented

Upon further investigation, indeed, the config controls tracking of entities, which is the EntityPropertiesTracker class, but the leaking class is the EntityDataHandler, which cannot be disabled. There are registerExtendedEntityData and getEntityData methods, but no unregister. This should be a simple fix that makes a lot of people happy, please take a look into it.

commented

@ViRb3 CurseForge provides a deobf version of citadel. After decompilation (citadel-1.4.1-deobf), the EntityDataHandler is:

public enum EntityDataHandler {
  INSTANCE;
  
  private Map<Entity, List<IEntityData<?>>> registeredEntityData;
  
  EntityDataHandler() {
    this.registeredEntityData = new WeakIdentityHashMap<>();
  }
  
  public <T extends Entity> void registerExtendedEntityData(T entity, IEntityData<T> entityData) {
    List<IEntityData<?>> registered = this.registeredEntityData.get(entity);
    if (registered == null)
      registered = new ArrayList<>(); 
    if (!registered.contains(entityData))
      registered.add(entityData); 
    this.registeredEntityData.put((Entity)entity, registered);
  }
  
  public <T extends Entity> IEntityData<T> getEntityData(T entity, String identifier) {
    List<IEntityData<T>> managers = getEntityData(entity);
    if (managers != null)
      for (IEntityData<T> manager : managers) {
        if (manager.getID().equals(identifier))
          return manager; 
      }  
    return null;
  }
  
  public <T extends Entity> List<IEntityData<T>> getEntityData(T entity) {
    if (this.registeredEntityData.containsKey(entity))
      return (List<IEntityData<T>>)this.registeredEntityData.get(entity); 
    return new ArrayList<>();
  }
}
commented

The new Citadel release 1.5.2 seems to address this issue, I will test soon and report back.

commented

Is this issue still occurring? I seem to still have huge lag spikes during particular animations on stage IV or V dragons..

commented

I'm still having this issue on 1.16.5.