fix: some improvements to GET chunk writing #2853
Conversation
dordsor21
force-pushed
the
fix/chunk-get-write-improvements
branch
from
3b7e56e to
79a5a50
Compare
|
fwiw I threw this at someone who experienced fewer issues after installing |
|
Please take a moment and address the merge conflicts of your pull request. Thanks! |
- ensure levelChunk is loaded before giving to copy GET - this is not necessarily guaranteed to be nonnull if two edits overlap. Whilst not advised, such an easy failure should not occur when two edits collide
…a chunk and vice versa - alter IntPair hashcode to be more often unique - Utilise ConcurrentHashMap for free synchronisation
dordsor21
force-pushed
the
fix/chunk-get-write-improvements
branch
from
88e771e to
15bd019
Compare
| } | ||
| return ReflectionUtils.compareAndSet(sections, expected, value, layer); | ||
| } finally { | ||
| chunks = SENDING_CHUNKS.get(worldName); |
There was a problem hiding this comment.
is there any scenario where this is actually needed? The old reference should still be the correct one as we never remove it. Alternatively, did you consider using a ConcurrentHashMap<*WorldIntPair*, ChunkLock> instead? This way we won't have unused CHM instances laying around (which could probably be a problem with many temporary worlds).
There was a problem hiding this comment.
The idea was to reduce lock contention between worlds when locking/unlocking. I suppose the operations themselves shouldn't take too long, though perhaps it's worth having the CHM in a BukkitWorld instance so it's GCed as needed if still attempting to do this
I also think I was planning to remove from the map when a world is unloaded
There was a problem hiding this comment.
Moving it into BukkitWorld sounds good.
There was a problem hiding this comment.
FAWE does not actually cache/limit the number of BukkitWorld instances, and given upstream does not either, it feels quite wrong to do this. I don't know if it's worth wrapping a WeakHashMap into a synchronised map and using the NMS world (or a normal bukkit world) as a key?
worldedit-bukkit/src/main/java/com/fastasyncworldedit/bukkit/adapter/NMSAdapter.java
Outdated
Show resolved
Hide resolved
worldedit-bukkit/src/main/java/com/fastasyncworldedit/bukkit/adapter/NMSAdapter.java
Outdated
Show resolved
Hide resolved
dordsor21
force-pushed
the
fix/chunk-get-write-improvements
branch
from
a86c8d6 to
cd6a701
Compare
Uh oh!
There was an error while loading. Please reload this page.