BentoBoxWorld / CaveBlock

CaveBlock addon for BentoBox. An interesting variation from Skyblock, where your goal is to survive in a cube of stone!
Eclipse Public License 2.0
16 stars 6 forks source link

"/cave reset" executed few times consumes a lot of server performance #44

Closed MorkaZ closed 3 years ago

MorkaZ commented 4 years ago

Description

Describe the bug

When player execute /cave reset few times (limit is 4) or do it with friend and they will launch few reseting tasks, server will lose a lot of performance. From 20 TPS to ~8-10 on GAME-3 OVH dedicated server. I have latest paper spigot (after chunk generating performance drop fix).

Steps to reproduce the behavior

  1. Execute /cave reset few times.
  2. Monitor server's TPS.

Expected behavior

Basicly - optimalization. Here are few of my ideas:

  1. Create queue for chunks over all tasks. Do not execute few generating tasks at once when players will reset their caves.
  2. Try to do it asynchronously like FAWE did/is doing (I do not know if this plugin is still alive).
  3. Add option to disable regeneration of abandoned caves. Players in this case will just get next free cave at next coordinates. I think the 2. and even 3. will be enough. 1. will still generate performance drops but in way lesser scale.

Screenshots and videos (Optional)

Environment

Latest Paper spigot 1.15.2 & Bentobox & CaveBlock GAME-3 OVH Dedicated server Java 8 12GB RAM allocated to paper spigot.

BentoBox Version (Mandatory)
1.11.1

Plugins (Optional)

[07:40:48 INFO]: Plugins (37): AuctionHouse, BentoBox, BossShopPro, ChatInjector*, ColoredSigns*, CommandButtons, CraftBook, Essentials, EssentialsChat, EssentialsProtect, EssentialsSpawn, HeadDatabase*, HolographicDisplays, ItemShops*, MorkazSk, MoxChatTitles, MoxCore, MoxPerms*, MoxPremiumShop, MoxTokensDatabase*, MoxTransmutators, Multiverse-Core*, PlaceholderAPI, PlugMan, ProtocolLib, ProtocolSupport, SK-NBeeT, SkQuery, skRayFall*, Skript, SQLibrary*, TAB, TuSKe*, Vault, WorldBorderAPI*, WorldEdit, WorldGuard

Additional context (Optional)

MorkaZ commented 4 years ago

I have noticed that setting delete-speed to 0 makes cave unclaimable by auto claim after reset but if bentobox is getting next cave coords in "snail" way then after year on bigger server I think getting next cave may lagg a lot.

tastybento commented 4 years ago

Yes, resetting causes a lot of CPU because it is doing a lot of things. First, you should use a cooldown period so that resetting cannot be done many times quickly.

In regards to 1 and 2, we do try to do everything async and pipelined. We provide settings for island deletion (that you found) and paste speed. However, inevitably, if you request a lot of island pasting (remember islands are actually 3 islands - world, nether, end) then it will take a lot of server resources.

I have noticed that setting delete-speed to 0 makes cave unclaimable by auto claim after reset but if bentobox is getting next cave coords in "snail" way then after year on bigger server I think getting next cave may lagg a lot.

Nope. We store the location of islands in a database. The look up is very quick. Your world size on disk though will grow.

MorkaZ commented 4 years ago

Yes, resetting causes a lot of CPU because it is doing a lot of things. First, you should use a cooldown period so that resetting cannot be done many times quickly.

What if some players will execute /cave reset in same moment? I think reseting tasks should be queued instead of running them all in command executing time.

Here is simple runnable scheduler. I suggest you to implement something like this to your code and use it with island resets to significantly improve plugin's performance. SimpleRunnableScheduler.java

Nope. We store the location of islands in a database. The look up is very quick. Your world size on disk though will grow.

How bentobox is calculating next cave position? What will happen if there will be ~100.000 caves and someone who does not have cave will execute /cave?

tastybento commented 4 years ago

What if some players will execute /cave reset in same moment? I think reseting tasks should be queued instead of running them all in command executing time.

Here is simple runnable scheduler. I suggest you to implement something like this to your code and use it with island resets to significantly improve plugin's performance. SimpleRunnableScheduler.java

Thanks for the tip. It's a lot more complex than just staggering one Bukkit task though. The reset process involves multiple stages of sequential operation, which are staggered, including pasting of islands and regeneration of chunks. Those could take minutes to run so serializing them all between players could give an interesting user experience. Anyway, thanks for the tip.

How bentobox is calculating next cave position? What will happen if there will be ~100.000 caves and someone who does not have cave will execute /cave?

All the cave locations are on a grid and the currently used locations are stored in a 2D array in memory. There's a one-time search, usually after boot up to find the first free spot. Worst case, it could be the 100,001 spot, but it's usually less than that because there are deleted island spots from the last time. After that, the last position is saved. So all subsequent spots are immediate.

It looks like you are a programmer - we're open to PRs. We can't cover every angle so your submission would be welcome!

MorkaZ commented 4 years ago

Thanks for the tip. It's a lot more complex than just staggering one Bukkit task though. The reset process involves multiple stages of sequential operation, which are staggered, including pasting of islands and regeneration of chunks. Those could take minutes to run so serializing them all between players could give an interesting user experience. Anyway, thanks for the tip.

I am really hopping it will be added because regenerating tasks in caveblock breaks server's tps a lot. Due to this problem, cave regeneration can not be used on any bigger server.
What about reseting/purger command? It would be nice to use it at night-time to clear some empty islands automatically.

tastybento commented 3 years ago

Fixed by latest snapshot of BentoBox BentoBoxWorld/BentoBox#1589