search

CitizensDev / Citizens2

Citizens - the premier plugin and API for creating server-side NPCs in Minecraft.
https://citizensnpcs.co
Open Software License 3.0
589 stars 313 forks source link

Legacy Material is slowing down server boot #2473

Closed Artuto closed 3 years ago

Artuto commented 3 years ago

Server boot is getting slowed down by Citizen's usage of Legacy Material.

How to reproduce:

  1. /npc create test
  2. /npc inventory
  3. Give it any item
  4. Enable debug in server.properties and reboot server

The result will be this warning that gives a convenient exception to track who is using Legacy Materials:

[18:46:14 WARN]: Initializing Legacy Material Support. Unless you have legacy plugins and/or data this is a bug!
[18:46:14 WARN]: java.lang.Exception
[18:46:14 WARN]:        at org.bukkit.craftbukkit.v1_16_R3.legacy.CraftLegacy.<clinit>(CraftLegacy.java:259)
[18:46:14 WARN]:        at org.bukkit.craftbukkit.v1_16_R3.util.CraftMagicNumbers.toLegacy(CraftMagicNumbers.java:192)
[18:46:14 WARN]:        at org.bukkit.inventory.ItemStack.getData(ItemStack.java:176)
[18:46:14 WARN]:        at net.citizensnpcs.api.util.ItemStorage.loadItemStack(ItemStorage.java:326)
[18:46:14 WARN]:        at net.citizensnpcs.api.trait.trait.Inventory.parseContents(Inventory.java:127)
[18:46:14 WARN]:        at net.citizensnpcs.api.trait.trait.Inventory.load(Inventory.java:81)
[18:46:14 WARN]:        at net.citizensnpcs.api.npc.AbstractNPC.loadTrait(AbstractNPC.java:328)
[18:46:14 WARN]:        at net.citizensnpcs.api.npc.AbstractNPC.load(AbstractNPC.java:322)
[18:46:14 WARN]:        at net.citizensnpcs.npc.CitizensNPC.load(CitizensNPC.java:157)
[18:46:14 WARN]:        at net.citizensnpcs.api.npc.SimpleNPCDataStore.loadInto(SimpleNPCDataStore.java:59)
[18:46:14 WARN]:        at net.citizensnpcs.Citizens$CitizensLoadTask.run(Citizens.java:501)
[18:46:14 WARN]:        at org.bukkit.craftbukkit.v1_16_R3.scheduler.CraftTask.run(CraftTask.java:100)
[18:46:14 WARN]:        at org.bukkit.craftbukkit.v1_16_R3.scheduler.CraftScheduler.mainThreadHeartbeat(CraftScheduler.java:468)
[18:46:14 WARN]:        at net.minecraft.server.v1_16_R3.MinecraftServer.w(MinecraftServer.java:947)
[18:46:14 WARN]:        at net.minecraft.server.v1_16_R3.MinecraftServer.lambda$a$0(MinecraftServer.java:175)
[18:46:14 WARN]:        at java.base/java.lang.Thread.run(Thread.java:834)
[18:46:21 INFO]: [Citizens] Loaded 3 NPCs.

The output of command /version on my server is: This server is running Paper version git-Paper-467 (MC: 1.16.5) (Implementing API version 1.16.5-R0.1-SNAPSHOT) The output of command /version citizens on my server is: Citizens version 2.0.27-SNAPSHOT (build 2260)

mcmonkey4eva commented 3 years ago

I would be pretty surprised if that's actually slowing anything down, but... yeah Citizens in 1.16.x should not be operating with 1.12-era data (this is probably only there at all because Citizens still theoretically supports all the way back to MC 1.8, which tends to leave a lot of weird messes in the code like that one) https://github.com/CitizensDev/CitizensAPI/blob/e772f57f83ca16d2e771613cd05026de6728eafd/src/main/java/net/citizensnpcs/api/util/ItemStorage.java#L356-L358

EDIT: Apparently loading the legacy handler really does take a few seconds at startup, which... wtf, spigot.

Artuto commented 3 years ago

I assume it takes a while because it has to load all the conversions. In the exception I sent it took about 7 seconds

mcmonkey4eva commented 3 years ago

(That loading should take a matter of milliseconds at worst ... Spigot did something weird)

This should be fixed by https://github.com/CitizensDev/CitizensAPI/commit/5658d91266012aa491af59cb2942005090fa6d7e https://github.com/CitizensDev/CitizensAPI/commit/2676e358021f81c9308606be926e0df71f826d17 for build 2268+ from https://ci.citizensnpcs.co/job/Citizens2/

Artuto commented 3 years ago

After updating to Citizens version 2.0.27-SNAPSHOT (build 2268) I'm afraid it is still happening:

[22:23:57 WARN]: Initializing Legacy Material Support. Unless you have legacy plugins and/or data this is a bug!
[22:23:57 WARN]: java.lang.Exception
[22:23:57 WARN]:        at org.bukkit.craftbukkit.v1_16_R3.legacy.CraftLegacy.<clinit>(CraftLegacy.java:259)
[22:23:57 WARN]:        at org.bukkit.craftbukkit.v1_16_R3.util.CraftMagicNumbers.toLegacy(CraftMagicNumbers.java:192)
[22:23:57 WARN]:        at org.bukkit.inventory.ItemStack.getData(ItemStack.java:176)
[22:23:57 WARN]:        at net.citizensnpcs.api.util.ItemStorage.loadItemStack(ItemStorage.java:326)
[22:23:57 WARN]:        at net.citizensnpcs.api.trait.trait.Inventory.parseContents(Inventory.java:127)
[22:23:57 WARN]:        at net.citizensnpcs.api.trait.trait.Inventory.load(Inventory.java:81)
[22:23:57 WARN]:        at net.citizensnpcs.api.npc.AbstractNPC.loadTrait(AbstractNPC.java:328)
[22:23:57 WARN]:        at net.citizensnpcs.api.npc.AbstractNPC.load(AbstractNPC.java:322)
[22:23:57 WARN]:        at net.citizensnpcs.npc.CitizensNPC.load(CitizensNPC.java:157)
[22:23:57 WARN]:        at net.citizensnpcs.api.npc.SimpleNPCDataStore.loadInto(SimpleNPCDataStore.java:59)
[22:23:57 WARN]:        at net.citizensnpcs.Citizens$CitizensLoadTask.run(Citizens.java:501)
[22:23:57 WARN]:        at org.bukkit.craftbukkit.v1_16_R3.scheduler.CraftTask.run(CraftTask.java:100)
[22:23:57 WARN]:        at org.bukkit.craftbukkit.v1_16_R3.scheduler.CraftScheduler.mainThreadHeartbeat(CraftScheduler.java:468)
[22:23:57 WARN]:        at net.minecraft.server.v1_16_R3.MinecraftServer.w(MinecraftServer.java:947)
[22:23:57 WARN]:        at net.minecraft.server.v1_16_R3.MinecraftServer.lambda$a$0(MinecraftServer.java:175)
[22:23:57 WARN]:        at java.base/java.lang.Thread.run(Thread.java:834
mcmonkey4eva commented 3 years ago

That might just be old data still present - ie at next restart it won't appear again