Locality-aware chunk distribution

[SirYwell]

Overview

Description

ParallelQueueExtent deals with distributing work onto multiple threads to speed up edits. In its current implementation, this is achieved by just using the Region#getChunks().iterator() as a queue and let all threads pull the next chunk to work on from there. This works well typically, but it has one downside: Different threads work on neighboring chunks. There are multiple reasons this isn't optimal:

• Filters accessing data of surrounding chunks will cause more cache misses in their STQE. As currently this also means that one STQE will trim the chunk cached/processed by another STQE, we do a lot of extra work in such cases.
• Due to how Minecraft generates chunks, processing neighboring chunks on different threads can be slower if both threads wait for the same resources.

By distributing work more location-aware, these downsides can be prevented, at the cost of a somewhat more complicated work-splitting logic. This means threads will process more tasks, but those tasks are smaller and faster to process. As an additional benefit, this allows better work distribution if multiple edits are done concurrently, as the threads don't have to finish one edit before they are free again for the next edit. Consequently, thread-local data needs to be cleared and re-set more often.

Exception handling is a bit of a question mark with this approach, I tried to get it near to the original handling, but I'm not sure if it makes sense the way it is right now.

Submitter Checklist

Preview Give feedback

1. Make sure you are opening from a topic branch (/feature/fix/docs/ branch (right side)) and not your main branch.
2. Ensure that the pull request title represents the desired changelog entry.
3. New public fields and methods are annotated with @since TODO.
4. I read and followed the contribution guidelines.

[github-actions]

Please take a moment and address the merge conflicts of your pull request. Thanks!

[dordsor21]

I can see a scenario where we end up with one ApplyTask instance effectively only utilising one thread for a 512x512 region of the edit, causing the whole edit to wait for it when everything else is finished. I don't know if there's a particular way around that, but in a complex operation this could be quite a perceived performance hit

[dordsor21]

Is postProcess necessarily the best name here? Given the use of postprocess already to define actions per-chunk. Perhaps just flushQueues is fine as we're not doing any processing (postly or otherwise) here

[dordsor21]

I would replace can with should. Whilst it's a trade-off between cross-thread mask access, I would still call higher parallelism the "wanted" outcome

[SirYwell]

I can see a scenario where we end up with one ApplyTask instance effectively only utilising one thread for a 512x512 region of the edit, causing the whole edit to wait for it when everything else is finished. I don't know if there's a particular way around that, but in a complex operation this could be quite a perceived performance hit

Yes, that's possible in theory, and the heuristic when to split is rather simple currently. In practice, it would mean the thread pool is already busy and there are even more tasks waiting. We could e.g. force splitting on 512x512 regions, or check ForkJoinTask.getSurplusQueuedTaskCount() > Settings.settings().QUEUE.PARALLEL_THREADS * f(this.shift) (where f is some "damping" function) to make that case even less likely.

I also though about re-checking the heuristics during processing, but that complicates things too much I think.

[dordsor21]

I can see a scenario where we end up with one ApplyTask instance effectively only utilising one thread for a 512x512 region of the edit, causing the whole edit to wait for it when everything else is finished. I don't know if there's a particular way around that, but in a complex operation this could be quite a perceived performance hit

Yes, that's possible in theory, and the heuristic when to split is rather simple currently. In practice, it would mean the thread pool is already busy and there are even more tasks waiting. We could e.g. force splitting on 512x512 regions, or check ForkJoinTask.getSurplusQueuedTaskCount() > Settings.settings().QUEUE.PARALLEL_THREADS * f(this.shift) (where f is some "damping" function) to make that case even less likely.

I also though about re-checking the heuristics during processing, but that complicates things too much I think.

If we are processing a larger region by smaller region at a time in the first place, rather than the for x; for z I think it would be much more reasonable to check if we want to parallelise further at the end of the completion of each subregion, and can therefore submit the rest. E.g. create a queue at the start of a region's processing and just submit the remaining as/when needed

[SirYwell]

If we are processing a larger region by smaller region at a time in the first place, rather than the for x; for z I think it would be much more reasonable to check if we want to parallelise further at the end of the completion of each subregion, and can therefore submit the rest. E.g. create a queue at the start of a region's processing and just submit the remaining as/when needed

Hm I guess processing chunks using a hilbert curve might be useful there. Otherwise, just always splitting and then unforking is basically that, and the overhead of creating that many tasks eagerly is most likely not a problem.