AlexModGuy / Citadel

A Library containing shared code used throughout most of my projects. Can be considered a continuation of LLibrary, of which this borrows the majority of its source code from.
25 stars 29 forks source link

[1.16.3] Huge memory leak causes client to crash #12

Open ViRb3 opened 3 years ago

ViRb3 commented 3 years ago

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

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

ViRb3 commented 3 years ago

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?

ViRb3 commented 3 years ago

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

ViRb3 commented 3 years ago

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.

andytimes commented 3 years ago

@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<>();
  }
}
ViRb3 commented 3 years ago

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

DallasCarraher commented 3 years ago

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

starmun-0010 commented 3 years ago

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.

ViRb3 commented 3 years ago

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

EightBlade commented 3 years ago

I'm still having this issue on 1.16.5.

JavierMewloCx commented 2 years ago

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