Repository navigation
Conversation
For every changed block, FastAsyncWorldEditLogger converted the old and new block state to Bukkit Material and BlockData and serialized both with getAsString(). These results depend only on the block state, and an edit usually repeats a few states many times. BlockStateConversion holds the Material, a BlockData template and the data string of one block state, all in final fields. The processor indexes conversions by FAWE ordinal in a plain array, which is safe across FAWE's threads because the fields are final; two threads filling the same slot store equivalent conversions. BlockData is mutable and postProcess may modify it (checkWaterlogged), so callers get a clone. WorldEditLogger gains a postProcess overload that takes the converted values; the existing overloads delegate to it unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CoreProtectLogger, used with WorldEdit and with FAWE when fast-placement is off, did the same per-block conversions, and getOtherHalf converted the neighbour of every bisected block. WorldEdit's BukkitAdapter builds BlockData by serializing the state and parsing the string, without caching. Both now use BlockStateConversion, looked up in a ConcurrentHashMap keyed by the WorldEdit block state. WorldEdit keeps a single instance per state with a cached hash code, so lookups match by identity. The postProcess overloads that converted the data are now unused and removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
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
For every block a WorldEdit or FAWE edit changes, CoreProtect converts the old and new block state to Bukkit
MaterialandBlockDataand serializes both withgetAsString(). These results depend only on the block state, and an edit usually repeats a small number of states many times. This change converts each state once and reuses the result. In a benchmark of 262,144-block edits, it cuts the time CoreProtect adds to an edit by about 45% on both paths:The problem
For each changed block, the loggers currently:
Material;BlockDatawithBukkitAdapter.adapt, which builds a new instance each time. In WorldEdit 7.4.5, this serializes the state to a string and parses it withBukkit.createBlockData, without caching;BlockDatawithgetAsString(), which builds a new string each time.WorldEditLogger.getOtherHalfdoes the same conversion for the neighbouring block of every bisected block. A//set stoneover 262,144 blocks only involves two states (air and stone), but does all of this 262,144 times.The fix
BlockStateConversionholds theMaterial, a templateBlockDataand the data string of one WorldEdit block state, all in final fields.BlockDatais mutable, andpostProcesscan modify it (BlockUtils.checkWaterlogged), so callers getcreateBlockData(), a clone of the template. The template itself is only read, byisOtherHalf.CoreProtectLogger, also used by FAWE withfast-placement: false) andgetOtherHalf:BlockStateConversion.of(state)looks the state up in aConcurrentHashMap. WorldEdit keeps a single instance per block state and caches its hash code, so lookups match by identity.FastAsyncWorldEditLogger): the conversions are indexed by FAWE block state ordinal in an array sized fromBlockTypesCache.states.length, which avoids hashing for every changed block. FAWE runs this processor on several threads. A plain array is safe because every field ofBlockStateConversionis final, and two threads filling the same slot store equivalent conversions.WorldEditLogger.postProcessnow takes the convertedMaterial,BlockDataand data strings. The two overloads that converted them are no longer used and are removed.extentfield andBlockVector3import are removed fromFastAsyncWorldEditLogger.Behaviour change
None. On every test scenario below, the
co_blockrows written by this branch are identical to master's.Scope
Risk
BlockDataand a short string.BlockDatacopy is still made for each use, becausepostProcessmay modify it.Testing
Build:
mvn package -Dproject.branch=developmentpasses.Environment: Paper 26.2 build 135, DuckDB (the default backend), a 3-core VM. The edits are run by a small test plugin that builds an
EditSessionwith the console as actor. The plugin is not part of this PR.Row parity, in four modes: WorldEdit 7.4.5, FAWE 2.15.4 on the main thread, FAWE from an async task (parallel queue), and FAWE with
fast-placement: false. Each run, on master and on this branch:BlockTypeMask);/co rollbackand/co restorefor those edits;Then all
co_blockrows (user, world, position, type, data, meta, blockdata, action, rolled_back) were dumped, sorted and compared:fast-placement: falseBlock counts after each step, rollback and restore were also identical, with no errors in the console.
Performance:
setBlockson a 64×64×64 region, alternating stone and air (262,144 changed blocks per edit). Per setup: 2 server runs × (1 warm-up edit + 10 measured edits), 20 measured edits in total. All setups were run alternately in the same session. The time is measured around theEditSessionand includes the edit itself.The overhead figures in the summary are the medians minus the median without CoreProtect. In every run the consumer queue was empty by the end. I have not profiled where the remaining time goes. Timings on this VM vary between sessions, so the table above, where all setups ran side by side, is the one to compare.