From 963516769c54831f2f4cd8fbd21d18bca7c70955 Mon Sep 17 00:00:00 2001 From: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com> Date: Sat, 5 Nov 2022 04:19:27 -0700 Subject: [PATCH] Changes how the garbage subsystem queue stores and processes info (#19608) The way """we""" currently do it, each time you want to walk the queue you're forced to make a copy in memory of the whole thing There's no real reason to want this, so it seems best to just avoid it entirely. It creates a TON of usage for no reason, and also risks a lot of overtime since you can't really batch a list copy like that. So instead let's just iterate over the length of the queue, constant rather then O(N) time. Similarly, rather then using an associated list in the form queue[ref] = gc time, we could store queue entries in what amounts to a tuple. This means no associated list stuff, so the operation of queuing becomes cheaper, and pulling gc time similarly goes from O(log n) to constant time I stole this work from myself and mso, tg pr 55595 I'm pring it here because I keep seeing affected complain about the garbage subsystem and he refuses to do it himself. No I don't have an ego problem I swear --- code/controllers/subsystem/garbage.dm | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/code/controllers/subsystem/garbage.dm b/code/controllers/subsystem/garbage.dm index a8a673aac1e..79fd1d11863 100644 --- a/code/controllers/subsystem/garbage.dm +++ b/code/controllers/subsystem/garbage.dm @@ -136,18 +136,24 @@ SUBSYSTEM_DEF(garbage) lastlevel = level - for(var/refID in queue) - if(!refID) + // The instinct is to use a for in loop here, to walk the entries in the queue + // The trouble is this performs a copy of the queue list, and since this can in theory balloon a LOT + // It's better to just go index by index. It's not a huge deal but it's worth doin IMO + for(var/i in 1 to length(queue)) + var/list/packet = queue[i] + if(length(packet) != 2) count++ if(MC_TICK_CHECK) return continue - var/GCd_at_time = queue[refID] + var/GCd_at_time = packet[2] if(GCd_at_time > cut_off_time) break // Everything else is newer, skip them count++ + var/refID = packet[1] + var/datum/D D = locate(refID) @@ -212,14 +218,12 @@ SUBSYSTEM_DEF(garbage) HardDelete(D) return var/gctime = world.time - var/refid = "\ref[D]" D.gc_destroyed = gctime - var/list/queue = queues[level] - if(queue[refid]) - queue -= refid // Removing any previous references that were GC'd so that the current object will be at the end of the list. - queue[refid] = gctime + var/list/queue = queues[level] + // I hate byond lists so much man + queue[++queue.len] = list("\ref[D]", gctime) //this is mainly to separate things profile wise. /datum/controller/subsystem/garbage/proc/HardDelete(datum/D)