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 Developer

    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

    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 Developer

    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.
  5. malon

    malon Member

    Is there any sort of way to over-ride default commandhelper functions? Like "re-write" the broadcast function? I've used broadcast() in a ton of places.

    I googled "PermissionSubscriptions" and searched github for "PermissionSubscriptions" - both with zero results. What should I actually be searching for? I'm interested in maybe trying to submit a pull request for a fix, but I'm not sure I understand where to start.

    When are permissions recalcluated? /pex reload? /pex add? I'm not quite sure I grasp what this is all about.
  6. PseudoKnight

    PseudoKnight Well-Known Member Developer

    You could include this in your, and add whatever features you need to it, like console output. Then just search and replace "broadcast(" with "_broadcast(".
    proc _broadcast(@msg) {
      foreach(@p in all_players()) {
        tmsg(@p, @msg);
    But again, the problem isn't the function. It's the Bukkit method. Any plugin that uses it will have this problem if unaddressed at its source.
  7. PseudoKnight

    PseudoKnight Well-Known Member Developer

    So, in PermissibleBase.recalculatePermissions() you can see it call subscribeToPermission(). And in clearPermissions() right below that, it calls unsubscribeFromPermission(). You can see those methods here in SimplePluginManager. And here in Server.broadcastMessage() we see how it loops through all permissibles from the permission subscriptions.
  8. PseudoKnight

    PseudoKnight Well-Known Member Developer

    Well, PEX looks like it overrides this in its Permissibles and has its own PermissionSubscriptionMap for it. From what I can tell, it might not even apply to Citizens! But looking at more code in Craftbukkit, it looks like any time a Permissible is even created it recalculates permissions.