[DRAFT] [DO NOT MERGE YET] Fix entity ticking and other improvements #28
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
UnlimitedBytes
force-pushed
the
ver/1.21.11
branch
2 times, most recently
from
5de4441 to
867545a
Compare
UnlimitedBytes
changed the title
Apply scoreboard thread-safety as source patches instead of feature patch
Apply raid thread-safety as source patches instead of feature patch
Apply redstone torch thread-safety as source patch instead of feature patch
Apply entity activation optimization as source patch instead of feature patch
Apply item gravity optimization as source patch instead of feature patch
Execute scheduled tasks on the player's task scheduler during tick (e.g., packet handlers)
Sync patch context and naming conventions with upstream
UnlimitedBytes
force-pushed
the
ver/1.21.11
branch
from
867545a to
4b61bfd
Compare
These 12 patches have been verified as already integrated into the 1.21.11 codebase: - 0026-Run-unsupported-plugins-in-sync (fully integrated) - 0027-Thread-safe-AreaMap (superseded by Moonrise architecture) - 0028-Threaded-chunk-changes-broadcasting (fully integrated) - 0039-Thread-safe-redstoneUpdateInfos (fully integrated) - 0040-Thread-safe-chunksBeingWorkedOn (superseded by Moonrise) - 0048-Synchronize-playersSentChunkTo (fully integrated) - 0056-Don-t-allow-actions-outside-of-our-region (fully integrated) - 0061-Optimization-Use-lazyExecute-if-we-aren-t-flushing (fully integrated) - 0069-Optimization-Load-chunk-before-joining-player (fully integrated) - 0071-Fix-Concurrent-modification-when-removing-player (fully integrated) - X0016-EntityLookup-accessibleEntities (fully integrated) - X0055-Moving-into-another-region (fully integrated)
These patches cannot be applied to 1.21.11: - X0017-World-gen: Marked as obsolete, functionality refactored - X0019-Multithreaded-WeakSeqLock: Target class (WeakSeqLock) doesn't exist in 1.21.11 - X0036-Entity-retirement-debug-log: Debug/experimental patch, no longer needed - 0058-Fix-EntityRemoveEventTest: Depends on Pufferfish optimization that doesn't exist
Adds thread safety checks to CraftServer.createWorld() to ensure worlds are only loaded from the global scheduler tick thread, not from async or region tick threads.
Adds thread safety checks to CraftChunk, CraftWorld, and CraftBlock to ensure chunk and block operations are only performed from the correct region tick thread: - CraftChunk.getHandle(): Check region ownership - CraftWorld.getChunkAt(): Check region ownership - CraftBlock: getNMS(), getData(), getType(), setBlockState() ensure tick thread for block position
Most changes from the original patch were already applied. This ports the remaining null safety check for this.server in PaperEventManager callEvent() to prevent NPE during early initialization.
Adds ReentrantLock-based thread safety to TrackedEntity's seenBy set operations in ChunkMap to prevent ConcurrentModificationException when multiple threads access entity tracking data.
Adds tracker update frequency optimization and maximum trackers per entity limit.
Adds thread synchronization for command block output updates.
Adds event calls for packet broadcasting in ChunkMap, ServerLevel, and PlayerList.
Adds asynchronous writing for player save data to improve performance.
UnlimitedBytes
force-pushed
the
ver/1.21.11
branch
from
a8a26f4 to
606722c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
ensureFullStatusChangeWithWriteLockusinghasWriteLockinstead ofisCurrentlyTickingRegionto prevent infinite deferral loopDetails
The main fix addresses an issue where entities were not receiving
onTickingStartcallbacks becauseensureFullStatusChangeWithWriteLock()was checkingisCurrentlyTickingRegion()which only returns true inside the tick loop. Tasks scheduled viaShreddedPaper.runSync()run on the thread pool with a write lock but NOT inside the tick loop, causing the check to always fail and creating an infinite deferral loop.The fix changes the check to use
hasWriteLock()which correctly identifies when the thread has proper synchronization to modify chunk status.Test plan