-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Add random tick batching config option #12950
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
base: main
Are you sure you want to change the base?
Conversation
Is my understanding correct that the randomization of ticking prevents aliasing, such that not all chunks run the bundled random ticks all at once? |
Yes, if randomTickScale is set to 10 for instance, on average 10% of chunks will be random ticked per tick |
this pull request seems to modify the moonrise patch, as such it should be pr’d to the moonrise repo iirc |
This patch does not require moonrise and needs to be moved infront of moonrise. Additionally, IntOr.Disabled would make more sense here. Lastly, the premise of this patch seems pretty interesting. |
https://en.wiktionary.org/wiki/batch ?
cache locality too. Something OOP is usually bad at, and Minecraft's architecture never tried to work around. |
Sure, cache locality may account for some savings. 2ms saving tho, even assuming a 100ns on RAM reads, is a pretty strong estimate around a 28k ticking chunks as we'd have to miss nearly every single cache read to get to numbers like 2ms saving. I am not saying these numbers are false or anything, just that a lot of profiling needs to happen around this to find out how "supported" we would want to consider something like this. |
It's not exactly a fair benchmark because one was on 1.21.4 and one was on 1.21.8, but you can see the spark reports I based it on here. Feel free to try and benchmark this yourself as well. randomTickScale = 10 https://spark.lucko.me/BVRT4Sikki I'll change the PR to work before the moonrise patches |
bd588f7
to
204e779
Compare
I've changed the PR to work by apply the changes before moonrise. I'm not sure about using IntOr.Disabled though because this option being 1 is semantically the same as vanilla behaviour, so I'm not sure if it's really helpful to say that it's "disabled". Many other options in the Paper config work like this, for instance the TickRates config |
Yea but we are adding a completely new concept here that doesn't exist yet. |
That does make more sense, should I change it to be called |
Yea I think that' be great 👍 |
fe1c96b
to
97051d4
Compare
1fb63be
to
19affcb
Compare
This is a config option where, instead of running random ticking once every tick, you can run it once every *n* ticks, but when randomly ticked, chunks will be ticked *n* times more. While this does affect vanilla behaviour, the nature of random ticking is that this effect is barely noticeable, but it can have a significant impact on TPS. With 50 bots randomly spread on a server, and a benchmark running for two minutes, setting randomTickScale to 10 reduced the footprint of optimiseRandomDespawn from 6 ms/t to 3.9 ms/t. On a real server with about 90 players, I have observed a reduction in the footprint from 4.04 ms/t to 0.76 ms/t (both with approximately 28,000 chunks loaded, plus or minus 10%), which is a massive TPS saving.
19affcb
to
f53b2b2
Compare
should be good now |
This is a config option where, instead of running random ticking once every tick, you can run it once every n ticks, but when randomly ticked, chunks will be ticked n times more. While this does affect vanilla behaviour, the nature of random ticking is that this effect is barely noticeable, but it can have a significant impact on TPS.
With 50 bots randomly spread on a server, and a benchmark running for two minutes, setting randomTickScale to 10 reduced the footprint of optimiseRandomDespawn from 6 ms/t to 3.9 ms/t. On a real server with about 90 players, I have observed a reduction in the footprint from 4.04 ms/t to 0.76 ms/t (both with approximately 28,000 chunks loaded, plus or minus 10%), which is a significant performance boost for such a simple option.