Reduce send block loops at large distances #16811
Open
+4
−1
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.
There are two mechanism to reduce the work the server's send loop is doing per iteration:
max_simultaneous_block_sends_per_clientto limit the number of blocks sent to a client per iterationRemoteClient::getNextBlocks, specifically themax_d_increment_at_timepart, which limits how many successive "layers" of blocks are handled. (default 2, i.e. 3 layers are considered in one iteration).When a map is mostly or fully loaded it is only the second mechanism that limits server work.
At smaller viewing ranges this is fine. At a distance of 12 blocks (viewing range 190) there are only 3458 blocks to consider per distance. But at larger range of 62 (1000 nodes) it's already 92258 blocks per layer, and at 125 (2000 nodes) it is over 370k blocks.
So as is now, the server would check over 1m blocks on each call, and a call is attempted at each server step.
I noticed significant useless work the server is doing. At viewing range 2000, the server spends up to 2700ms every 3s (i.e. 90% of the time) in just checking for changed blocks. At viewing range 1000 it is still 1700ms (over 50%).
Note that this is done under the env lock!
What this PR is doing is limiting the number of layers tracked at larger distances, since each layer by itself already is plenty of work (unfortunately there is no way to limit it even further).
With this PR applied the time spend at viewing range 2000 stays below 2000ms, and at viewing range 1000 it stays below 600ms. Loading speed and other things are unaffected, it simply splits up the work better between calls to
getNextBlocks. (If I track the max time spend per call, this is equally reduced.)The use of
m_block_cull_optimize_distance* 2 (50 by default) as cut-off point is perhaps arbitrary. I wanted to avoid adding yet another config option, but I can do so, or even hard code this.This will only have any affect players with very large viewing ranges. And in any case there should be no changes in behavior other than better server utilization.
This is a lot of explanation for trivial change. We could make it more fancy and reduce to 1 and then to 0 at different distances, but I found this sufficient in my testing.
To do
This PR is Ready for Review.
How to test
Configure a large viewing_range along with the server configs (
max_block_send_distance, andmax_block_generate_distance), wait until the world is fully loaded, observer the time spent inServer::SendBlocks(): Collect list. Notice how the time spent there is reduced with this PR.Observe that nothing else is changed... Load speed, update speed after changes, etc, etc.