From ba7333184b1ecac3784367809ea1f17f79226988 Mon Sep 17 00:00:00 2001 From: KennyTV Date: Sun, 9 Feb 2020 13:10:06 +0100 Subject: [PATCH] Partly fix memory leak, remove some unnecessary concurrenthashmap usages The block entity caching in 1.13->1.12 protocol did not uncache any of the positions/blocks - this at least clears the cache on respawn, but an uncache in the chunkunload is still required to be done --- .../data/AdvancementTranslations.java | 8 ++------ .../packets/ChatPackets1_12.java | 11 +++++++---- .../Protocol1_12_2To1_13.java | 1 - .../block_entity_handlers/FlowerPotHandler.java | 13 ++++++------- .../packets/BlockItemPackets1_13.java | 16 ++++++++++++++++ .../packets/EntityPackets1_13.java | 2 ++ .../providers/BackwardsBlockEntityProvider.java | 10 +++++----- .../storage/BackwardsBlockStorage.java | 11 +++++------ 8 files changed, 43 insertions(+), 29 deletions(-) diff --git a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_11_1to1_12/data/AdvancementTranslations.java b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_11_1to1_12/data/AdvancementTranslations.java index 84907ba3..92ced218 100644 --- a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_11_1to1_12/data/AdvancementTranslations.java +++ b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_11_1to1_12/data/AdvancementTranslations.java @@ -10,11 +10,11 @@ package nl.matsv.viabackwards.protocol.protocol1_11_1to1_12.data; +import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; public class AdvancementTranslations { - private static final Map advancements = new ConcurrentHashMap<>(); + private static final Map advancements = new HashMap<>(); static { add("advancements.nether.get_wither_skull.title", "Spooky Scary Skeleton"); @@ -140,10 +140,6 @@ public class AdvancementTranslations { advancements.put(key, value); } - public static boolean has(String key) { - return advancements.containsKey(key); - } - public static String get(String key) { return advancements.get(key); } diff --git a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_11_1to1_12/packets/ChatPackets1_12.java b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_11_1to1_12/packets/ChatPackets1_12.java index 79cef96d..528aa7a9 100644 --- a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_11_1to1_12/packets/ChatPackets1_12.java +++ b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_11_1to1_12/packets/ChatPackets1_12.java @@ -75,10 +75,13 @@ public class ChatPackets1_12 extends Rewriter { if (object.isJsonObject()) { for (Map.Entry entry : copiedObj.entrySet()) { // Get the text that doesn't exist for 1.11 < - if (entry.getKey().equalsIgnoreCase("translate") && AdvancementTranslations.has(entry.getValue().getAsString())) { - String trans = entry.getValue().getAsString(); - object.remove("translate"); - object.addProperty("translate", AdvancementTranslations.get(trans)); + + if (entry.getKey().equalsIgnoreCase("translate")) { + String translate = entry.getValue().getAsString(); + String text = AdvancementTranslations.get(translate); + if (text != null) { + object.addProperty("translate", text); + } } // Handle arrays if (entry.getValue().isJsonArray()) diff --git a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/Protocol1_12_2To1_13.java b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/Protocol1_12_2To1_13.java index 9ca592f2..8d0191f7 100644 --- a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/Protocol1_12_2To1_13.java +++ b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/Protocol1_12_2To1_13.java @@ -96,7 +96,6 @@ public class Protocol1_12_2To1_13 extends BackwardsProtocol { out(State.PLAY, 0x1C, 0x1B); // Entity Status out(State.PLAY, 0x1D, -1, cancel()); // NBT Query Response (client won't send a request, so the server should not answer) out(State.PLAY, 0x1E, 0x1C); // Explosion - out(State.PLAY, 0x1F, 0x1D); // Unload Chunk out(State.PLAY, 0x20, 0x1E); // Change Game State out(State.PLAY, 0x21, 0x1F); // Keep Alive (clientbound) out(State.PLAY, 0x27, 0x25); // Entity diff --git a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/block_entity_handlers/FlowerPotHandler.java b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/block_entity_handlers/FlowerPotHandler.java index 7a406efc..cc52bdff 100644 --- a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/block_entity_handlers/FlowerPotHandler.java +++ b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/block_entity_handlers/FlowerPotHandler.java @@ -17,15 +17,16 @@ import us.myles.viaversion.libs.opennbt.tag.builtin.CompoundTag; import us.myles.viaversion.libs.opennbt.tag.builtin.IntTag; import us.myles.viaversion.libs.opennbt.tag.builtin.StringTag; +import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; public class FlowerPotHandler implements BackwardsBlockEntityProvider.BackwardsBlockEntityHandler { - private static final Map> flowers = new ConcurrentHashMap<>(); + private static final Map> flowers = new HashMap<>(); + private static final Pair AIR = new Pair<>("minecraft:air", (byte) 0); static { - register(5265, "minecraft:air", (byte) 0); + flowers.put(5265, AIR); register(5266, "minecraft:sapling", (byte) 0); register(5267, "minecraft:sapling", (byte) 1); register(5268, "minecraft:sapling", (byte) 2); @@ -58,10 +59,8 @@ public class FlowerPotHandler implements BackwardsBlockEntityProvider.BackwardsB } public Pair getOrDefault(int blockId) { - if (flowers.containsKey(blockId)) - return flowers.get(blockId); - - return flowers.get(5265); + Pair pair = flowers.get(blockId); + return pair != null ? pair : AIR; } // TODO THIS IS NEVER CALLED BECAUSE ITS NO LONGER A BLOCK ENTITY :( diff --git a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/BlockItemPackets1_13.java b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/BlockItemPackets1_13.java index 54545b76..71014d02 100644 --- a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/BlockItemPackets1_13.java +++ b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/BlockItemPackets1_13.java @@ -174,6 +174,22 @@ public class BlockItemPackets1_13 extends nl.matsv.viabackwards.api.rewriters.It } }); + // Unload chunk + protocol.registerOutgoing(State.PLAY, 0x1F, 0x1D, new PacketRemapper() { + @Override + public void registerMap() { + handler(new PacketHandler() { + @Override + public void handle(PacketWrapper wrapper) throws Exception { + /*int x = wrapper.passthrough(Type.INT); + int z = wrapper.passthrough(Type.INT; + BackwardsBlockStorage blockStorage = wrapper.user().get(BackwardsBlockStorage.class);*/ + //TODO UNCACHE BLOCKSTORAGE ENTRIES - MEMORY LEAK! + } + }); + } + }); + // Block Change protocol.out(State.PLAY, 0x0B, 0x0B, new PacketRemapper() { @Override diff --git a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/EntityPackets1_13.java b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/EntityPackets1_13.java index 457dceac..412227f8 100644 --- a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/EntityPackets1_13.java +++ b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/EntityPackets1_13.java @@ -8,6 +8,7 @@ import nl.matsv.viabackwards.protocol.protocol1_12_2to1_13.Protocol1_12_2To1_13; import nl.matsv.viabackwards.protocol.protocol1_12_2to1_13.data.EntityTypeMapping; import nl.matsv.viabackwards.protocol.protocol1_12_2to1_13.data.PaintingMapping; import nl.matsv.viabackwards.protocol.protocol1_12_2to1_13.data.ParticleMapping; +import nl.matsv.viabackwards.protocol.protocol1_12_2to1_13.storage.BackwardsBlockStorage; import nl.matsv.viabackwards.protocol.protocol1_12_2to1_13.storage.PlayerPositionStorage1_13; import us.myles.ViaVersion.api.PacketWrapper; import us.myles.ViaVersion.api.entities.Entity1_13Types; @@ -216,6 +217,7 @@ public class EntityPackets1_13 extends LegacyEntityRewriter wrapper.user().get(BackwardsBlockStorage.class).clear()); } }); diff --git a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/providers/BackwardsBlockEntityProvider.java b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/providers/BackwardsBlockEntityProvider.java index f2024752..6fceac23 100644 --- a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/providers/BackwardsBlockEntityProvider.java +++ b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/providers/BackwardsBlockEntityProvider.java @@ -21,11 +21,11 @@ import us.myles.viaversion.libs.opennbt.tag.builtin.CompoundTag; import us.myles.viaversion.libs.opennbt.tag.builtin.IntTag; import us.myles.viaversion.libs.opennbt.tag.builtin.StringTag; +import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; public class BackwardsBlockEntityProvider implements Provider { - private final Map handlers = new ConcurrentHashMap<>(); + private final Map handlers = new HashMap<>(); public BackwardsBlockEntityProvider() { handlers.put("minecraft:flower_pot", new FlowerPotHandler()); // TODO requires special treatment, manually send @@ -64,15 +64,15 @@ public class BackwardsBlockEntityProvider implements Provider { } BackwardsBlockStorage storage = user.get(BackwardsBlockStorage.class); - - if (!storage.contains(position)) { + Integer blockId = storage.get(position); + if (blockId == null) { if (Via.getManager().isDebug()) { ViaBackwards.getPlatform().getLogger().warning("Handled BlockEntity does not have a stored block :( " + id + " full tag: " + tag); } return tag; } - return handler.transform(user, storage.get(position), tag); + return handler.transform(user, blockId, tag); } /** diff --git a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/storage/BackwardsBlockStorage.java b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/storage/BackwardsBlockStorage.java index 469ee450..87260e9d 100644 --- a/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/storage/BackwardsBlockStorage.java +++ b/core/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/storage/BackwardsBlockStorage.java @@ -56,7 +56,7 @@ public class BackwardsBlockStorage extends StoredObject { } - private Map blocks = new ConcurrentHashMap<>(); + private final Map blocks = new ConcurrentHashMap<>(); public BackwardsBlockStorage(UserConnection user) { super(user); @@ -76,11 +76,7 @@ public class BackwardsBlockStorage extends StoredObject { return whitelist.contains(block); } - public boolean contains(Position position) { - return blocks.containsKey(position); - } - - public int get(Position position) { + public Integer get(Position position) { return blocks.get(position); } @@ -88,4 +84,7 @@ public class BackwardsBlockStorage extends StoredObject { return blocks.remove(position); } + public void clear() { + blocks.clear(); + } }