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

Support for Async Chunk Generation #106

Closed aikar closed 5 years ago

aikar commented 6 years ago

Hello,

Paper has an API addition that allows plugins to request chunks asynchronously. This API has existed since 1.9 roughly, however was limited to load only.

However, as of 1.13.1, I have Asynchronous Generation working too! You may track my progress here: https://achunks.emc.gs

I am requesting that WorldBorder add support for this by detecting if World#getChunkAtAsync exists, and if it does, use an alternative chunk loader that uses the Paper API method.

By doing this, Paper will be able to asynchronously generate all chunks. We also have a World#isGenerated(x,z) method that you may find useful too!

Many server owners are switching to Paper, and I'm hoping you can add this support so users who still wish to pregen, can do so with even better performance.

Thanks!

Brettflan commented 6 years ago

Not sure when I'll get a chance to look into implementing this, but: very interesting.

DoNotSpamPls commented 6 years ago

PaperLib may also be found useful - it removes the need to check if the server runs Paper entirely!

BaronyCraft commented 5 years ago

I implemented this in a fork (https://github.com/BaronyCraft/WorldBorder/blob/asyncFill/src/main/java/com/wimbli/WorldBorder/WorldFillTask.java), and while the results aren't that impressive, I get about 10% faster filling (with /wb fill 1000), and with less impact on players.

There's still just one chunk gen thread per world (this is in Paper, can't do anything about that), so the results are similar to synced chunk generation on an empty server and a sufficiently high rate per second. However. the fill task can consume 100% of that thread while not affecting players who don't need to generate chunks (unless I/O becomes a bottleneck), so we can safely tune up the rate per second. And we could probably run several fill tasks at once (world, nether, end, multiverse), which would use even more cores of the host CPU, still without affecting players too much (again, assuming I/O doesn't become the bottleneck).

Feel free to use my code or let it inspire you, or just tell me if you want a pull request.

aikar commented 5 years ago

@BaronyCraft check out PaperLib for Paper Detection and ACL linked above ^ it can let you consolidate the code into 1 approach likely.

But yes you should PR it and WB can then review it.

Brettflan commented 5 years ago

Interesting.

I've looked briefly over your code, and it generally looks good. The code style doesn't match that of this project, but I can easily clean that up. Feel free to make a pull request.

Actually, one concern I did just notice: since all pending chunks are checked in the async section and generated ones are then added to storedChunks, it seems that could potentially cause trouble for the "previous" and "inner" chunks which are loaded to allow for complete chunk generation, since adjacent chunks need to be loaded for proper full chunk generation. Since those chunks definitely already exist, yet they're handled through the async section like any other chunks, they would immediately be added to storedChunks, as if ready to be unloaded, while the target chunk actually being generated takes much longer. In that time, several sets of "previous" and "inner" chunks could have been gone through and then set to be unloaded, potentially leaving the target chunk only partially generated. Or so it appears.

BaronyCraft commented 5 years ago

Thank you for pointing that out. Yes, you're right; in my current code base, I just add already generated chunks to the same list, and I'm adding the to storedChunks and unloading them way too soon.

This didn't have much effect with Paper, as Paper has an option to delay chunk unloading (which is, by default, set to 30 seconds), so as long as I'm not stuffing the queue to 30 seconds' worth of chunk generation, chunks will stay loaded. (Unless World#unloadChunkRequest forces unloading, I didn't check that). Of course, relying on this isn't the thing to do.

Now, I'm afraid just setting dependecies (don't unload loaded chunks until the to-be-generated chunk they have been loaded for was generated) might be problematic as well; if we assume that the server is free to unload any chunk as long as no player is in range, it might unload those as well even without us requesting it to. I'll have to check into the chunk loading/unloading code of spigot and paper a bit more before making a decision here.

BaronyCraft commented 5 years ago

I rewrote the chunk-unloading logic and created a pull request. Because "starting to generate a chunk" and "receiving the generated chunk" can have several ticks between them, the server might decide the chunks that have been loaded for the benefit of the to-be-generated chunks are too far from a player to be interesting; so I added a ChunkUnloadEvent handler that prevents unloading of all chunks that are needed for another one. I got rid of the logic that checked if a chunk was loaded originally as well; the task will generally run for a long time, and players will move, so there's not much point in remembering what was loaded at the start.

Brettflan commented 5 years ago

https://github.com/Brettflan/WorldBorder/commit/8258963fd74e1ff05c70bb69273fab7290f0f693