TPS Lag caused by me.ryanhamshire.GriefPrevention.DataStore.getClaim()

Question

TPS Lag caused by me.ryanhamshire.GriefPrevention.DataStore.getClaim()

DarkChromaMC opened this issue 4 months ago · 3 comments

DarkChromaMC commented 4 months ago

Observed Behavior

We are experiencing significant server lag (8%) of tick at higher player counts. It seems to specifically come from me.ryanhamshire.GriefPrevention.DataStore.getClaim() method.
https://spark.lucko.me/0oIUisWjFY

Expected Behavior

The plugin does not lag.

Reproduction steps

Start the server with GriefPrevention installed.
Increase the player count to 80+
Paste spark profile report
Observe the performance impact related to the me.ryanhamshire.GriefPrevention.DataStore.getClaim() method.
https://spark.lucko.me/0oIUisWjFY

Stack trace or error log

No response

Server version

1.21.1

GriefPrevention version

16.18.4

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:
    world: -1
    world_nether: -1
    world_the_end: -1
    haunted_dungeon: -1
    MazeGame: -1
    Egypt: -1
    catgods_mythology: -1
    pvp: -1
    netherhub: -1
    spawn: -1
    pirateisland: -1
    minigames: -1
    ender_arena: -1
    Void: -1
    resource_world: -1
    CATCRAFTMARKET: -1
  Claims:
    Mode:
      spawn: Survival
      world: Survival
      pvp: Survival
      CATCRAFTMARKET: Disabled
      pirateisland: Survival
      minigames: Disabled
      resource_world: Survival
      catgods_mythology: Survival
      world_nether: Survival
      Egypt: Survival
      Void: Disabled
      haunted_dungeon: Survival
      world_the_end: Survival
      netherhub: Disabled
      ender_arena: Survival
      MazeGame: Disabled
    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: 1000
    Claim Blocks Accrued Per Hour:
      Default: 200
    Max Accrued Claim Blocks:
      Default: 80000
    Accrued Idle Threshold: 0
    AccruedIdlePercent: 0
    AbandonReturnRatio: 1.0
    AutomaticNewPlayerClaimsRadius: 4
    AutomaticNewPlayerClaimsRadiusMinimum: 0
    ExtendIntoGroundDistance: 64
    MinimumWidth: 5
    MinimumArea: 100
    MaximumDepth: -64
    InvestigationTool: STICK
    ModificationTool: GOLDEN_SHOVEL
    Expiration:
      ChestClaimDays: 7
      UnusedClaimDays: 30
      AllClaims:
        DaysInactive: 60
        ExceptWhenOwnerHasTotalClaimBlocks: 50
        ExceptWhenOwnerHasBonusClaimBlocks: 50
      AutomaticNatureRestoration:
        SurvivalWorlds: false
    AllowTrappedInAdminClaims: false
    MaximumNumberOfClaimsPerPlayer: 0
    CreationRequiresWorldGuardBuildPermission: true
    VillagerTradingRequiresPermission: true
    CommandsRequiringAccessTrust: /sethome
    DeliverManuals: true
    ManualDeliveryDelaySeconds: 30
    RavagersBreakBlocks: true
    FireSpreadsInClaims: false
    FireDamagesInClaims: false
    LecternReadingRequiresAccessTrust: true
  Spam:
    Enabled: false
    LoginCooldownSeconds: 20
    LoginLogoutNotificationsPerMinute: 5
    ChatSlashCommands: /me;/global;/local
    WhisperSlashCommands: /tell;/pm;/r;/whisper;/msg
    WarningMessage: Please reduce your noise level.  Spammers will be banned.
    BanOffenders: false
    BanMessage: Banned for spam.
    AllowedIpAddresses: 1.2.3.4; 5.6.7.8
    DeathMessageCooldownSeconds: 120
    Logout Message Delay In Seconds: 0
  PvP:
    RulesEnabledInWorld:
      world: false
      world_nether: false
      world_the_end: false
      haunted_dungeon: true
      MazeGame: true
      Egypt: true
      catgods_mythology: true
      pvp: true
      netherhub: true
      spawn: true
      pirateisland: true
      minigames: true
      ender_arena: true
      Void: true
      resource_world: true
      CATCRAFTMARKET: false
    ProtectFreshSpawns: true
    PunishLogout: false
    CombatTimeoutSeconds: 10
    AllowCombatItemDrop: false
    BlockedSlashCommands: /home;/vanish;/spawn;/tpa
    ProtectPlayersInLandClaims:
      PlayerOwnedClaims: true
      AdministrativeClaims: true
      AdministrativeSubdivisions: true
    AllowLavaDumpingNearOtherPlayers:
      PvPWorlds: false
      NonPvPWorlds: false
    AllowFlintAndSteelNearOtherPlayers:
      PvPWorlds: false
      NonPvPWorlds: false
    ProtectPetsOutsideLandClaims: false
  Economy:
    ClaimBlocksMaxBonus: 0
    ClaimBlocksPurchaseCost: 0.0
    ClaimBlocksSellValue: 0.0
  ProtectItemsDroppedOnDeath:
    PvPWorlds: true
    NonPvPWorlds: true
  BlockLandClaimExplosions: true
  BlockSurfaceCreeperExplosions: true
  BlockSurfaceOtherExplosions: true
  LimitSkyTrees: true
  LimitTreeGrowth: false
  PistonMovement: EVERYWHERE_SIMPLE
  PistonExplosionSound: true
  FireSpreads: false
  FireDestroys: false
  AdminsGetWhispers: true
  AdminsGetSignNotifications: true
  VisualizationAntiCheatCompatMode: false
  SmartBan: true
  Mute New Players Using Banned Words: false
  MaxPlayersPerIpAddress: 15
  SilenceBans: true
  Siege:
    Worlds: []
    BreakableBlocks:
    - DIRT
    - GRASS_BLOCK
    - GRASS     <-- can't understand this entry, see BukkitDev documentation
    - FERN
    - DEAD_BUSH
    - COBBLESTONE
    - GRAVEL
    - SAND
    - GLASS
    - GLASS_PANE
    - OAK_PLANKS
    - SPRUCE_PLANKS
    - BIRCH_PLANKS
    - JUNGLE_PLANKS
    - ACACIA_PLANKS
    - DARK_OAK_PLANKS
    - WHITE_WOOL
    - ORANGE_WOOL
    - MAGENTA_WOOL
    - LIGHT_BLUE_WOOL
    - YELLOW_WOOL
    - LIME_WOOL
    - PINK_WOOL
    - GRAY_WOOL
    - LIGHT_GRAY_WOOL
    - CYAN_WOOL
    - PURPLE_WOOL
    - BLUE_WOOL
    - BROWN_WOOL
    - GREEN_WOOL
    - RED_WOOL
    - BLACK_WOOL
    - SNOW
    DoorsOpenDelayInSeconds: 300
    CooldownEndInMinutes: 60
  EndermenMoveBlocks: false
  SilverfishBreakBlocks: false
  CreaturesTrampleCrops: false
  RabbitsEatCrops: true
  HardModeZombiesBreakDoors: false
  MobProjectilesChangeBlocks: false
  Database:
    URL: ''
    UserName: ''
    Password: ''
  UseBanCommand: false
  BanCommandPattern: ban %name% %reason%
  Advanced:
    fixNegativeClaimblockAmounts: true
    ClaimExpirationCheckRate: 60
    OfflinePlayer_cache_days: 360
  Abridged Logs:
    Days To Keep: 7
    Included Entry Types:
      Social Activity: true
      Suspicious Activity: true
      Administrative Activity: false
      Debug: false
      Muted Chat Messages: false
  ConfigVersion: 1

