Apply DataArray patch and update Bukkit adapters.

[JayemCeekay]

Overview

Fixes #1938

Description

This pull request implements the patch for the use of type ambiguous DataArrays created by @SirYwell. This is necessary for the upcoming implementation of updated Forge and Fabric adapters in order to allow the registration and use of more than 65535 blockstates.

Submitter Checklist

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

[SirYwell]

I updated this branch to reflect the latest changes on main

[SirYwell]

Retargeted to v3 and rebased, though I need to re-review the changes myself because there were some ugly merge conflicts that probably broke something.

[github-actions[bot]]

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

[github-actions[bot]]

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

[github-actions[bot]]

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

[SirYwell]

The casts to char should only be present in the bukkit module, but I'm fine with changing it to int there too.

I kinda wonder if we should just move entirely to int based arrays. The amount of extra method calls for setting blocks required it quite substantial and will have a performance impact. I don't really see the memory impact being particularly large.

I don't expect the methods to have measurable overhead over direct array access (we have one additional memory indirection until we have value classes, the methods are small, the JVM can inline them easily). Memory overhead of always using int[] might be fine, but that also mean more cache misses. It also might be interesting in future to explore Foreign Memory based approaches, in which case the DataArray abstraction would be useful too. So I think it's not worth to not have the DataArray abstraction.

Also, clipboards will need to be investigated as they are very char based (and moving to an int-based system will halve the capacity of disk-based solutions)

I started investigating that in https://github.com/IntellectualSites/FastAsyncWorldEdit/tree/feature/disk-based-clipboard, but there are more things to consider.

[dordsor21]

The casts to char should only be present in the bukkit module, but I'm fine with changing it to int there too.

I'm not sure that's something we can 100% assume with datapacks, etc?

I don't expect the methods to have measurable overhead over direct array access (we have one additional memory indirection until we have value classes, the methods are small, the JVM can inline them easily). Memory overhead of always using int[] might be fine, but that also mean more cache misses. It also might be interesting in future to explore Foreign Memory based approaches, in which case the DataArray abstraction would be useful too. So I think it's not worth to not have the DataArray abstraction.

I'm not sure if an increased cache miss likelihood is really a large performance hit? I already assume that large edits that cannot be done per-chunk (large clipboard operations, any recursive operations) are already missing L1 (and very possibly L2 and L3 cache too) as the number of chunks being loaded and re-loaded is quite large. And then if a whole chunk's block arrays (max 200 KiB with char, 400 with int) are missing L3 cache we should be attempting to do something about that. Besides, I doubt that most servers have sole access to a single CPU to be able to be making full using the L1-3 cache anyway, and I would expect lots of cache misses due to running websites, other servers, services etc anyway. Switching to all int[] is more maintainable too

I started investigating that in feature/disk-based-clipboard, but there are more things to consider.

Ah yeah I'd forgotten about that, looks to make it work

[SirYwell]

I'm not sure that's something we can 100% assume with datapacks, etc?

AFAIK it's still not possible to have additional block types/states. Custom biomes are possible though.

I'm not sure if an increased cache miss likelihood is really a large performance hit

Most likely not, my point was more that we'd need to measure to really know the impact here (and even then, there are many parts that probably aren't true anymore in the next Java release). I'd really like to keep the DataArray abstraction, there are a few things that are far easier to understand with that approach (e.g. avoiding System.arraycopy in the middle of already complex code, filling arrays, etc). It generally is more future-proof and will potentially allow for a bunch of optimizations in future.