MT-CTF / capturetheflag

Capture the Flag game using the Minetest Voxel Engine
https://ctf.rubenwardy.com
83 stars 90 forks source link

Performance Issues #320

Closed rubenwardy closed 2 years ago

rubenwardy commented 5 years ago

The purpose of this thread is to discuss the performance issues the server is afflicted by, and to find the cause.

Profiler results: https://gist.github.com/rubenwardy/649c921947c64a60bf123ba4e3eb2bb4

rubenwardy commented 5 years ago

sauth and sban

A large amount of the effort turns out to be performed in sauth and sban. I switched to those mods because the builtin authenticator would cause server freezes of up to a second every 18 seconds, which was very visible. no_guests calls sauth and sban.

Note that the auth database has 295,638 player entries as of 2019-01-02, and ban database had 646,817 entries (players=187728, playerdata=457834, bans=1255).

https://github.com/shivajiva101/sauth https://github.com/shivajiva101/sban

In-memory caching

\<shivajiva> I have been looking at caching scenarios to speed it up such as preloading the 50 most frequent players on server start, won't stop it entirely but it will reduce it considerably that reduces the lag event to new and infrequently used accounts

In-memory storing

Use LuaJIT's C-memory extension to allow sauth to store data outside of the Lua memory limit, possibly using some form of pooling (ie: allocate more memory than is used in preparation for future auths)

Switch to 5.0's builtin sqlite authentication

I'm not positive as to whether this would actually perform better

Switch to another mod, like redux

I really don't like using non-standard things, but a lot of the fixes in this topic are leaning towards non-standard

Reduce data stored

Table showing the number of players that have more than a certain amount of score.

Score Number of players Percentage
0 295,638 100%
100 11,115 3.76%
200 7,417 2.51%
500 4,160 1.41%
1000 2,578 0.87%
ClobberXD commented 5 years ago

Going by the figures, it'd be great if the player data (technically auth data) of players with score <100 (or <50?) is deleted on player leave.

ghost commented 5 years ago

100 is a bit difficult to achieve so 50 is a good shout. Also limiting the number of alt accounts per IP should limit the increasing database

ClobberXD commented 5 years ago

100 is a bit difficult to achieve

One kill can fetch upto 200 points. So a player can easily get upto 200 by hitting a badly damaged player. Combined with a bounty on that player's head, this noob can potentially bag a maximum score of 700 with one hit.

limiting the number of alt accounts per IP should limit the increasing database

Not a great idea as it can be bypassed by changing IP, which is something even a toddler can do these days.

ghost commented 5 years ago

But what is the probability of that? I think its quite low for a noob

Thomas--S commented 5 years ago

Going by the figures, it'd be great if the player data (technically auth data) of players with score <100 (or <50?) is deleted on player leave.

I'd actually suggest an even smaller value, e.g. 20 points.

When a player joins first, it is possible that they don't have great success in the beginning, so we should give them the chance to slowly improve.

ClobberXD commented 5 years ago

I also think locally caching settings will improve performance. e.g. the setting ctf.friendly_fire is retrieved every time a player is punched. If we cache it at startup, we could churn out a little more performance. This is just an example for a potential optimization, but if not fixed, this could turn out to become a noticeable source of lag when there are 8-10 players punching each other 4-5 times a second.

Note that it could still be set and retrieved while the server is running by using local methods that also update the setting itself.

rubenwardy commented 5 years ago

Settings are already locally cached by the precompiler

rubenwardy commented 5 years ago

That being said, here is a list of settings that aren't cached currently, because they're not in the settings_cache.csv file:

See build.sh and settings_cache.csv.

ClobberXD commented 5 years ago

Oh right. But when does this run? Does it have to be manually executed?

rubenwardy commented 5 years ago

This runs as part of update.sh, the script I use to update the game.

The game won't ever be updated on the server without the precompiler, because the git repo is situated at ~/.minetest/capturetheflag, and the compiled game (ie: without settings) is at ~/.minetest/games/capturetheflag. This means I can't just git pull to update the server, I need to run the precompiler too

rubenwardy commented 5 years ago

New profiler output: https://gist.github.com/rubenwardy/e478eb8f7af459be0cdddefa4ab11d04

This time sauth uses 0-1us, probably due to the caching or the server being quiet. This is 83615x faster!

