search

Brettflan / WorldBorder

Bukkit plugin for maintaining borders for your worlds to limit their sizes, as well as generating missing chunks or trimming excess chunks.
https://www.spigotmc.org/resources/worldborder.60905/
BSD 2-Clause "Simplified" License
139 stars 210 forks source link

Don't request chunk unloads during fill #173

Closed Puremin0rez closed 4 years ago

Puremin0rez commented 4 years ago

Requesting chunk unloads during the fill task takes a tremendous hit on the server TPS, which leads to a really bad experience while players are online. It also slows down the fill process overall. By removing the chunk unload request, the server TPS will stay at a near 20 TPS due to the server now unloading the chunks naturally.

This leads to no noticeable downsides in my personal testing as well as many others personal testing over at the PaperMC discord. This will simply allow the server to handle unloading itself, which is seemingly much faster than a plugin requesting unloads.

I attempted to follow your comment style and commented out the line rather than deletion.

As always, feel free to do your own testing :)

clrxbl commented 4 years ago

Definitely has improved the speed & TPS, but can easily run out of memory with this patch.

Puremin0rez commented 4 years ago

Definitely has improved the speed & TPS, but can easily run out of memory with this patch.

Just to verify - did you directly test against WorldBorder with and without this PR? It's easy to run out of memory with standard WorldBorder as well since I think there may be a memory leak somewhere (either within WB or MC itself - refer to the multiple open issues about memory).

While this change will lead to chunks being loaded slightly longer - the difference is like 1-2 seconds longer, and I don't really see how you could run out of memory since the chunks are still being unloaded - we're just not requesting their unload immediately.

Let me know what heap size you used and any other information that may be helpful so I can see if I can reproduce anything.

Brettflan commented 4 years ago

can easily run out of memory with this patch.

This is of course the reason WorldBorder requests the server to unload chunks that no longer need to be loaded after generation. Way back when this plugin was initially written, this was absolutely needed to keep from quickly running out of memory and effectively stalling the server due to the server just keeping all of the chunks loaded. This was looked into again briefly a couple of years ago and found to still be just as necessary.

@Puremin0rez It's possible that Spigot/Paper somehow better decides to go ahead and unload chunks on its own during the process now, but I'd frankly be surprised if it kept up well enough for this change to be preferable for most servers. And if it really is working better in your experience for large Fill tasks, is that with Paper only, or Spigot as well? Also, how much memory do you have allocated to Paper/Spigot?

Puremin0rez commented 4 years ago

I can completely understand why the force unloads were added - Minecraft used to leak chunks like crazy and forcing it to unload them was essentially required.

MC 1.14 changed the chunk system quite significantly and everything uses tickets now (as you are aware), and it appears that MC is good at unloading them all or at least doing the chunk cleanup by itself.

The actual problem this PR is trying to solve is that with the unload requests happening, it's forcing the chunk unloads sync (right now, right this moment) rather than vanilla doing it. This causes a significant TPS drop while a fill is running, essentially making filling on a live server not easily doable. I have no idea why the chunk unload request is so heavy - because you'd think it'd be fine, but it's clearly doing something that's quite intensive.

Majority of my testing was done on Spigot (as it's the upstream / most popular) but results on Paper were pretty much the same. All testing was done with 2GB (what I consider minimum), 5GB (average server?) and 10GB (larger server) heaps. I observed memory usage and chunks that remained loaded and while yes, chunks WILL remain loaded longer than if the chunks were not being forcibly unloaded, it was never more than 2 seconds at most in my testing in my personal environment. This might change based on storage speed, cpu speed, etc. It will also lead to an increase in memory as it's possible for chunks to remain loaded for slightly longer - but the memory increase wasn't even that significant even in my 2GB heap testing. I did not use memory dump analyzers or anything crazy like that.

The main takeaway for me was that there were never any leaked chunks / chunks that remained loaded even after a huge fill (I did 50k x 50k which took nearly a week).

I encourage you to to do your own testing when you get a chance and see what you think, as it's really good to have this tested in different environments. If you don't believe this to be preferred for all setups - it could always be thrown behind a config option for severe memory limited cases.

Thanks for reading!

partydev commented 4 years ago

Am doing testing on this currently, 10gb of RAM allocated.

It appears to be about the same speed as regular WorldBorder. However, there is no loss of speed over time present in this PR as there is in the regular WorldBorder plugin. I assume this is due to the unload function no longer being called (since that was the only change.

After about five minutes: image Typically there is already a loss of speed noticed at this point.

TPS during the fill process: image

Clearly this pull request solves the issue upon initial testing. I am currently generated a 10k x 10k world and will report back with my findings.

EDIT1:

No crashes or OOM issues. No TPS loss at all during the fill process. Recommend.

EDIT2:

Tested in a low RAM environment, 2GB allocated. Results: image

Same conclusions after about 15 minutes of running the fill task. 0 TPS drop, no OOM/crash, no slowdown.

EDIT3:

After about 35 minutes. image

So this can definitely cause issues on low RAM environments. Maybe add this as an option for people with enough memory?

compieter-gh commented 4 years ago

i like this change it no longer indeed causes tps lagg on servers with more ram

edwerd commented 4 years ago

Does anyone have a build of this I can use? I'm having the same issue on my Minecraft server.

compieter-gh commented 4 years ago

@edwerd https://github.com/TheCompieter/WorldBorder/releases/tag/v1.10.1

TheoRetisch1 commented 4 years ago

@Puremin0rez will you update your awesome fork to version 1.16.1? :) He has been my faithful companion for the last months, thanks! Or does everybody use Chunkmaster these days?

Puremin0rez commented 4 years ago

@Puremin0rez will you update your awesome fork to version 1.16.1? :) He has been my faithful companion for the last months, thanks! Or does everybody use Chunkmaster these days?

No update is needed, everything remains completely functional on 1.16.

It seems many people use Chunkmaster, but I personally find it to be slower even when configured to be "fast"... so... I'll keep using the true and tested WorldBorder.

I will probably do an actual proper fork and maintain it going forward though (and provide downloads, etc) since I see quite a few people sharing "updated" WorldBorders that don't even have this PR in them, which is why people prefer ChunkMaster since it's quite a bit faster than the vanilla, standard WorldBorder.

docwhomc commented 4 years ago

Time to get everyone with a WorldBorder fork together to unify and officially maintain the project suppose...

docwhomc commented 4 years ago

@Puremin0rez do you have a discord?

docwhomc commented 4 years ago

Well, if you do have one, I set up a WorldBorder Reboot Discord, which you can join here.

Puremin0rez commented 4 years ago

I will be maintaining my own fork of WorldBorder going forward.

Thank you to the wonderful Brettflan for all of his years of hard work and dedication to the science behind world gen and creating a valuable tool for everyone.

You may find it here: https://github.com/Puremin0rez/WorldBorder