Skip to content

Commit

Permalink
[PORTS] Garbage Collection Optimizations (#2435)
Browse files Browse the repository at this point in the history
* Refsearch Info Injection (#78574)

## About The Pull Request

Adds a proc that types can override to inject extra information into the
refsearch
This'll allow us to more easily track and deal with refs held by general
datums, like callbacks.
I've implemented a template example FOR callbacks, to provide an example
and assist in solving future issues

Done to help lumipharon from TGMC, they were having trouble with this
case.

This isn't perfectly optimized, but this proc has a LOT of issues just
in general. Need to rework it to cut down on string churn someday

* Optimizes Reftracking (Bigly) (Plus harddel fixes) (#80443)

Alllright so reftracking is slow, really really slow.
That's a problem for me, both because I want it to be fast so I can more
efficiently torture players by running it on live, but also because it
impedes both local and CI runs.

So I've set out to micro optimize the DoSearchVar proc, one of the
hottest in the game.
I've done this in a few different ways.

Removing redundant proc args
Yeeting assoc arg setting (extra cost)
Moving if statements around to prioritize the more common case
Ignoring empty lists.

Throwing our snowflake list checking into the sun
(Background, byond has some special lists that cannot be accessed like
an assoc list, trying to will lead to runtimes)
The way we handle this involves inspecting their ref string, and it eats
a LOT of time.

Faster then to mark all the lists we know are special by var name, and
then use try/catch to detect and silence anything that sneaks through
(this is on the order of like 1/3 per run, kinda curious what they are
tbh)
Thanks to MSO for the idea for this btw.

Removes the vars and logic that tied ref searching to clients.
It's not how this code is used, and it slows everything else down for
really no reason

Added support for handing in a known "hanging reference" count, and then
searching for that.
This lets us early exit the ref search if we find everything we were
looking for, which is REALLY powerful, and why I asked for refcount() in
the first place.

[Fixes some harddels w gulag stuff born of the 515 one way ref
issues](tgstation/tgstation@046d7da)

[Ensures proximity cameras clean their ref to their proximity datum if
it's
deleted](tgstation/tgstation@ff607e9)

[Deleting a pipe connected via the gas_machine_connector datum to a
machine should also delete that machine (harddel
fix)](tgstation/tgstation@9eecca2)

All this combined speeds up refsearching massively, on the order of
hundreds of seconds, and makes it far less time consuming for both CI
and running on live.
I'll be bullying some servers semi soon, want to see what I can cut out.

---------

Co-authored-by: LemonInTheDark <[email protected]>
  • Loading branch information
LikeLakers2 and LemonInTheDark authored Jun 27, 2024
1 parent 0bcae55 commit c00f89b
Show file tree
Hide file tree
Showing 7 changed files with 186 additions and 124 deletions.
19 changes: 13 additions & 6 deletions code/controllers/subsystem/garbage.dm
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ SUBSYSTEM_DEF(garbage)

lastlevel = level

//We do this rather then for(var/refID in queue) because that sort of for loop copies the whole list.
// 1 from the hard reference in the queue, and 1 from the variable used before this
#define REFS_WE_EXPECT 2

//We do this rather then for(var/list/ref_info in queue) because that sort of for loop copies the whole list.
//Normally this isn't expensive, but the gc queue can grow to 40k items, and that gets costly/causes overrun.
for (var/i in 1 to length(queue))
var/list/L = queue[i]
Expand All @@ -173,10 +176,9 @@ SUBSYSTEM_DEF(garbage)
count++

var/datum/D = L[GC_QUEUE_ITEM_REF]
// 1 from the hard reference in the queue, and 1 from the variable used before this
// If that's all we've got, send er off
if (refcount(D) == 2)

// If that's all we've got, send er off
if (refcount(D) == REFS_WE_EXPECT)
++gcedlasttick
++totalgcs
pass_counts[level]++
Expand All @@ -197,12 +199,15 @@ SUBSYSTEM_DEF(garbage)
switch (level)
if (GC_QUEUE_CHECK)
#ifdef REFERENCE_TRACKING
// Decides how many refs to look for (potentially)
// Based off the remaining and the ones we can account for
var/remaining_refs = refcount(D) - REFS_WE_EXPECT
if(reference_find_on_fail[text_ref(D)])
INVOKE_ASYNC(D, TYPE_PROC_REF(/datum,find_references))
INVOKE_ASYNC(D, TYPE_PROC_REF(/datum,find_references), remaining_refs)
ref_searching = TRUE
#ifdef GC_FAILURE_HARD_LOOKUP
else
INVOKE_ASYNC(D, TYPE_PROC_REF(/datum,find_references))
INVOKE_ASYNC(D, TYPE_PROC_REF(/datum,find_references), remaining_refs)
ref_searching = TRUE
#endif
reference_find_on_fail -= text_ref(D)
Expand Down Expand Up @@ -250,6 +255,8 @@ SUBSYSTEM_DEF(garbage)
queue.Cut(1,count+1)
count = 0

#undef REFS_WE_EXPECT

/datum/controller/subsystem/garbage/proc/Queue(datum/D, level = GC_QUEUE_FILTER)
if (isnull(D))
return
Expand Down
6 changes: 5 additions & 1 deletion code/datums/datum.dm
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,12 @@
var/list/filter_data

#ifdef REFERENCE_TRACKING
var/running_find_references
/// When was this datum last touched by a reftracker?
/// If this value doesn't match with the start of the search
/// We know this datum has never been seen before, and we should check it
var/last_find_references = 0
/// How many references we're trying to find when searching
var/references_to_clear = 0
#ifdef REFERENCE_TRACKING_DEBUG
///Stores info about where refs are found, used for sanity checks and testing
var/list/found_refs
Expand Down
5 changes: 5 additions & 0 deletions code/game/machinery/camera/camera.dm
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ MAPPING_DIRECTIONAL_HELPERS(/obj/machinery/camera/xray, 0)
/obj/machinery/camera/proc/create_prox_monitor()
if(!proximity_monitor)
proximity_monitor = new(src, 1)
RegisterSignal(proximity_monitor, COMSIG_QDELETING, PROC_REF(proximity_deleted))

/obj/machinery/camera/proc/proximity_deleted()
SIGNAL_HANDLER
proximity_monitor = null

/obj/machinery/camera/proc/set_area_motion(area/A)
area_motion = A
Expand Down
1 change: 1 addition & 0 deletions code/game/machinery/gulag_item_reclaimer.dm
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
I.forceMove(get_turf(src))
if(linked_teleporter)
linked_teleporter.linked_reclaimer = null
linked_teleporter = null
return ..()

/obj/machinery/gulag_item_reclaimer/emag_act(mob/user, obj/item/card/emag/emag_card)
Expand Down
1 change: 1 addition & 0 deletions code/game/machinery/gulag_teleporter.dm
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ The console is located at computer/gulag_teleporter.dm
/obj/machinery/gulag_teleporter/Destroy()
if(linked_reclaimer)
linked_reclaimer.linked_teleporter = null
linked_reclaimer = null
return ..()

/obj/machinery/gulag_teleporter/interact(mob/user)
Expand Down
218 changes: 133 additions & 85 deletions code/modules/admin/view_variables/reference_tracking.dm
Original file line number Diff line number Diff line change
@@ -1,82 +1,76 @@
#ifdef REFERENCE_TRACKING
#define REFSEARCH_RECURSE_LIMIT 64

/datum/proc/find_references(skip_alert)
running_find_references = type
/datum/proc/find_references(references_to_clear = INFINITY)
if(usr?.client)
if(usr.client.running_find_references)
log_reftracker("CANCELLED search for references to a [usr.client.running_find_references].")
usr.client.running_find_references = null
running_find_references = null
//restart the garbage collector
SSgarbage.can_fire = TRUE
SSgarbage.update_nextfire(reset_time = TRUE)
return

if(!skip_alert && tgui_alert(usr,"Running this will lock everything up for about 5 minutes. Would you like to begin the search?", "Find References", list("Yes", "No")) != "Yes")
running_find_references = null
if(tgui_alert(usr,"Running this will lock everything up for about 5 minutes. Would you like to begin the search?", "Find References", list("Yes", "No")) != "Yes")
return

src.references_to_clear = references_to_clear
//this keeps the garbage collector from failing to collect objects being searched for in here
SSgarbage.can_fire = FALSE

if(usr?.client)
usr.client.running_find_references = type
_search_references()
//restart the garbage collector
SSgarbage.can_fire = TRUE
SSgarbage.update_nextfire(reset_time = TRUE)

log_reftracker("Beginning search for references to a [type].")
/datum/proc/_search_references()
log_reftracker("Beginning search for references to a [type], looking for [references_to_clear] refs.")

var/starting_time = world.time

log_reftracker("Refcount for [type]: [refcount(src)]")

//Time to search the whole game for our ref
DoSearchVar(GLOB, "GLOB", search_time = starting_time) //globals
DoSearchVar(GLOB, "GLOB", starting_time) //globals
log_reftracker("Finished searching globals")
if(src.references_to_clear == 0)
return

//Yes we do actually need to do this. The searcher refuses to read weird lists
//And global.vars is a really weird list
var/global_vars = list()
for(var/key in global.vars)
global_vars[key] = global.vars[key]

DoSearchVar(global_vars, "Native Global", search_time = starting_time)
DoSearchVar(global_vars, "Native Global", starting_time)
log_reftracker("Finished searching native globals")
if(src.references_to_clear == 0)
return

for(var/datum/thing in world) //atoms (don't beleive its lies)
DoSearchVar(thing, "World -> [thing.type]", search_time = starting_time)
DoSearchVar(thing, "World -> [thing.type]", starting_time)
if(src.references_to_clear == 0)
break
log_reftracker("Finished searching atoms")
if(src.references_to_clear == 0)
return

for(var/datum/thing) //datums
DoSearchVar(thing, "Datums -> [thing.type]", search_time = starting_time)
DoSearchVar(thing, "Datums -> [thing.type]", starting_time)
if(src.references_to_clear == 0)
break
log_reftracker("Finished searching datums")
if(src.references_to_clear == 0)
return

//Warning, attempting to search clients like this will cause crashes if done on live. Watch yourself
#ifndef REFERENCE_DOING_IT_LIVE
for(var/client/thing) //clients
DoSearchVar(thing, "Clients -> [thing.type]", search_time = starting_time)
DoSearchVar(thing, "Clients -> [thing.type]", starting_time)
if(src.references_to_clear == 0)
break
log_reftracker("Finished searching clients")
if(src.references_to_clear == 0)
return
#endif

log_reftracker("Completed search for references to a [type].")

if(usr?.client)
usr.client.running_find_references = null
running_find_references = null

//restart the garbage collector
SSgarbage.can_fire = TRUE
SSgarbage.update_nextfire(reset_time = TRUE)

/datum/proc/DoSearchVar(potential_container, container_name, recursive_limit = 64, search_time = world.time)
#ifdef REFERENCE_TRACKING_DEBUG
if(SSgarbage.should_save_refs && !found_refs)
found_refs = list()
#endif

if(usr?.client && !usr.client.running_find_references)
/datum/proc/DoSearchVar(potential_container, container_name, search_time, recursion_count, is_special_list)
if(recursion_count >= REFSEARCH_RECURSE_LIMIT)
log_reftracker("Recursion limit reached. [container_name]")
return

if(!recursive_limit)
log_reftracker("Recursion limit reached. [container_name]")
if(references_to_clear == 0)
return

//Check each time you go down a layer. This makes it a bit slow, but it won't effect the rest of the game at all
Expand All @@ -92,68 +86,122 @@
datum_container.last_find_references = search_time
var/list/vars_list = datum_container.vars

var/is_atom = FALSE
var/is_area = FALSE
if(isatom(datum_container))
is_atom = TRUE
if(isarea(datum_container))
is_area = TRUE
for(var/varname in vars_list)
#ifndef FIND_REF_NO_CHECK_TICK
CHECK_TICK
#endif
if (varname == "vars" || varname == "vis_locs") //Fun fact, vis_locs don't count for references
continue
var/variable = vars_list[varname]

if(variable == src)
if(islist(variable))
//Fun fact, vis_locs don't count for references
if(varname == "vars" || (is_atom && (varname == "vis_locs" || varname == "overlays" || varname == "underlays" || varname == "filters" || varname == "verbs" || (is_area && varname == "contents"))))
continue
// We do this after the varname check to avoid area contents (reading it incures a world loop's worth of cost)
if(!length(variable))
continue
DoSearchVar(variable,\
"[container_name] [datum_container.ref_search_details()] -> [varname] (list)",\
search_time,\
recursion_count + 1,\
/*is_special_list = */ is_atom && (varname == "contents" || varname == "vis_contents" || varname == "locs"))
else if(variable == src)
#ifdef REFERENCE_TRACKING_DEBUG
if(SSgarbage.should_save_refs)
if(!found_refs)
found_refs = list()
found_refs[varname] = TRUE
continue //End early, don't want these logging
else
log_reftracker("Found [type] [text_ref(src)] in [datum_container.type]'s [datum_container.ref_search_details()] [varname] var. [container_name]")
#else
log_reftracker("Found [type] [text_ref(src)] in [datum_container.type]'s [datum_container.ref_search_details()] [varname] var. [container_name]")
#endif
log_reftracker("Found [type] [text_ref(src)] in [datum_container.type]'s [text_ref(datum_container)] [varname] var. [container_name]")
references_to_clear -= 1
if(references_to_clear == 0)
log_reftracker("All references to [type] [text_ref(src)] found, exiting.")
return
continue

if(islist(variable))
DoSearchVar(variable, "[container_name] [text_ref(datum_container)] -> [varname] (list)", recursive_limit - 1, search_time)

else if(islist(potential_container))
var/normal = IS_NORMAL_LIST(potential_container)
var/list/potential_cache = potential_container
for(var/element_in_list in potential_cache)
#ifndef FIND_REF_NO_CHECK_TICK
CHECK_TICK
#endif
//Check normal sublists
if(islist(element_in_list))
if(length(element_in_list))
DoSearchVar(element_in_list, "[container_name] -> [element_in_list] (list)", search_time, recursion_count + 1)
//Check normal entrys
if(element_in_list == src)
else if(element_in_list == src)
#ifdef REFERENCE_TRACKING_DEBUG
if(SSgarbage.should_save_refs)
if(!found_refs)
found_refs = list()
found_refs[potential_cache] = TRUE
continue //End early, don't want these logging
#endif
continue
else
log_reftracker("Found [type] [text_ref(src)] in list [container_name].")
#else
log_reftracker("Found [type] [text_ref(src)] in list [container_name].")
continue

var/assoc_val = null
if(!isnum(element_in_list) && normal)
assoc_val = potential_cache[element_in_list]
//Check assoc entrys
if(assoc_val == src)
#ifdef REFERENCE_TRACKING_DEBUG
if(SSgarbage.should_save_refs)
found_refs[potential_cache] = TRUE
continue //End early, don't want these logging
#endif
log_reftracker("Found [type] [text_ref(src)] in list [container_name]\[[element_in_list]\]")
continue
//We need to run both of these checks, since our object could be hiding in either of them
//Check normal sublists
if(islist(element_in_list))
DoSearchVar(element_in_list, "[container_name] -> [element_in_list] (list)", recursive_limit - 1, search_time)
//Check assoc sublists
if(islist(assoc_val))
DoSearchVar(potential_container[element_in_list], "[container_name]\[[element_in_list]\] -> [assoc_val] (list)", recursive_limit - 1, search_time)

/proc/qdel_and_find_ref_if_fail(datum/thing_to_del, force = FALSE)
thing_to_del.qdel_and_find_ref_if_fail(force)
// This is dumb as hell I'm sorry
// I don't want the garbage subsystem to count as a ref for the purposes of this number
// If we find all other refs before it I want to early exit, and if we don't I want to keep searching past it
var/ignore_ref = FALSE
var/list/queues = SSgarbage.queues
for(var/list/queue in queues)
if(potential_cache in queue)
ignore_ref = TRUE
break
if(ignore_ref)
log_reftracker("[container_name] does not count as a ref for our count")
else
references_to_clear -= 1
if(references_to_clear == 0)
log_reftracker("All references to [type] [text_ref(src)] found, exiting.")
return

if(!isnum(element_in_list) && !is_special_list)
// This exists to catch an error that throws when we access a special list
// is_special_list is a hint, it can be wrong
try
var/assoc_val = potential_cache[element_in_list]
//Check assoc sublists
if(islist(assoc_val))
if(length(assoc_val))
DoSearchVar(potential_container[element_in_list], "[container_name]\[[element_in_list]\] -> [assoc_val] (list)", search_time, recursion_count + 1)
//Check assoc entry
else if(assoc_val == src)
#ifdef REFERENCE_TRACKING_DEBUG
if(SSgarbage.should_save_refs)
if(!found_refs)
found_refs = list()
found_refs[potential_cache] = TRUE
continue
else
log_reftracker("Found [type] [text_ref(src)] in list [container_name]\[[element_in_list]\]")
#else
log_reftracker("Found [type] [text_ref(src)] in list [container_name]\[[element_in_list]\]")
#endif
references_to_clear -= 1
if(references_to_clear == 0)
log_reftracker("All references to [type] [text_ref(src)] found, exiting.")
return
catch
// So if it goes wrong we kill it
is_special_list = TRUE
log_reftracker("Curiosity: [container_name] lead to an error when acessing [element_in_list], what is it?")

#undef REFSEARCH_RECURSE_LIMIT
#endif

/datum/proc/qdel_and_find_ref_if_fail(force = FALSE)
SSgarbage.reference_find_on_fail[text_ref(src)] = TRUE
qdel(src, force)
// Kept outside the ifdef so overrides are easy to implement

#endif
/// Return info about us for reference searching purposes
/// Will be logged as a representation of this datum if it's a part of a search chain
/datum/proc/ref_search_details()
return text_ref(src)

/datum/callback/ref_search_details()
return "[text_ref(src)] (obj: [object] proc: [delegate] args: [json_encode(arguments)] user: [user?.resolve() || "null"])"
Loading

0 comments on commit c00f89b

Please sign in to comment.