rubenwardy commented 5 years ago

New profiler output with chatplus and ABMs removed: https://gist.github.com/rubenwardy/5812e2ad23bbc77e04d5aa616c1ca71c

rubenwardy commented 5 years ago

New profiler: https://gist.github.com/rubenwardy/40eb0a95966f3892f570c8142dbe427e

This is with the current master of sauth and with 23 players online. The profiler ran for about a minute

rubenwardy commented 5 years ago

New profiler: https://gist.github.com/rubenwardy/09779b81de310dc23a1b0d0088ae0cac With 10 players online

ClobberXD commented 5 years ago

Note that this isn't a very accurate estimate of server load as it's missing chat-command induced load like /rankings / /r. That's one part which could do with a ton of optimization too. :)

ranfdev commented 5 years ago

By playing on the server for a bit, i noticed old maps aren't deleted. When a new match starts, the new map is spawned next to the old one... You can actually see the old one by pressing "r" and enabling the infinite range. I suggest to delete the old map when a new match start, so the old blocks are never loaded.

rubenwardy commented 5 years ago

This would cause worse performance as deleting blocks isn't cheap. New clients won't see the old map, and the old map isn't saved to disk

rubenwardy commented 5 years ago

New profiler output: https://gist.github.com/rubenwardy/0abb9990643e93a9076b62767ce0bdcf

ranfdev commented 5 years ago

In the new profiler output, the gauges mod uses 18% of the cpu. Most of the time of the mod is spent on the on_step callback. If we remove the breath bar and keep only the health bar, we can remove the on_step callback and register to the minetest.register_on_player_hpchange callback. By doing this, if the health of a player doesn't change, nothing happens, so cpu usage of the mod drops to 0%.

Proof of concept implementation: https://github.com/ranfdev/gauges

if it works, it should reduce the server load by 18% when everyone is out of combat

ClobberXD commented 5 years ago

we can remove the on_step callback and register to the minetest.register_on_player_hpchange callback. By doing this, if the health of a player doesn't change, nothing happens, so cpu usage of the mod drops to 0%.

I like this idea :+1:

rubenwardy commented 5 years ago

we can remove the on_step callback and register to the minetest.register_on_player_hpchange callback. By doing this, if the health of a player doesn't change, nothing happens, so cpu usage of the mod drops to 0%.

Please do this, it sounds like it would be a good improvement

Here's another profiler output: https://gist.github.com/rubenwardy/38b1d79002575bf5f521cde3410643d3

Calinou commented 5 years ago

@ranfdev If it's proven to work well after some testing, I'm interested in merging this into minetest-mods/gauges :slightly_smiling_face:

ranfdev commented 5 years ago

So... I did some changes, but i don't have a server with a lot of players... If you want to test it, get the code from here: https://github.com/ranfdev/gauges. Tell me if anything goes wrong

ClobberXD commented 5 years ago

PRs: #455, #457

rubenwardy commented 4 years ago

Profile taken with 14-15 players online

https://gist.github.com/rubenwardy/9e6b8675a191a7e742876685cfed9a09

ClobberXD commented 4 years ago

Rankings seems to be a (the most, in fact) consistent bottleneck across all profiler outputs. I hope to fully refactor and optimise ctf_stats (see part 1 of the ctf_stats rework - #491) in time for 2.0.0.

Apart from the massive optimisation of using two tables to store and access player stats (where the second one maps a player name to the index of their stats), I'm thinking using SQLite3 or the like might improve performance by a lot - using mod_storage isn't very optimal for storing huge tables in JSON format.

rubenwardy commented 4 years ago

The performance issues are due to how the stats are stored in memory, not how they're stored on desk. Also, note that the slowness is only when the rankings page is generated.

Sqlite would be terrible for performance as it doesn't keep the data in memory. The data is also not relational at all, so a RDMS would be inappropriate. Ideally what we need is something that keeps it all in memory, and then writes any changes to a journal. The database file and the journal would then be reconciliated at times where demand is low, ie: when the server is booting or at the lowest hours

A good way to fix it would be to cache the ranking order and update it in an O(1)-ish operation simply by swapping elements

ClobberXD commented 4 years ago

I see. Interesting...

ClobberXD commented 4 years ago

PR #518 improves in-memory storage of rankings, thereby improving performance.