From 3bb64112e207c93f3874c82a4bfcc4955834fca7 Mon Sep 17 00:00:00 2001 From: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com> Date: Thu, 11 Nov 2021 13:49:21 -0800 Subject: [PATCH] Ref tracker TLC, makes unit test harddel errors more descriptive (#62725) --- code/__HELPERS/_logging.dm | 2 +- code/_compile_options.dm | 6 +++--- .../view_variables/reference_tracking.dm | 16 ++++++++++++-- code/modules/unit_tests/create_and_destroy.dm | 4 ++-- .../unit_tests/find_reference_sanity.dm | 21 +++++++++++++++++++ 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/code/__HELPERS/_logging.dm b/code/__HELPERS/_logging.dm index e93d654b5e8..426577ce801 100644 --- a/code/__HELPERS/_logging.dm +++ b/code/__HELPERS/_logging.dm @@ -57,7 +57,7 @@ GLOBAL_LIST_INIT(testing_global_profiler, list("_PROFILE_NAME" = "Global")) SEND_TEXT(world.log, text) #endif -#ifdef REFERENCE_TRACKING_LOG +#ifdef REFERENCE_TRACKING #define log_reftracker(msg) log_world("## REF SEARCH [msg]") #else #define log_reftracker(msg) diff --git a/code/_compile_options.dm b/code/_compile_options.dm index 83038d30137..7fee122655e 100644 --- a/code/_compile_options.dm +++ b/code/_compile_options.dm @@ -15,15 +15,14 @@ //#define REFERENCE_TRACKING #ifdef REFERENCE_TRACKING -///Should we be logging our findings or not -#define REFERENCE_TRACKING_LOG - ///Used for doing dry runs of the reference finder, to test for feature completeness +///Slightly slower, higher in memory. Just not optimal //#define REFERENCE_TRACKING_DEBUG ///Run a lookup on things hard deleting by default. //#define GC_FAILURE_HARD_LOOKUP #ifdef GC_FAILURE_HARD_LOOKUP +///Don't stop when searching, go till you're totally done #define FIND_REF_NO_CHECK_TICK #endif //ifdef GC_FAILURE_HARD_LOOKUP @@ -79,6 +78,7 @@ #define REFERENCE_TRACKING #define REFERENCE_TRACKING_DEBUG #define FIND_REF_NO_CHECK_TICK +#define GC_FAILURE_HARD_LOOKUP #endif #ifdef TGS diff --git a/code/modules/admin/view_variables/reference_tracking.dm b/code/modules/admin/view_variables/reference_tracking.dm index 40809719392..dcc093c5c90 100644 --- a/code/modules/admin/view_variables/reference_tracking.dm +++ b/code/modules/admin/view_variables/reference_tracking.dm @@ -27,9 +27,18 @@ var/starting_time = world.time //Time to search the whole game for our ref - DoSearchVar(GLOB, "GLOB") //globals + DoSearchVar(GLOB, "GLOB", search_time = starting_time) //globals log_reftracker("Finished searching globals") + //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) + log_reftracker("Finished searching native globals") + for(var/datum/thing in world) //atoms (don't beleive its lies) DoSearchVar(thing, "World -> [thing.type]", search_time = starting_time) log_reftracker("Finished searching atoms") @@ -55,7 +64,7 @@ /datum/proc/DoSearchVar(potential_container, container_name, recursive_limit = 64, search_time = world.time) #ifdef REFERENCE_TRACKING_DEBUG - if(!found_refs && SSgarbage.should_save_refs) + if(SSgarbage.should_save_refs && !found_refs) found_refs = list() #endif @@ -91,6 +100,7 @@ #ifdef REFERENCE_TRACKING_DEBUG if(SSgarbage.should_save_refs) found_refs[varname] = TRUE + continue //End early, don't want these logging #endif log_reftracker("Found [type] \ref[src] in [datum_container.type]'s \ref[datum_container] [varname] var. [container_name]") continue @@ -110,6 +120,7 @@ #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] \ref[src] in list [container_name].") continue @@ -122,6 +133,7 @@ #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] \ref[src] in list [container_name]\[[element_in_list]\]") continue diff --git a/code/modules/unit_tests/create_and_destroy.dm b/code/modules/unit_tests/create_and_destroy.dm index cfddffd643b..01e197a42a5 100644 --- a/code/modules/unit_tests/create_and_destroy.dm +++ b/code/modules/unit_tests/create_and_destroy.dm @@ -144,8 +144,8 @@ garbage_queue_processed = TRUE break - if(world.time > start_time + time_needed + 8 MINUTES) - Fail("Something has gone horribly wrong, the garbage queue has been processing for well over 10 minutes. What the hell did you do") + if(world.time > start_time + time_needed + 30 MINUTES) //If this gets us gitbanned I'm going to laugh so hard + Fail("Something has gone horribly wrong, the garbage queue has been processing for well over 30 minutes. What the hell did you do") break //Immediately fire the gc right after diff --git a/code/modules/unit_tests/find_reference_sanity.dm b/code/modules/unit_tests/find_reference_sanity.dm index f41714f0659..67b6072d3b9 100644 --- a/code/modules/unit_tests/find_reference_sanity.dm +++ b/code/modules/unit_tests/find_reference_sanity.dm @@ -2,12 +2,14 @@ /datum/unit_test/find_reference_sanity /atom/movable/ref_holder + var/static/atom/movable/ref_test/static_test var/atom/movable/ref_test/test var/list/test_list = list() var/list/test_assoc_list = list() /atom/movable/ref_holder/Destroy() test = null + static_test = null test_list.Cut() test_assoc_list.Cut() return ..() @@ -109,3 +111,22 @@ TEST_ASSERT(victim.found_refs[to_find_in_key], "The ref-tracking tool failed to find a nested assoc list key") TEST_ASSERT(victim.found_refs[to_find_null_assoc_nested], "The ref-tracking tool failed to find a null key'd nested assoc list entry") SSgarbage.should_save_refs = FALSE + +/datum/unit_test/find_reference_static_investigation/Run() + var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) + var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) + SSgarbage.should_save_refs = TRUE + + //Lets check static vars now, since those can be a real headache + testbed.static_test = victim + + //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] + + victim.DoSearchVar(global_vars, "Sixth Run", search_time = 7) + + TEST_ASSERT(victim.found_refs[global_vars], "The ref-tracking tool failed to find a natively global variable") + SSgarbage.should_save_refs = FALSE