[MIRROR] Resolves is_banned_from headaches and lag (Speeds up roundstart significantly) [MDB IGNORE] (#16001)

* Resolves is_banned_from headaches and lag (Speeds up roundstart significantly) (#69376)

About The Pull Request

Just to be clear, when I refer to time here, I am not talking about cpu time. I'm talking about real time.
This doesn't significantly reduce the amount of work we do, it just removes a lot of the waiting around we need to do for db calls to finish.

Adds queuing support to sql bans, so if an ongoing ban retrieval query is active any successive ban retrieval attempts will wait for the active query to finish

This uses the number/blocking_query_timeout config option, I hope it's still valid

This system will allow us to precache ban info, in parallel (or in batches)
With this, we can avoid needing to setup all uses of is_banned_from to support parallelization or eat the cost of in-series database requests

Clients who join after initialize will now build a ban cache automatically

Those who join before init is done will be gathered by a batch query sent by a new subsystem, SSban_cache.

This means that any post initalize uses of is_banned_from are worst case by NATURE parallel (since the request is already sent, and we're just waiting for the response)

This saves a lot of headache for implementers (users) of the proc, and saves ~0.9 second from roundstart setup for each client (on /tg/station)

There's a lot of in series is_banned_from calls in there, and this nukes them. This should bring down roundstart join times significantly.

It's hard to say exactly how much, since some cases generate the ban cache at other times.
At base tho, we save about 0.9 seconds of real time per client off doing this stuff in parallel.
Why It's Good For The Game

    When I use percentages I'm speaking about cost per player

I don't like how slow roundstart feels, this kills about 66% of that. the rest is a lot of misc things. About 11% (it's actually 16%) is general mob placing which is hard to optimize. 22% is manifest generation, most of which is GetFlatIcons which REALLY do not need to be holding up the main thread of execution.

An additional 1 second is constant cost from a db query we make to tell the server we exist, which can be made async to avoid holding the proc chain.

That's it. I'm bullying someone into working on the manifest issue, so that should just leave 16% of mob placing, which is really not that bad compared to what we have now.
Changelog

cl
code: The time between the round starting and the game like, actually starting has been reduced by 66%
refactor: I've slightly changed how ban caches are generated, admins please let me know if anything goes fuckey
server: I'm using the blocking_query_timeout config. Make sure it's up to date and all.
/cl

* Resolves is_banned_from headaches and lag (Speeds up roundstart significantly)

Co-authored-by: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com>
This commit is contained in:
SkyratBot
2022-09-02 04:11:40 +02:00
committed by GitHub
parent f6317d2c2c
commit e7230e8b4a
8 changed files with 122 additions and 5 deletions

View File

@@ -0,0 +1,73 @@
/// Subsystem that batches a ban cache list for clients on initialize
/// This way we don't need to do ban checks in series later in the code
SUBSYSTEM_DEF(ban_cache)
/datum/controller/subsystem/ban_cache
name = "Ban Cache"
init_order = INIT_ORDER_BAN_CACHE
flags = SS_NO_FIRE
var/query_started = FALSE
/datum/controller/subsystem/ban_cache/Initialize(start_timeofday)
generate_queries()
return ..()
/// Generates ban caches for any logged in clients. This ensures the amount of in-series ban checking we have to do that actually involves sleeps is VERY low
/datum/controller/subsystem/ban_cache/proc/generate_queries()
query_started = TRUE
if(!SSdbcore.Connect())
return
var/current_time = REALTIMEOFDAY
var/list/look_for = list()
for(var/ckey in GLOB.directory)
var/client/lad = GLOB.directory[ckey]
// If they've already got a ban cached, or a request goin, don't do it
if(lad.ban_cache || lad.ban_cache_start)
continue
look_for += ckey
lad.ban_cache_start = current_time
// We're gonna try and make a query for clients
var/datum/db_query/query_batch_ban_cache = SSdbcore.NewQuery(
"SELECT ckey, role, applies_to_admins FROM [format_table_name("ban")] WHERE ckey IN (:ckeys) AND unbanned_datetime IS NULL AND (expiration_time IS NULL OR expiration_time > NOW())",
list("ckeys" = look_for.Join(","))
)
var/succeeded = query_batch_ban_cache.Execute()
for(var/ckey in look_for)
var/client/lad = GLOB.directory[ckey]
if(!lad || lad.ban_cache_start != current_time)
continue
lad.ban_cache_start = 0
if(!succeeded)
qdel(query_batch_ban_cache)
return
var/list/ckey_to_bans = list()
// Runs after the check for safety, don't want to override anything
for(var/ckey in look_for)
ckey_to_bans[ckey] = list()
while(query_batch_ban_cache.NextRow())
var/ckey = query_batch_ban_cache.item[1]
var/role = query_batch_ban_cache.item[2]
var/hits_admins = query_batch_ban_cache.item[3]
var/list/bans = ckey_to_bans[ckey]
if(!bans)
continue
// Yes I know this is slightly unoptimal, no I do not care
var/is_admin = GLOB.admin_datums[ckey] || GLOB.deadmins[ckey]
if(is_admin && !text2num(hits_admins))
continue
bans[role] = TRUE
for(var/ckey in ckey_to_bans)
var/client/lad = GLOB.directory[ckey]
if(!lad)
continue
lad.ban_cache = ckey_to_bans[ckey]
qdel(query_batch_ban_cache)

View File

@@ -276,7 +276,7 @@ SUBSYSTEM_DEF(ticker)
log_world("Game start took [(world.timeofday - init_start)/10]s")
round_start_time = world.time
SSdbcore.SetRoundStart()
INVOKE_ASYNC(SSdbcore, /datum/controller/subsystem/dbcore/proc/SetRoundStart)
to_chat(world, span_notice("<B>Welcome to [station_name()], enjoy your stay!</B>"))
alert_sound_to_playing(sound(SSstation.announcer.get_rand_welcome_sound())) //SKYRAT EDIT CHANGE