Observed Behavior

Two different scenarios triggers this issue:

• When having an area with a lot of separate claims and you spam shift+rightclick the ground with a stick or holding down shift+rightclick (assuming you have the right permission to do so)
• When having a claim with a lot of subclaims, and you spam rightclick the ground with a stick or holding down rightclick. (Requires no extra permissions other than creating claims).

With "a lot" of claims/subclaims I mean at least 200+ for the "full effect",
In both of these scenarios the server MSPT starts raising and the TPS drops after a little while.

I run a public smp with a shopping area that exists of one big top level claim, with more than 300 sub claims. Any player can hold down right click with a stick in that area and start lagging the server.

Here is a quick video showing this in action, in a test claim I made with maybe around 150 subclaims. It's not as bad as with 300+ of course, but with the added TabTPS plugin it's clearly visible in the bossbar what is happening to the MSPT while clicking or holding down right click with a stick.
https://www.youtube.com/watch?v=eFa45dlXjIY

Expected Behavior

Not sure what I'd expect, other than not making the server lag.
Suggested solutions other than re-writing some code to remove the lag(I have no idea what's possible) could be to maybe add some kind of cooldown to claim visualizations.
Or adding a separate permission required to visualize subclaim burders, that would at least limit the amount of players that can do this.

Reproduction steps

1. Create a claim
2. Create 200+ subclaims inside that claim
3. Grab a stick and hold down right click inside the claim
4. Watch the MSPT increase by a lot

Stack trace or error log

No response

Server version

03.01 14:56:56 [Server] INFO This server is running Paper version git-Paper-366 (MC: 1.19.3) (Implementing API version 1.19.3-R0.1-SNAPSHOT) (Git: 8d7d927)

GriefPrevention version

03.01 14:58:28 [Server] INFO GriefPrevention version da68020

Configuration

# Default values are perfect for most servers.  If you want to customize and have a question, look for the answer here first: http://dev.bukkit.org/bukkit-plugins/grief-prevention/pages/setup-and-configuration/

GriefPrevention:
  SeaLevelOverrides:
    Simplex: -1
    Simplex_nether: -1
    Simplex_the_end: -1
  Claims:
    Mode:
      Simplex: Survival
      Simplex_the_end: Survival
      Simplex_nether: Survival
    PreventGlobalMonsterEggs: true
    PreventTheft: true
    ProtectCreatures: true
    PreventButtonsSwitches: true
    LockWoodenDoors: true
    LockTrapDoors: true
    LockFenceGates: true
    EnderPearlsRequireAccessTrust: true
    RaidTriggersRequireBuildTrust: true
    ProtectHorses: true
    ProtectDonkeys: true
    ProtectLlamas: true
    InitialBlocks: 225
    Claim Blocks Accrued Per Hour:
      Default: 150
    Max Accrued Claim Blocks:
      Default: 300000
    Accrued Idle Threshold: 0
    AccruedIdlePercent: 0
    AbandonReturnRatio: 1.0
    AutomaticNewPlayerClaimsRadius: 4
    AutomaticNewPlayerClaimsRadiusMinimum: 0
    ExtendIntoGroundDistance: 20
    MinimumWidth: 1
    MinimumArea: 10
    MaximumDepth: -64
    InvestigationTool: STICK
    ModificationTool: GOLDEN_SHOVEL
    Expiration:
      ChestClaimDays: 9999999
      UnusedClaimDays: 9999999
      AllClaims:
        DaysInactive: 9999999
        ExceptWhenOwnerHasTotalClaimBlocks: 10000
        ExceptWhenOwnerHasBonusClaimBlocks: 5000
      AutomaticNatureRestoration:
        SurvivalWorlds: false
    AllowTrappedInAdminClaims: false
    MaximumNumberOfClaimsPerPlayer: 0
    CreationRequiresWorldGuardBuildPermission: true
    VillagerTradingRequiresPermission: true
    CommandsRequiringAccessTrust: /sethome;/esethome;/createhome;/ecreatehome
    DeliverManuals: true
    ManualDeliveryDelaySeconds: 30
    RavagersBreakBlocks: true
    FireSpreadsInClaims: false
    FireDamagesInClaims: false
    LecternReadingRequiresAccessTrust: true
  Spam:
    Enabled: true
    LoginCooldownSeconds: 60
    LoginLogoutNotificationsPerMinute: 5
    ChatSlashCommands: /me;/global;/local;/minecraft:me;/tpa;/suicide
    WhisperSlashCommands: ''
    WarningMessage: Please reduce your noise level.  Spammers will be muted.
    BanOffenders: false
    BanMessage: Banned for spam.
    AllowedIpAddresses: 1.2.3.4; 5.6.7.8
    DeathMessageCooldownSeconds: 60
    Logout Message Delay In Seconds: 0
  PvP:
    RulesEnabledInWorld:
      Simplex: true
      Simplex_nether: true
      Simplex_the_end: true
    ProtectFreshSpawns: true
    PunishLogout: false
    CombatTimeoutSeconds: 15
    AllowCombatItemDrop: true
    BlockedSlashCommands: /home;/vanish;/spawn;/tpa
    ProtectPlayersInLandClaims:
      PlayerOwnedClaims: false
      AdministrativeClaims: false
      AdministrativeSubdivisions: false
    AllowLavaDumpingNearOtherPlayers:
      PvPWorlds: false
      NonPvPWorlds: false
    AllowFlintAndSteelNearOtherPlayers:
      PvPWorlds: false
      NonPvPWorlds: false
    ProtectPetsOutsideLandClaims: true
  Economy:
    ClaimBlocksMaxBonus: 0
    ClaimBlocksPurchaseCost: 0.0
    ClaimBlocksSellValue: 0.0
  ProtectItemsDroppedOnDeath:
    PvPWorlds: true
    NonPvPWorlds: true
  BlockLandClaimExplosions: true
  BlockSurfaceCreeperExplosions: false
  BlockSurfaceOtherExplosions: false
  LimitSkyTrees: false
  LimitTreeGrowth: false
  PistonMovement: EVERYWHERE
  PistonExplosionSound: true
  FireSpreads: true
  FireDestroys: true
  AdminsGetWhispers: false
  AdminsGetSignNotifications: true
  VisualizationAntiCheatCompatMode: false
  SmartBan: true
  Mute New Players Using Banned Words: true
  MaxPlayersPerIpAddress: 5
  SilenceBans: true
  Siege:
    Worlds: []
    BreakableBlocks:
    - DIRT
    - GRASS
    - LONG_GRASS     <-- can't understand this entry, see BukkitDev documentation
    - COBBLESTONE
    - GRAVEL
    - SAND
    - GLASS
    - THIN_GLASS     <-- can't understand this entry, see BukkitDev documentation
    - WOOD     <-- can't understand this entry, see BukkitDev documentation
    - WOOL     <-- can't understand this entry, see BukkitDev documentation
    - SNOW
    DoorsOpenDelayInSeconds: 300
    CooldownEndInMinutes: 60
  EndermenMoveBlocks: true
  SilverfishBreakBlocks: true
  CreaturesTrampleCrops: true
  RabbitsEatCrops: true
  HardModeZombiesBreakDoors: true
  Database:
    URL: ''
    UserName: ''
    Password: ''
  UseBanCommand: false
  BanCommandPattern: ban %name% %reason%
  Advanced:
    fixNegativeClaimblockAmounts: true
    ClaimExpirationCheckRate: 60
    OfflinePlayer_cache_days: 90
  Abridged Logs:
    Days To Keep: 7
    Included Entry Types:
      Social Activity: true
      Suspicious Activity: false
      Administrative Activity: true
      Debug: false
      Muted Chat Messages: true
  ConfigVersion: 1

Plugin list

03.01 15:00:39 [Server] INFO Plugins (26): AntiXRay, AutoMessage*, Chunky, CommandHook, EntityWatch*, Essentials, EssentialsChat, EssentialsSpawn, F3NPerm, GriefPrevention, HolographicDisplays, InteractiveBooks, InventoryRollback-Continued, LogBlock, LuckPerms, Matrix, NBTAPI, OpenInv, PlugMan, ProtocolLib, Reports*, Simplex, TabTPS, Vault, WorldEdit, WorldGuard

Running without GriefPrevention

• I attempted running the server without GriefPrevention installed.
• The problem does not occur when GriefPrevention is removed from the server.

Running with only GriefPrevention

• I attempted running only GriefPrevention on the server.
• The issue still occurs when GriefPrevention is the only plugin running.

Running on a fresh, clean server installation

• I attempted testing for the issue on a new server.
• The issue still occurs on a new server.

Using unmodified client

• I attempted testing for the issue with the vanilla client.
• The issue still occurs when using the vanilla client.

We appreciate you taking the time to fill out a bug report!

• I searched for similar issues before submitting this bug report.

Maybe snurre0 can try each of these pull requests separately and see if they solve his problem.

For my PR, he would have to set VisualizeAllSubdivisions to false.

Easiest solution is to modify BoundaryVisualization#callAndVisualize to ignore duplicate visualizations (possibly post-event call so addons modifying the boundary collection don't cause duplicate re-renders?). The stick should also have a cooldown, though it should be relatively short (1s?) to prevent too much nuisance when misclicking the wrong claim. Shift-click should have a longer cooldown to prevent abuse.

Would also be interested in seeing how this compares to GPBHB's drawing strategy which doesn't do world-based calculation, though that would be a bandaid at best.

I can probably write a patch to not re-render duplicates on GitHub, but as my PC is currently experiencing a joyous time, I don't feel like writing a cooldown system for the stick.

I finally got around to test these, thanks for looking into this!

#1987 doesn't really feel any different (running version c6fb38b, just to verify what version I'm testing).
Maybe the visualizations is slightly slower than before, but it's hard to tell. Either way, I can hold down right click just fine and still lag the server.

#1991 works as expected (version e8a1216), with VisualizeAllSubdivisions set to false I can only visualize one subclaim at a time with a regular player without any of the specified permissions.

#1991 works as a solution/workaround for our situation I guess, as staff members with one of the mentioned permissions should know better :)
I'm interested to know what #1987 is supposed to do though, sounds like it was not supposed to re-render boundaries that is already rendered?

I'm interested to know what #1987 is supposed to do though, sounds like it was not supposed to re-render boundaries that is already rendered?

Yes. The boundary should not be re-drawn if it matches the one already displayed - GP's default draw system iterates over a ton of blocks to find exposed ground blocks. The equality check must be failing, not sure why without a deeper dive into the code though. That's also why I suggested seeing how GPBHB performs; it doesn't bother doing world checks when using FLAT visualization style.

#1991 is a good start to mitigating some of the issue and should be compatible with changes required by mine, but it doesn't stop careless admins or knowledgeable malicious users. Mine will (when working) stop careless admins, but the only way to truly prevent a malicious user taking advantage of high-claim renders is to add a cooldown to the stick.

For clarity, because based on the now-deleted comment I received an email about this morning I probably offended @Bobcat00 with poor wording in my previous post, the primary issue I see with #1991 is that claim owners still see subclaims rendered. That's necessary, claim owners need to see their subclaims to be able to manage them. Unfortunately, that also means that a sufficiently determined malicious user could make their own claim that would cause these issues. GP should not be delivering tools to players that can be used to grief the entire server, no matter how niche.
I still think that PR is good and should be pulled. I just think that it (and my own small work, if it actually worked) are not a sufficient solution to consider this fully addressed.

No, no offense taken. I hadn't considered that a malicious person could be the one creating all the subclaims! I realized that after I posted my comment, so I decided to withdraw it.

I think this nails it:

but the only way to truly prevent a malicious user taking advantage of high-claim renders is to add a cooldown to the stick.

Anything we can do to make the visualisation more efficient is good, but avoiding it being triggered too often has to be the proper fix.

Yeah I had a similar "bruh" moment adding my own glowing-falling-block visualizer.

The elements are redrawn each time a boundary is added to the visualization by the "draw" method in BlockBoundaryVisualization.class

See the below PR for a solution.

I've fixed #1987, so that should be another huge improvement here. Kinda reluctant to work on a cooldown system myself because there are other areas I'd like to be able to apply cooldowns (the various "XYZ message is spammy" reports and whatnot) and I know I'd add feature creep and burn out before I started.