Plugin list

No response

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.

Jikoo · Answer 1 · 2024-08-31T23:15:25.000Z

The plugin BanFromClaim appears to be checking if the player is in a claim from which they have been banned in a roundabout way. Contact the author and ask them to please use DataStore#getClaimAt instead of DataStore#getClaim.

This is an area where GP can do better (it's not only this addon that has had issues with getting claims by ID not being performant, see #2249) but it isn't GP making the calls that are causing your server to lag.

DarkChromaMC · Answer 2 · 2024-09-01T07:33:26.000Z

Thank you very much for the valuable insight @Jikoo

RoboMWM · Answer 3 · 2024-09-01T22:07:46.000Z

[04:17:14] +Dumcord: <baktus_79> Hey, quick question. Which is more efficient? DataStore#getClaimAt or DataStore#getClaim (edited)
[04:18:34] +Dumcord: <baktus_79> It just seems like DataStore#getClaim iterates through an arraylist and DataStore#getClaimAt has a concurrentHashMap for its storage?
[06:25:28] +Dumcord: <evident._.> #2361 (comment) kind of weird that was just talked about in an issue
[09:13:17] +Dumcord: <baktus_79> haha, I am the author of BanFromClaim. I cant see how DataStore#getClaimAt should be logically more efficient.
[09:15:45] +Dumcord: <baktus_79> They should go through the same list I guess? Doesnt DataStore#getClaimAt have a few more steps to get the claim?
[09:27:47] +Dumcord: <robomwm> Hashmap is faster than iterating an array
[10:23:24] @RoboMWM: getclaimat gets only a subset of possible claims (via hashmap call, O(1)) in the chunk of the provided location, rather than iterating the entire list of claims (via iteration, O(n))
[10:24:03] @RoboMWM: it may seem like more steps, but it's far faster and can be seen as such via big O notation
[10:25:19] @RoboMWM: if we sorted the arraylist, we could also make it an O(1) lookup (would have to account for deleted claims tho), but that would be moot since we do plan to change how claims are identified
[10:25:39] @RoboMWM: we could also map claims to id too, but I don't often see the use case for that.
[10:25:53] @RoboMWM: or rather see any use case for that
[13:37:54] +Dumcord: <baktus_79> Thanks for the answer, RoboMWM. I didnt know they operated different when obtaining the claim. I always thought that by getting it by providing the claim id would be way faster. But again, thanks for the insight 🙂

Share to