Lag from broadcast()

Discussion in 'CommandHelper' started by malon, May 14, 2017.

  1. malon

    malon Member

    The Issue:
    I've noticed that the longer my server runs, the more of a delay broadcast() has. Restarting the server solves this, but the broadcast delay still just slowly creeps back after another day or so.

    Why It Happens:
    I don't know for sure, but I believe I have a fair theory.
    I use Citizens NPCs and LibsDisguses very heavily on my server. Using some commandhelper wizardry, I spawn and despawn NPCs dynamically and frequently, as well as the players are VERY often changing their own player disguises.
    What I believe is happening is broadcast() is iterating over the fake players from either libsdisguises or NPCs (not sure which) and broadcasting them messages. After a couple days of running, I wouldn't be surprised if the dynamically created NPCs reach into the thousands.
    I need to be clear on this: The thousands of NPCs are never existing at the same time! This is not a case of "you have too many spawned NPCs" as the most NPCs that ever exist simultaneously is 6. The "dynamic" part really just involves spawning an NPC for just a few seconds then despawning it. I use it for "showcasing" skins for a brief few moments.

    What can I do to help narrow this issue down? I run a Linux server and when using htop I view the CPU usage and everything is normal until a broadcast() is used, then a huge lag spike occurs.

    Thank you.
  2. PseudoKnight

    PseudoKnight Well-Known Member

    First off, absolutely nothing fancy is going on in CommandHelper here. It simply sends the string to Bukkit to broadcast. So any plugin that uses Server.broadcastMessage() is going to have these same symptoms.

    So I dug into the Craftbukkit code. (i'll try not to get too technical) It's checking the permissions for every human entity on the server, regardless if you specify a permission or not in broadcast(). (it defaults to bukkit.broadcast.user) But that's not why it's getting worse over time. I dug into how the PermissionSubscriptions are populated and it seems that they're added whenever permissions are recalculated, and only removed when a player disconnects or if it's explicitly called. Citizens would probably have to do this manually when their NPCs unload. (unless they simply don't expect permissions to be recalculated for NPCs. This might be done by a permissions plugin.) I don't see any of these methods being used when I search for them in Citizens Github. So maybe that's why this is happening. Every time you spawn an NPC, it adds it to PermissionSubscriptions list and checks its permissions when broadcast() is run. But since it's never removed from this list (since NPCs can't "disconnect"), it keeps building up.
  3. malon

    malon Member

    Excellent response, I've submitted a bug report here
    https://github.com/CitizensDev/Citizens2/issues/1151

    Question:
    When I do "/say @a something" in a commandblock, does that use Server.broadcastMessage() ? I don't get lag with that. Same with "/tellraw @a {text:whatever,color:gray}" - no lag here either.
  4. PseudoKnight

    PseudoKnight Well-Known Member

    That just sends to everyone in the player list. (you can just loop through all_players()) Broadcast is designed to send to everyone and console. Same thing will happen with most Bukkit commands because they do the same thing to broadcast admin messages. Bukkit wasn't designed for players that aren't players. Citizens is bypassing Bukkit to make this happen, so it can cause unexpected behavior, like with how it triggers player_teleport events.

    If this is what's happening, then this is essentially a small memory leak. You'll want to restart once every day or two anyway. Avoid Bukkit commands and broadcasting until its fixed or they offer a recommendation to workaround this issue.