Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize MovingBlockSuppressorRenderer for common case of no structures #1885

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

embeddedt
Copy link
Contributor

This PR fixes an issue I actually originally noted & patched in 1.12.2, and then encountered on 1.20.1 when trying out the new ProjectRed beta. The logic to suppress moving blocks runs through quite a bit of logic on every single block in the world, which has a measurable hit to chunk meshing performance (see below profiler screenshot).

2024-07-30_22-15

To solve this I made a number of optimizations, tailored for the common case of there being no structures in the world:

  • Added a direct getter for the client level's MovementManager that skips needing to check if the level is client-side, and also skips creating the movement manager if it doesn't exist.
  • Make the maps from dimension to MovementManager use identity equality & hashing, for slightly better lookup performance.
  • Bail out early from MovingBlockSuppressorRenderer#canHandleBlock if the movement manager is not tracking any moving structures.
  • Hoist retrieval of the MovementManager to a local variable in canHandleBlock, so it doesn't need to be re-retrieved for the block position & all adjacent positions.

This code could be optimized further to benefit the case where there are structures as well, but the current optimizations are relatively simple & should not break anything, while fully addressing the original issue (see profiling after these changes below).

2024-07-30_22-21

@MrTJP
Copy link
Owner

MrTJP commented Jul 31, 2024

Sweet! Will check this out and merge it in the morning. I was aware that canHandleBlock is going to run for every block but didn't get around to actually profiling it.

Thanks!

@MrTJP MrTJP merged commit 13e8320 into MrTJP:main Jul 31, 2024
1 of 2 checks passed
@embeddedt embeddedt deleted the reduce-movementmanager-overhead branch August 1, 2024 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants