[MIRROR] Experiment with holding hard references to objects being qdeleted in 515 (saves 1.1+ seconds on init times, more on prod) [MDB IGNORE] (#18361)

* Experiment with holding hard references to objects being qdeleted in 515 (saves 1.1+ seconds on init times, more on prod) (#72033)

## About The Pull Request
Adds `EXPERIMENT_515_QDEL_HARD_REFERENCE`, which will queue to the GC
subsystem using hard references rather than `\ref`. This is only
possible in 515 because of the new `refcount` proc. `\ref` is very very
slow and has some nasty knock on effects, so removing its usages where
possible is good.

This is an explicit opt in define because I want to give us the ability
to test 515 on live while only testing 515 itself, not our experimental
changes. We have a few more of these we want to do so I made a separate
file for them. They're auto-defined in unit tests so we see them with
the alternate test runner. In a perfect world we'd test both on and off,
but eh.

Closes https://github.com/tgstation/dev-cycles-initiative/issues/10

* Experiment with holding hard references to objects being qdeleted in 515 (saves 1.1+ seconds on init times, more on prod)

* fix missed underbarrels

* HEV radio

* Keeps gc_destroyed from getting updated on every step thru the gc queue. (#72401)

Keeps gc_destroyed from getting updated on every step thru the gc queue.

Fixes logic that assumed gc_destroyed is the time the object first
qdel'ed. it used to get updated on each stage of the garbage controller
and there are 3 stages.

Added list index defines for the inner gc item list.

* test fix

* final fix

Co-authored-by: Mothblocks <35135081+Mothblocks@users.noreply.github.com>
Co-authored-by: tastyfish <crazychris32@gmail.com>
Co-authored-by: Kyle Spier-Swenson <kyleshome@gmail.com>

code/__DEFINES/qdel.dm

@@ -30,6 +30,13 @@
 #define GC_QUEUE_HARDDELETE 3 //! short queue for things that hard delete instead of going thru the gc subsystem, this is purely so if they *can* softdelete, they will soft delete rather then wasting time with a hard delete.
 #define GC_QUEUE_COUNT 3 //! Number of queues, used for allocating the nested lists. Don't forget to increase this if you add a new queue stage
 
+
+// Defines for the ssgarbage queue items
+#define GC_QUEUE_ITEM_QUEUE_TIME 1 //! Time this item entered the queue
+#define GC_QUEUE_ITEM_REF 2 //! Ref to the item
+#define GC_QUEUE_ITEM_GCD_DESTROYED 3 //! Item's gc_destroyed var value. Used to detect ref reuse.
+#define GC_QUEUE_ITEM_INDEX_COUNT 3 //! Number of item indexes, used for allocating the nested lists. Don't forget to increase this if you add a new queue item index
+
 // Defines for the time an item has to get its reference cleaned before it fails the queue and moves to the next.
 #define GC_FILTER_QUEUE (1 SECONDS)
 #define GC_CHECK_QUEUE (5 MINUTES)

code/_experiments.dm

@@ -0,0 +1,21 @@
+// This file contains experimental flags that may not be production ready yet,
+// but that we want to be able to easily flip as well as run on CI.
+// Any flag you see here can be flipped with the `-D` CLI argument.
+// For example, if you want to enable EXPERIMENT_MY_COOL_FEATURE, compile with -DEXPERIMENT_MY_COOL_FEATURE
+
+// EXPERIMENT_515_QDEL_HARD_REFERENCE
+// - On 515, will hold a hard reference for qdeleted items, and check ref_count, rather than using refs.
+
+#if DM_VERSION < 515
+
+// You can't X-macro custom names :(
+#ifdef EXPERIMENT_515_QDEL_HARD_REFERENCE
+#warn EXPERIMENT_515_QDEL_HARD_REFERENCE is only available on 515+
+#undef EXPERIMENT_515_QDEL_HARD_REFERENCE
+#endif
+
+#elif defined(UNIT_TESTS)
+
+#define EXPERIMENT_515_QDEL_HARD_REFERENCE
+
+#endif

code/controllers/subsystem/atoms.dm

@@ -22,6 +22,8 @@ SUBSYSTEM_DEF(atoms)
 	/// Atoms that will be deleted once the subsystem is initialized
 	var/list/queued_deletions = list()
 
+	var/init_start_time
+
 	#ifdef PROFILE_MAPLOAD_INIT_ATOM
 	var/list/mapload_init_times = list()
 	#endif
@@ -29,6 +31,7 @@ SUBSYSTEM_DEF(atoms)
 	initialized = INITIALIZATION_INSSATOMS
 
 /datum/controller/subsystem/atoms/Initialize()
+	init_start_time = world.time
 	setupGenetics() //to set the mutations' sequence
 
 	initialized = INITIALIZATION_INNEW_MAPLOAD
@@ -122,8 +125,11 @@ SUBSYSTEM_DEF(atoms)
 /// Init this specific atom
 /datum/controller/subsystem/atoms/proc/InitAtom(atom/A, from_template = FALSE, list/arguments)
 	var/the_type = A.type
+
 	if(QDELING(A))
-		BadInitializeCalls[the_type] |= BAD_INIT_QDEL_BEFORE
+		// Check init_start_time to not worry about atoms created before the atoms SS that are cleaned up before this
+		if (A.gc_destroyed > init_start_time)
+			BadInitializeCalls[the_type] |= BAD_INIT_QDEL_BEFORE
 		return TRUE
 
 	// This is handled and battle tested by dreamchecker. Limit to UNIT_TESTS just in case that ever fails.

code/controllers/subsystem/garbage.dm

@@ -139,6 +139,13 @@ SUBSYSTEM_DEF(garbage)
 			pass_counts[i] = 0
 			fail_counts[i] = 0
 
+#ifdef EXPERIMENT_515_QDEL_HARD_REFERENCE
+// 1 from the hard reference in the queue, and 1 from the variable used before this
+#define IS_DELETED(datum, _) (refcount(##datum) == 2)
+#else
+#define IS_DELETED(datum, gcd_at_time) (isnull(##datum) || ##datum.gc_destroyed != gcd_at_time)
+#endif
+
 /datum/controller/subsystem/garbage/proc/HandleQueue(level = GC_QUEUE_FILTER)
 	if (level == GC_QUEUE_FILTER)
 		delslasttick = 0
@@ -159,26 +166,32 @@ SUBSYSTEM_DEF(garbage)
 	//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]
-		if (length(L) < 2)
+		if (length(L) < GC_QUEUE_ITEM_INDEX_COUNT)
 			count++
 			if (MC_TICK_CHECK)
 				return
 			continue
 
-		var/GCd_at_time = L[1]
-		if(GCd_at_time > cut_off_time)
+		var/queued_at_time = L[GC_QUEUE_ITEM_QUEUE_TIME]
+		var/GCd_at_time = L[GC_QUEUE_ITEM_GCD_DESTROYED]
+		if(queued_at_time > cut_off_time)
 			break // Everything else is newer, skip them
 		count++
-		var/refID = L[2]
+
+#ifdef EXPERIMENT_515_QDEL_HARD_REFERENCE
+		var/datum/D = L[GC_QUEUE_ITEM_REF]
+#else
+		var/refID = L[GC_QUEUE_ITEM_REF]
 		var/datum/D
 		D = locate(refID)
+#endif
 
-		if (!D || D.gc_destroyed != GCd_at_time) // So if something else coincidently gets the same ref, it's not deleted by mistake
+		if (IS_DELETED(D, GCd_at_time)) // So if something else coincidently gets the same ref, it's not deleted by mistake
 			++gcedlasttick
 			++totalgcs
 			pass_counts[level]++
 			#ifdef REFERENCE_TRACKING
-			reference_find_on_fail -= refID //It's deleted we don't care anymore.
+			reference_find_on_fail -= text_ref(D) //It's deleted we don't care anymore.
 			#endif
 			if (MC_TICK_CHECK)
 				return
@@ -194,7 +207,7 @@ SUBSYSTEM_DEF(garbage)
 		switch (level)
 			if (GC_QUEUE_CHECK)
 				#ifdef REFERENCE_TRACKING
-				if(reference_find_on_fail[refID])
+				if(reference_find_on_fail[text_ref(D)])
 					INVOKE_ASYNC(D, TYPE_PROC_REF(/datum,find_references))
 					ref_searching = TRUE
 				#ifdef GC_FAILURE_HARD_LOOKUP
@@ -202,12 +215,17 @@ SUBSYSTEM_DEF(garbage)
 					INVOKE_ASYNC(D, TYPE_PROC_REF(/datum,find_references))
 					ref_searching = TRUE
 				#endif
-				reference_find_on_fail -= refID
+				reference_find_on_fail -= text_ref(D)
 				#endif
 				var/type = D.type
 				var/datum/qdel_item/I = items[type]
 
-				log_world("## TESTING: GC: -- [text_ref(D)] | [type] was unable to be GC'd --")
+				var/message = "## TESTING: GC: -- [text_ref(D)] | [type] was unable to be GC'd --"
+#if DM_VERSION >= 515
+				message = "[message] (ref count of [refcount(D)])"
+#endif
+				log_world(message)
+
 				#ifdef TESTING
 				for(var/c in GLOB.admins) //Using testing() here would fill the logs with ADMIN_VV garbage
 					var/client/admin = c
@@ -242,19 +260,27 @@ SUBSYSTEM_DEF(garbage)
 		queue.Cut(1,count+1)
 		count = 0
 
+#undef IS_DELETED
+
 /datum/controller/subsystem/garbage/proc/Queue(datum/D, level = GC_QUEUE_FILTER)
 	if (isnull(D))
 		return
 	if (level > GC_QUEUE_COUNT)
 		HardDelete(D)
 		return
-	var/gctime = world.time
-	var/refid = text_ref(D)
+	var/queue_time = world.time
 
-	D.gc_destroyed = gctime
+#ifdef EXPERIMENT_515_QDEL_HARD_REFERENCE
+	var/refid = D
+#else
+	var/refid = text_ref(D)
+#endif
+	if (D.gc_destroyed <= 0)
+		D.gc_destroyed = queue_time
+	
 	var/list/queue = queues[level]
 
-	queue[++queue.len] = list(gctime, refid) // not += for byond reasons
+	queue[++queue.len] = list(queue_time, refid, D.gc_destroyed) // not += for byond reasons
 
 //this is mainly to separate things profile wise.
 /datum/controller/subsystem/garbage/proc/HardDelete(datum/D)

code/game/machinery/buttons.dm

@@ -49,6 +49,12 @@
 
 	setup_device()
 
+/obj/machinery/button/Destroy()
+	// Let them dump on the ground.
+	device = null
+	board = null
+	return ..()
+
 /obj/machinery/button/update_icon_state()
 	if(panel_open)
 		icon_state = "button-open"

code/game/objects/items/tanks/watertank.dm

@@ -144,6 +144,10 @@
 		return INITIALIZE_HINT_QDEL
 	reagents = tank.reagents //This mister is really just a proxy for the tank's reagents
 
+/obj/item/reagent_containers/spray/mister/Destroy(force)
+	tank = null
+	return ..()
+
 /obj/item/reagent_containers/spray/mister/afterattack(obj/target, mob/user, proximity)
 	if(target.loc == loc) //Safety check so you don't fill your mister with mutagen or something and then blast yourself in the face with it
 		return

code/modules/clothing/chameleon.dm

@@ -404,6 +404,10 @@
 	chameleon_action.initialize_disguises()
 	add_item_action(chameleon_action)
 
+/obj/item/clothing/under/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/clothing/under/chameleon/emp_act(severity)
 	. = ..()
 	if(. & EMP_PROTECT_SELF)
@@ -445,6 +449,10 @@
 	chameleon_action.initialize_disguises()
 	add_item_action(chameleon_action)
 
+/obj/item/clothing/suit/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/clothing/suit/chameleon/emp_act(severity)
 	. = ..()
 	if(. & EMP_PROTECT_SELF)
@@ -482,6 +490,10 @@
 	chameleon_action.initialize_disguises()
 	add_item_action(chameleon_action)
 
+/obj/item/clothing/glasses/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/clothing/glasses/chameleon/emp_act(severity)
 	. = ..()
 	if(. & EMP_PROTECT_SELF)
@@ -521,6 +533,10 @@
 	chameleon_action.initialize_disguises()
 	add_item_action(chameleon_action)
 
+/obj/item/clothing/gloves/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/clothing/gloves/chameleon/emp_act(severity)
 	. = ..()
 	if(. & EMP_PROTECT_SELF)
@@ -558,6 +574,10 @@
 	chameleon_action.initialize_disguises()
 	add_item_action(chameleon_action)
 
+/obj/item/clothing/head/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/clothing/head/chameleon/emp_act(severity)
 	. = ..()
 	if(. & EMP_PROTECT_SELF)
@@ -625,6 +645,10 @@
 	chameleon_action.initialize_disguises()
 	add_item_action(chameleon_action)
 
+/obj/item/clothing/mask/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/clothing/mask/chameleon/emp_act(severity)
 	. = ..()
 	if(. & EMP_PROTECT_SELF)
@@ -698,6 +722,10 @@
 	chameleon_action.initialize_disguises()
 	add_item_action(chameleon_action)
 
+/obj/item/clothing/shoes/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/clothing/shoes/chameleon/emp_act(severity)
 	. = ..()
 	if(. & EMP_PROTECT_SELF)
@@ -746,6 +774,10 @@
 	chameleon_action.initialize_disguises()
 	add_item_action(chameleon_action)
 
+/obj/item/storage/backpack/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/storage/backpack/chameleon/emp_act(severity)
 	. = ..()
 	if(. & EMP_PROTECT_SELF)
@@ -783,6 +815,10 @@
 
 	atom_storage.silent = TRUE
 
+/obj/item/storage/belt/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/storage/belt/chameleon/emp_act(severity)
 	. = ..()
 	if(. & EMP_PROTECT_SELF)
@@ -816,6 +852,10 @@
 	chameleon_action.initialize_disguises()
 	add_item_action(chameleon_action)
 
+/obj/item/radio/headset/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/radio/headset/chameleon/emp_act(severity)
 	. = ..()
 	if(. & EMP_PROTECT_SELF)
@@ -847,6 +887,10 @@
 	chameleon_action.initialize_disguises()
 	add_item_action(chameleon_action)
 
+/obj/item/modular_computer/pda/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/modular_computer/pda/chameleon/emp_act(severity)
 	. = ..()
 	if(. & EMP_PROTECT_SELF)
@@ -876,6 +920,10 @@
 	chameleon_action.initialize_disguises()
 	add_item_action(chameleon_action)
 
+/obj/item/stamp/chameleon/Destroy()
+	QDEL_NULL(chameleon_action)
+	return ..()
+
 /obj/item/stamp/chameleon/broken/Initialize(mapload)
 	. = ..()
 	chameleon_action.emp_randomise(INFINITY)
@@ -952,8 +1000,9 @@
 	set_chameleon_disguise(/obj/item/gun/energy/laser)
 
 /obj/item/gun/energy/laser/chameleon/Destroy()
-	. = ..()
 	chameleon_projectile_vars.Cut()
+	QDEL_NULL(chameleon_action)
+	return ..()
 
 /obj/item/gun/energy/laser/chameleon/emp_act(severity)
 	return

code/modules/mob/living/silicon/robot/robot.dm

@@ -150,6 +150,7 @@
 	QDEL_NULL(hands)
 	QDEL_NULL(spark_system)
 	QDEL_NULL(alert_control)
+	QDEL_LIST(upgrades)
 	cell = null
 	return ..()
 

code/modules/mob/living/silicon/silicon.dm

@@ -76,6 +76,7 @@
 	QDEL_NULL(builtInCamera)
 	laws?.owner = null //Laws will refuse to die otherwise.
 	QDEL_NULL(laws)
+	QDEL_NULL(modularInterface)
 	GLOB.silicon_mobs -= src
 	return ..()
 

code/modules/mob/living/simple_animal/heretic_monsters.dm

@@ -261,6 +261,7 @@
 		front.icon_state = "armsy_end"
 		front.icon_living = "armsy_end"
 		front.back = null
+		front = null
 	if(back)
 		QDEL_NULL(back) // chain destruction baby
 	return ..()

code/modules/mob/living/simple_animal/parrot.dm

@@ -134,6 +134,10 @@
 						/obj/item/radio/headset/headset_cargo)
 		ears = new headset(src)
 
+/mob/living/simple_animal/parrot/Destroy()
+	QDEL_NULL(ears)
+	return ..()
+
 /mob/living/simple_animal/parrot/examine(mob/user)
 	. = ..()
 	if(stat)

code/modules/pai/death.dm

@@ -9,4 +9,11 @@
 	* life. This new mind causes problems down the line if it's not deleted here.
 	*/
 	ghostize()
+
+	if (!QDELETED(card) && loc != card)
+		card.forceMove(drop_location())
+		card.pai = null
+		card.emotion_icon = initial(card.emotion_icon)
+		card.update_appearance()
+
 	qdel(src)

code/modules/pai/pai.dm

@@ -159,12 +159,7 @@
 	QDEL_NULL(internal_gps)
 	QDEL_NULL(newscaster)
 	QDEL_NULL(signaler)
-	if(!QDELETED(card) && loc != card)
-		card.forceMove(drop_location())
-		// these are otherwise handled by paicard/handle_atom_del()
-		card.pai = null
-		card.emotion_icon = initial(card.emotion_icon)
-		card.update_appearance()
+	card = null
 	GLOB.pai_list.Remove(src)
 	return ..()
 

code/modules/projectiles/guns/ballistic.dm

@@ -133,6 +133,10 @@
 	update_appearance()
 	RegisterSignal(src, COMSIG_ITEM_RECHARGED, PROC_REF(instant_reload))
 
+/obj/item/gun/ballistic/Destroy()
+	QDEL_NULL(magazine)
+	return ..()
+
 /obj/item/gun/ballistic/add_weapon_description()
 	AddElement(/datum/element/weapon_description, attached_proc = PROC_REF(add_notes_ballistic))
 

code/modules/projectiles/guns/ballistic/shotgun.dm

@@ -97,6 +97,10 @@
 	alt_mag_type = alt_mag_type || mag_type
 	alternate_magazine = new alt_mag_type(src)
 
+/obj/item/gun/ballistic/shotgun/automatic/dual_tube/Destroy()
+	QDEL_NULL(alternate_magazine)
+	return ..()
+
 /obj/item/gun/ballistic/shotgun/automatic/dual_tube/attack_self(mob/living/user)
 	if(!chambered && magazine.contents.len)
 		rack()
@@ -158,6 +162,10 @@
 	secondary_magazine = new secondary_magazine_type(src)
 	update_appearance()
 
+/obj/item/gun/ballistic/shotgun/bulldog/Destroy()
+	QDEL_NULL(secondary_magazine)
+	return ..()
+
 /obj/item/gun/ballistic/shotgun/bulldog/examine(mob/user)
 	. = ..()
 	if(secondary_magazine)

code/modules/projectiles/guns/energy.dm

@@ -128,6 +128,13 @@
 	if (cell)
 		QDEL_NULL(cell)
 	STOP_PROCESSING(SSobj, src)
+
+	// Intentional cast.
+	// Sometimes ammo_type has paths, sometimes it has atom.
+	for (var/atom/item in ammo_type)
+		qdel(item)
+		ammo_type -= item
+
 	return ..()
 
 /obj/item/gun/energy/handle_atom_del(atom/A)

modular_skyrat/modules/hev_suit/code/hev_suit.dm

@@ -208,8 +208,7 @@
 	current_user = null
 
 /obj/item/clothing/suit/space/hev_suit/Destroy()
-	if(internal_radio)
-		qdel(internal_radio)
+	QDEL_NULL(internal_radio)
 	if(current_internals_tank)
 		REMOVE_TRAIT(current_internals_tank, TRAIT_NODROP, "hev_trait")
 		current_internals_tank = null

modular_skyrat/modules/marines/code/gear.dm

@@ -77,6 +77,10 @@
 	underbarrel = new /obj/item/gun/ballistic/shotgun/automatic/as2/ubsg(src)
 	update_appearance()
 
+/obj/item/gun/ballistic/automatic/ar/modular/m44a/shotgun/Destroy()
+	QDEL_NULL(underbarrel)
+	return ..()
+
 /obj/item/gun/ballistic/automatic/ar/modular/m44a/shotgun/afterattack_secondary(atom/target, mob/living/user, flag, params)
 	underbarrel.afterattack(target, user, flag, params)
 	return SECONDARY_ATTACK_CONTINUE_CHAIN
@@ -101,6 +105,10 @@
 	underbarrel = new /obj/item/gun/ballistic/revolver/grenadelauncher/unrestricted(src)
 	update_appearance()
 
+/obj/item/gun/ballistic/automatic/ar/modular/m44a/grenadelauncher/Destroy()
+	QDEL_NULL(underbarrel)
+	return ..()
+
 /obj/item/gun/ballistic/automatic/ar/modular/m44a/grenadelauncher/afterattack_secondary(atom/target, mob/living/user, flag, params)
 	underbarrel.afterattack(target, user, flag, params)
 	return SECONDARY_ATTACK_CONTINUE_CHAIN

tgstation.dme

@@ -17,6 +17,7 @@
 #include "code\__byond_version_compat.dm"
 #include "code\_compile_options.dm"
 #include "code\_debugger.dm"
+#include "code\_experiments.dm"
 #include "code\world.dm"
 #include "code\__DEFINES\_bitfields.dm"
 #include "code\__DEFINES\_click.dm"