Optimise WorldStatisticsProvider regionising #506
Merged
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
By using BFS and a chunk coordinate->chunk map, we can regionise in linear time - as opposed to quadratic. We also can remove the merging logic, as using a BFS guarantees that all adjacent chunks are in the same region. I noticed that this part of the code was taking excessive time while testing Moonrise on 1.21.5 with ~100k chunks loaded (using the larger view distance startup flag), as the old regionising code was quadratic (>60% of the upload time). With the new regionising code, it was reduced to <1% for the upload time. The adjacent distance checks are identical to the old code, so this will produce the exact same regions as before. If fastutil were in the common package, we could remove the added Coordinate class and just the long key directly. Using a Long in HashMap is not a solution, as the hashcode becomes x ^ z which has many duplicates.
lucko
approved these changes
May 8, 2025
|
Thank you! |
fonnymunkey
pushed a commit
to fonnymunkey/SparkUnforged
that referenced
this pull request
Jul 9, 2025
Fixed typo in Configuration.getInteger (lucko#353) Fix parsing null async profiler segments (lucko#354) Use a different websocket library Clarify command timeout warning msg Don't try to send statistics to a closed socket (lucko#357) Bump adventure versions Add note to /spark profiler open about expiry time Bump common dependency versions Tweak formatting of /spark profiler open note Include server brand in viewer payloads Add caveat to warning msg about Java agents Ignore trailing sleeping samples for duration estimation (lucko#413) Use WeakReference for command senders (lucko#417) Refactor ClassFinder into interface Add JVM information to viewer metadata Redact username from vm args (lucko#423) Add placeholder resolver API Include gamerule settings in metadata (lucko#425) Add some unit tests Check for async command completion before sleeping (lucko#431) Cleanup spark tmp dir on startup Add more automated tests Add missing license headers Relocate net.kyori.option package (lucko#432) Add uploadable health report (lucko#436) Allow mod/plugin author field to be null (lucko#442) Improve unit tests Refactor sampler node export process Include engine type in sampler proto Include datapacks info in sampler proto Include memory stats in ws updates Fix activity log unit test Improve error logging in various places Tidy up NeoForge/Forge mod init Support --ignore-sleeping with async-profiler (lucko#467) Upgrade async-profiler to v3 (lucko#386) Implement standalone profiling agent (lucko#480) Fix standalone attach on Windows (lucko#484) Add extra package relocations for standlone agent (lucko#488) Support --only-ticks-over with async-profiler (lucko#470) Upgrade async-profiler to v4 Fix name of temp directory (lucko#502) Optimise WorldStatisticsProvider regionising (lucko#506) Fix division by zero error in TPS calculation (lucko#507)
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.
By using BFS and a chunk coordinate->chunk map, we can regionise in linear time - as opposed to quadratic. We also can remove the merging logic, as using a BFS guarantees that all adjacent chunks are in the same region.
The logic essentially works by selecting any chunk, and then BFSing all of its adjacent neighbours into a region and repeating until no more chunks exist. The coordinate map facilitates the initial chunk selection and BFS marking on top of the lookup for adjacent neighbours. The coordinate map is linked so that the initial chunk selection is constant-time.
I noticed that this part of the code was taking excessive time while testing Moonrise on 1.21.5 with ~100k chunks loaded (using the larger view distance startup flag), as the old regionising code was quadratic (>60% of the upload time). With the new regionising code, it was reduced to <1% for the upload time.
The adjacent distance checks are identical to the old code, so this will produce the exact same regions as before.
If fastutil were in the common package, we could remove the added Coordinate class and just the long key directly. Using a Long in HashMap is not a solution, as the hashcode becomes
x ^ zwhich has many duplicates.