Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
MorkaZ opened this issue Mar 5, 2020 · 6 comments
Closed
Labels
enhancement New feature or request

Comments

@MorkaZ
Copy link

MorkaZ commented Mar 5, 2020

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
Copy link
Author

MorkaZ commented Mar 5, 2020

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
Copy link
Member

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
Copy link
Author

MorkaZ commented Mar 6, 2020

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
Copy link
Member

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!

tastybento added a commit to BentoBoxWorld/BentoBox that referenced this issue Mar 7, 2020
Relates to BentoBoxWorld/CaveBlock#44
Added a test case to benchmark search algorithms.
@MorkaZ
Copy link
Author

MorkaZ commented Mar 7, 2020

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 tastybento added the enhancement New feature or request label Mar 7, 2020
@tastybento
Copy link
Member

Fixed by latest snapshot of BentoBox BentoBoxWorld/BentoBox#1589

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants