Key the explosion scan by packed block position - #1021
Open
tricrotism wants to merge 1 commit into
Open
tricrotism wants to merge 1 commit into
tricrotism wants to merge 1 commit into
Conversation
The explosion scan cloned a Location for every block and built five more per block to find attached neighbours, hashing each one on the region thread. It now keys the map by a packed long position and walks the neighbours from an offset table. The same blocks are logged.
❌ Deploy Preview for coreprotect failed. Why did it fail? →
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Every logged explosion builds and hashes a pile of
Locationobjects on the region thread to find attached blocks next to the blast. Keying the scan by a packed long instead halves CoreProtect's cost per explosion on Paper in my measurements, with the same blocks logged.The problem
BlockExplodeListener.processBlockExplode(listener/block/BlockExplodeListener.java:34-88) runs for every explosion that CoreProtect logs:HashMap<Location, Block>, which clones aLocationper block;Locations (+x, -x, +z, -z, +y) and looks each one up.Location.hashCodeandequalshash and compare three doubles, yaw, pitch and the world on every lookup;Location, and the world's min and max height are re-read inside the loop.A TNT or creeper blast touches dozens of blocks, so this is hundreds of short-lived objects and double hashes per explosion, all on the thread that owns the blast.
The fix
long, using the same bit layout Minecraft uses forBlockPos(26 bits x, 26 bits z, 12 bits y).world.getBlockAt(int, int, int), with noLocationbuilt.The set of blocks logged is the same. Their order within one explosion follows the new map's iteration order. Upstream's
HashMap<Location, Block>order was already arbitrary.Behaviour change
None.
Risk
Low. One file, no signature change. The packing is exact for every coordinate a world can hold (x and z within ±33,554,432, y within the 4,096 block range).
Testing
Build:
mvn packagepasses.Performance, PathLoad
explosionsmode on Paper 26.2, 2 GB heap, autosave off: 100 explosions per second, 60 s warmup, then the time of eachcreateExplosioncall over a 60 s window. Three runs of each jar, alternating. With no CoreProtect installed a call costs 151.4 µs.The same runs on Folia 1.21.11 varied by up to 17 µs between runs of one jar and did not separate either way.
Row parity: a 47-step scenario plugin (which includes an explosion, then rollback and restore) on Paper 26.2 and Folia 1.21.11 with SQLite, compared order-insensitively against upstream run the same way. The explosion rows are identical. The only other differences are the plant that bone meal happens to grow (random on every run) and one far-basin water row whose settling window lands either side of the boundary depending on run pacing. No new errors.