Skip to content

Conversation

@lhofhansl
Copy link
Contributor

@lhofhansl lhofhansl commented Mar 27, 2025

I was looking into why the server throughput is lower when emergequeue_limit_diskonly/emergequeue_limit_generate are set to large values. I found that in that case much time is spent in doing occlusion culling over and over again for the same blocks until they are finally loaded. For large queue sizes (5000 or 10000) this can reduce the server throughput by 25x (from 19k to 800 or less blocks/s) - and increase CPU load by the same.

The fix is to check whether the block is already enqueued. In that case occlusion culling is no longer performed.
No attempt is made to reenqueue the block either (the only possible bit we can loose is the generate, which has no impact in this case as the block is already on the queue.)

This is similar to checking blocks_sending and blocks_send before we attempt to handle them again.

Note that will be even worse if retrieving blocks from the DB takes longer (as might be the case with the Postgres backend.)

(With this I see throughput close to the theoretical maximum. On my machine it takes about 45us to load/deserialize a block, i.e. ~22k blocks/s, what I see is about 19k blocks/s... Pretty close!)

  • Goal of the PR

Increase server loading throughput

  • How does the PR work?

See description

  • Does it resolve any reported issue?
  • Does this relate to a goal in the roadmap?
  • If not a bug fix, why is this PR needed? What usecases does it solve?

To do

This PR is Ready for Review.

How to test

Load any world, dig around. Set emergequeue_limit_diskonly/emergequeue_limit_generate to large values, notice that throughput is not reduced.

@lhofhansl
Copy link
Contributor Author

lhofhansl commented Mar 28, 2025

@sfan5 How about this? This now has the exact same condition as the emerge later, and sets nearest_unsent_d (which is correct as the queue won't be full since the block already exists.)

@lhofhansl lhofhansl merged commit 8f8d7c4 into luanti-org:master Mar 28, 2025
17 checks passed
lhofhansl added a commit to lhofhansl/minetest that referenced this pull request Mar 29, 2025

// if the block is already in the emerge queue we don't have to check again
if (want_emerge && emerge->isBlockInQueue(p)) {
nearest_emerged_d = d;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a bug here actually, the if (nearest_emerged_d == -1) check is missing
so the variable will contain the furthest position in the end

lhofhansl added a commit to lhofhansl/minetest that referenced this pull request Mar 29, 2025
lhofhansl added a commit that referenced this pull request Apr 1, 2025
Partially restore the existing logic, and try to enqueue a block as before, if the queue is full it will be handled correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants