perf(world_editor): single-traversal write on the default no-override path#1067
Merged
Merged
Conversation
… path set_block(..., None, None) is the dominant write pattern across all element processors (~240 callsites). It previously went through get_block() + set_block_with_properties(), descending the region/chunk/section FnvHashMap hierarchy twice per write. Add WorldToModify::set_with_props_if_absent and route the None/None branch of set_block_with_properties_absolute through it. A single entry().or_default() descent checks storage.get(idx) == AIR and writes in one pass. Benchmarked on Munich city centre (--terrain, cached data, 5 runs each): Before: mean 9925 ms, median 9916 ms After: mean 9480 ms, median 9441 ms (-4.5% / -4.8%)
Contributor
Author
|
The benchmark ran successfully but the verdict was "generation time is unchanged" since CI rounds to whole seconds, so the local 4.5% improvement falls below CI's resolution. Can retrigger, I don't mind |
Owner
|
Perfect, thank you so much! |
pull Bot
pushed a commit
to RealShocky/arnis
that referenced
this pull request
May 31, 2026
set_block_if_absent now delegates to set_with_props_if_absent (from louis-e#1067) instead of duplicating the region/chunk/section descent. Behaviourally identical for the no-props case; `#[inline]` keeps it free. Adds unit tests for the shared write-if-absent path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
set_block(..., None, None)is the dominant write pattern across all element processors (~240 callsites). It currently goes throughget_block()followed byset_block_with_properties(), descending the region/chunk/sectionFnvHashMaphierarchy twice per write.This PR adds
WorldToModify::set_with_props_if_absent, a single-traversal variant that does oneentry().or_default()descent and checkssection.storage.get(idx) == AIRdirectly. TheNone/Nonebranch ofset_block_with_properties_absoluteroutes to it via an early-return. The whitelist/blacklist slow path is untouched. Same pattern as the existingset_block_if_absentused byground_generation.SectionToModify::get_blockreturnsNoneiffstorage.get(idx) == AIR, so the condition checked is identical to the old path. Behaviour is preserved, only the number of map lookups is halved.Benchmarked on Munich centre (
48.125768,11.552296,48.148565,11.593838) with--terrain --benchmark --fileover cached OSM data, release build, 5 runs each:All five after-runs were faster than all five before-runs.