-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Rewrite chunk system #8177
Rewrite chunk system #8177
Conversation
chunks-2022-07-23_23.13.42.txt Second time this happened in about 90 minutes of testing with several different worlds. Didn't know about the debug command the first time it happened, so no report from that. Other than this it seems to work pretty well so far, and is a noticeable improvement over the normal Paper build. Haven't tested anything related to saving though. |
Done a little bit of testing with FastAsyncWorldEdit, and a Movecraft-Like Plugin I maintain, blocks are being set correctly server side, but those changes are not being sent to the client and are not visible to the client until a relog. I don't know how FAWE does it, but the plugin I maintain does this with I don't know if any of this is helpful, I can do more testing if you want, just wanted to try and be somewhat helpful. |
Perhaps worth finally closing out #1001 and its other couple children? |
Also ensure that during the unload tick that scheduling occurs only out of the schedule lock. Add more scheduling debug info to debug json and chunk task toString
Fixed in commit 7dd4845, jar has been updated
You need to provide code or i can't even begin to guess about this |
I was able to get over 300 cps with chunky generation 🚀 the rat is running overdrive!! |
This is some really interesting stuff, did a couple quickie performance comparisons to really get an idea of how this new chunk system works. Five in total, all using Aikar's flags with 8GB allocated and with I posted logs for all of these tests here but I'm just going to summarize the results below. Test 1: paper-77 (old chunk system) as a control
Test 2: paper-7dd4845
Test 3: paper-7dd4845 (with
Test 4: paper-7dd4845 (with only
Test 5: paper-7dd4845 (with only
With otherwise default configs, the new chunk system seems to be noticeably faster, especially with more worker threads in Paper. I also played around with max working count but it didn't seem to have a noticeable impact. The new chunk system looks to be able to achieve generation speeds up to about 2-3 times faster than previously. Though I acknowledge I did not actually test the control with the WorkerThreadCount flag, as I recall at least, it did not have nearly as much of an impact as with the new chunk system due to the bottlenecks in the old chunk system. If someone wants to go and confirm that, be my guest, but I think I've done enough testing of this for tonight. 😅 |
When the server shuts down, the chunk system is halted. So, any queued async chunk unload would not complete. To fix this, just execute the async chunk unload task synchronously in the save method.
Unload at least 50 chunks or 5% of the unload queue, whichever is larger
The jar has been updated to fix some unloading chunks not saving during shutdown, and to stagger chunk unloading across multiple ticks. |
Since it was not initialised, it would always be BLOCKING.
Jar has been updated to fix incorrect initial priority for I/O load tasks |
Also change all thread checks to check for whether the current thread is an instance of TickThread. This will improve reliability on watchdog shutdown as now the watchdog thread should now pass all of the thread checks in the chunk system.
I took a video for those who want to visually see how fast this new chunk system is. default.mp4 |
Fixes missing spawn/chunk ticks
Code which triggers this: @Executor
@Completion(arg = ["realmUUID"], value = ["@uuidWorlds"])
fun tp(@Sender player: Player, @Arg realmUUID: RealmUUID) {
val world = plugin.server.getWorld(realmUUID.toString()) ?: throw IllegalArgumentException("non-loaded/existing realm uuid")
player.teleportAsync(world.spawnLocation)
} This error occurs while attempting to use Player#teleportAsync method to world with no loaded chunks. I'm not sure if that's known or not so I thought it's better to report. |
@CDFN, Entity#teleportAsync isn't implemented rn, thats a todo |
The jar has been updated to 1.19.1 and has fixed chunks/entity spawning and re-added incremental chunk saving |
By dropping the data from stage 1, we did not run the unload logic in stage 2. Now, the data is simply not saved in stage 2, but still unloaded.
I have updated the jar to support mustNotSave API for chunks properly and to fix some issues with saving unnecessary chunks during shutdown / world unload, as well as to improve the performance of /save-all flush |
Can you provide the errors you are getting ? |
Okay, sweet :) thanks for the quick feedback 👍 |
I checked the test build, and on my old server Fujitsu Primergy TX100 S3P, with Intel Xeon E3 1230v2, 16GB DDR3-1600 ECC, HDD 4x500gb RAID-10, and the speed of chunks increased significantly. I'm waiting for this patch for prod env, I think on my Core i5 11400f the results will be much better. |
[18:07:03] [Craft Scheduler Thread - 21 - ModelEngine/WARN]: java.lang.NoSuchFieldError: P
|
@Chronoken ModelEngine needs to fix this, it is the same issue as mentioned before with Insights. |
Does not work with new chunk system, maybe will fix later
This also removes the other older chunk system patches
Patches that were dropped that will need to be looked into later:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raw diff looks OK to me
I know we're not supposed to use this for comments, but I just want to say a massive thank you to spotted for this terrific beast of a todo item. Best of luck with this new Paper build in October as more server setups will start using it. |
[12:32:50 WARN]: [UltimateStacker] Plugin UltimateStacker v2.3.0 generated an exception while executing task 832 Panilla & UltimateStacker Plugins (latest) are making this error on Paper#174 with new chunk rewrite system |
Please create a new issue for issues instead of commenting here. That's an error UltimateStacker will have to fix as they're getting a field through reflection that no longer exists. |
oh ok thanks i will try to ask |
5j3cBNdKmZ3vOQi9.mp4 |
In the future, I want to experiment with ticking separate independent regions of the world in parallel. However, the current chunk system is just not up to that task: The scheduling is tied to the main thread, the chunk holder state is tied to the main thread, and the chunk system cannot fully utilise the worker threads it is given.
The new chunk system can be scheduled by any thread and as a result, the only time the chunk generation logic needs to go to the main thread is when it needs to execute generation/load logic that can only be done by the main thread (i.e loading in entity/poi chunks). As a result, chunk generation should be far less affected by lower TPS.
The new chunk system is also capable of executing several chunk status generation tasks in parallel. In my testing, chunky was capable of pushing all 8 threads I gave it for generating a new world, unlike the current chunk system.
Current things left to do before this can be merged:
Current todo list:
Make player chunk scheduling asynchronous. Currently, the main thread has to aggressively add and propagate ticket levels and do the initial scheduling work, which isn't negligible and requires mostly good TPS. Once the chunk scheduling is moved to its own thread, chunk generation speed should be mostly independent of TPS and frees the main thread of the burden.There are currently many aspects of the chunk system that need to be tested:
In order to assist with debugging, I've rewritten
/paper debug chunks
and added a new command/paper holderinfo
. The new paper holderinfo command can be used to see how many read only chunks are in memory as well as protochunks.At this point, the chunk system is very new (in alpha) and as such will likely have problems that can result in data loss or even possibly corruption. Do not use this branch for anything you aren't OK with losing.
Current build: https://cdn.discordapp.com/attachments/876902758366736457/1020824757953904701/paper-paperclip-1.19.2-R0.1-SNAPSHOT-reobf.jar