From e9a699c911c524393d947d6008bf0791b6bee947 Mon Sep 17 00:00:00 2001 From: Fluffy <65877598+FluffyGhoster@users.noreply.github.com> Date: Thu, 21 Mar 2024 20:02:55 +0100 Subject: [PATCH] Update reftracking (#18631) * Atomization * changelog * sdf --------- Co-authored-by: FluffyGhost --- code/controllers/subsystems/garbage.dm | 15 +- code/datums/datum.dm | 6 +- .../view_variables/reference_tracking.dm | 215 ++++++++++-------- .../FluffyGhost-update_reftracking.yml | 41 ++++ 4 files changed, 183 insertions(+), 94 deletions(-) create mode 100644 html/changelogs/FluffyGhost-update_reftracking.yml diff --git a/code/controllers/subsystems/garbage.dm b/code/controllers/subsystems/garbage.dm index 17eee1f305c..ce20a13f9c0 100644 --- a/code/controllers/subsystems/garbage.dm +++ b/code/controllers/subsystems/garbage.dm @@ -162,6 +162,9 @@ SUBSYSTEM_DEF(garbage) lastlevel = level +// 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)) @@ -179,9 +182,8 @@ SUBSYSTEM_DEF(garbage) 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 (refcount(D) == REFS_WE_EXPECT) ++gcedlasttick ++totalgcs pass_counts[level]++ @@ -202,12 +204,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) @@ -258,6 +263,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 diff --git a/code/datums/datum.dm b/code/datums/datum.dm index 51b85007e5a..7d422e90ce8 100644 --- a/code/datums/datum.dm +++ b/code/datums/datum.dm @@ -35,8 +35,12 @@ var/datum/weakref/weak_reference #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 diff --git a/code/modules/admin/view_variables/reference_tracking.dm b/code/modules/admin/view_variables/reference_tracking.dm index 1003778e610..86be28dd831 100644 --- a/code/modules/admin/view_variables/reference_tracking.dm +++ b/code/modules/admin/view_variables/reference_tracking.dm @@ -1,36 +1,29 @@ #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_subsystem_garbage_harddel("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_subsystem_garbage_harddel("Beginning search for references to a [type].") +/datum/proc/_search_references() + log_subsystem_garbage_harddel("Beginning search for references to a [type], looking for [references_to_clear] refs.") var/starting_time = world.time - - log_subsystem_garbage_harddel("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_subsystem_garbage_harddel("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 @@ -38,45 +31,46 @@ 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_subsystem_garbage_harddel("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_subsystem_garbage_harddel("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_subsystem_garbage_harddel("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_subsystem_garbage_harddel("Finished searching clients") + if(src.references_to_clear == 0) + return #endif log_subsystem_garbage_harddel("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_subsystem_garbage_harddel("Recursion limit reached. [container_name]") return - if(!recursive_limit) - log_subsystem_garbage_harddel("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 @@ -90,73 +84,116 @@ return datum_container.last_find_references = search_time - var/container_print = datum_container.ref_search_details() 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_subsystem_garbage_harddel("Found [type] [text_ref(src)] in [datum_container.type]'s [datum_container.ref_search_details()] [varname] var. [container_name]") + #else + log_subsystem_garbage_harddel("Found [type] [text_ref(src)] in [datum_container.type]'s [datum_container.ref_search_details()] [varname] var. [container_name]") #endif - log_subsystem_garbage_harddel("Found [type] [text_ref(src)] in [datum_container.type]'s [container_print] [varname] var. [container_name]") + references_to_clear -= 1 + if(references_to_clear == 0) + log_subsystem_garbage_harddel("All references to [type] [text_ref(src)] found, exiting.") + return continue - if(islist(variable)) - DoSearchVar(variable, "[container_name] [container_print] -> [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 entrys - if(element_in_list == 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_subsystem_garbage_harddel("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_subsystem_garbage_harddel("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) + if(length(element_in_list)) + DoSearchVar(element_in_list, "[container_name] -> [element_in_list] (list)", search_time, recursion_count + 1) + //Check normal entrys + 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 + else + log_subsystem_garbage_harddel("Found [type] [text_ref(src)] in list [container_name].") + #else + log_subsystem_garbage_harddel("Found [type] [text_ref(src)] in list [container_name].") + #endif -/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_subsystem_garbage_harddel("[container_name] does not count as a ref for our count") + else + references_to_clear -= 1 + if(references_to_clear == 0) + log_subsystem_garbage_harddel("All references to [type] [text_ref(src)] found, exiting.") + return -/datum/proc/qdel_and_find_ref_if_fail(force = FALSE) - SSgarbage.reference_find_on_fail[text_ref(src)] = TRUE - qdel(src, force) + 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_subsystem_garbage_harddel("Found [type] [text_ref(src)] in list [container_name]\[[element_in_list]\]") + #else + log_subsystem_garbage_harddel("Found [type] [text_ref(src)] in list [container_name]\[[element_in_list]\]") + #endif + references_to_clear -= 1 + if(references_to_clear == 0) + log_subsystem_garbage_harddel("All references to [type] [text_ref(src)] found, exiting.") + return + catch + // So if it goes wrong we kill it + is_special_list = TRUE + log_subsystem_garbage_harddel("Curiosity: [container_name] lead to an error when acessing [element_in_list], what is it?") +#undef REFSEARCH_RECURSE_LIMIT #endif // Kept outside the ifdef so overrides are easy to implement diff --git a/html/changelogs/FluffyGhost-update_reftracking.yml b/html/changelogs/FluffyGhost-update_reftracking.yml new file mode 100644 index 00000000000..dd6b934173d --- /dev/null +++ b/html/changelogs/FluffyGhost-update_reftracking.yml @@ -0,0 +1,41 @@ +################################ +# Example Changelog File +# +# Note: This file, and files beginning with ".", and files that don't end in ".yml" will not be read. If you change this file, you will look really dumb. +# +# Your changelog will be merged with a master changelog. (New stuff added only, and only on the date entry for the day it was merged.) +# When it is, any changes listed below will disappear. +# +# Valid Prefixes: +# bugfix +# wip (For works in progress) +# tweak +# soundadd +# sounddel +# rscadd (general adding of nice things) +# rscdel (general deleting of nice things) +# imageadd +# imagedel +# maptweak +# spellcheck (typo fixes) +# experiment +# balance +# admin +# backend +# security +# refactor +################################# + +# Your name. +author: FluffyGhost + +# Optional: Remove this file after generating master changelog. Useful for PR changelogs that won't get used again. +delete-after: True + +# Any changes you've made. See valid prefix list above. +# INDENT WITH TWO SPACES. NOT TABS. SPACES. +# SCREW THIS UP AND IT WON'T WORK. +# Also, all entries are changed into a single [] after a master changelog generation. Just remove the brackets when you add new entries. +# Please surround your changes in double quotes ("), as certain characters otherwise screws up compiling. The quotes will not show up in the changelog. +changes: + - backend: "Updated the reference tracker, it should now be faster."