From 94cb4e0ce439d7f7f99013aeec56dc1f52ab5c19 Mon Sep 17 00:00:00 2001 From: SkyratBot <59378654+SkyratBot@users.noreply.github.com> Date: Sun, 29 Nov 2020 22:28:18 +0100 Subject: [PATCH] [MIRROR] sstimer no longer batches maintenance tasks to the bucket list to avoid edge cases and duplicated logic. (#1933) * sstimer no longer batches maintenance tasks to the bucket list to avoid edge cases and duplicated logic. (#55140) * sstimer no longer delays maintenance tasks if its going over its tick. This was leading to bugs if certain state operations happened while a task was delayed, furthermore if the timer subsystem was overloaded, the invoked timers list would bloat as it would never get cleared out, which would make all timer invocations take longer as they had to add to an ever growing list. * Update timer.dm * Fix error when a bucket has only one timer. * simply timer loop logic & improve timer debug string It would try to batch up linked list modifications and every issue we have ever had has been related to this, so now it just directly pulls the head of the linked list off, using bucketEject, rather then detect when it reaches the end of the linked list queue it will now just know because the bucket will be empty. All bucketCount logic has been moved to bucketEject and bucketJoin(), which should also keep that more proper. * Update timer.dm * Update timer.dm * sstimer no longer batches maintenance tasks to the bucket list to avoid edge cases and duplicated logic. Co-authored-by: Kyle Spier-Swenson --- code/controllers/subsystem/timer.dm | 140 +++++++++------------------- 1 file changed, 46 insertions(+), 94 deletions(-) diff --git a/code/controllers/subsystem/timer.dm b/code/controllers/subsystem/timer.dm index 13a18efb25a..ede03433453 100644 --- a/code/controllers/subsystem/timer.dm +++ b/code/controllers/subsystem/timer.dm @@ -149,112 +149,64 @@ SUBSYSTEM_DEF(timer) bucket_list = src.bucket_list resumed = FALSE - var/static/list/invoked_timers = list() - var/static/datum/timedevent/timer - if (!resumed) - timer = null // Iterate through each bucket starting from the practical offset while (practical_offset <= BUCKET_LEN && head_offset + ((practical_offset - 1) * world.tick_lag) <= world.time) - var/datum/timedevent/head = bucket_list[practical_offset] - if (!timer || !head || timer == head) - head = bucket_list[practical_offset] - timer = head - - // Iterate through each timer, invoking callbacks as necessary - while (timer) + var/datum/timedevent/timer + while ((timer = bucket_list[practical_offset])) var/datum/callback/callBack = timer.callBack if (!callBack) bucket_resolution = null // force bucket recreation CRASH("Invalid timer: [get_timer_debug_string(timer)] world.time: [world.time], \ head_offset: [head_offset], practical_offset: [practical_offset]") - + + timer.bucketEject() //pop the timer off of the bucket list. + // Invoke callback if possible if (!timer.spent) - invoked_timers += timer timer.spent = world.time callBack.InvokeAsync() last_invoke_tick = world.time - + + if (timer.flags & TIMER_LOOP) // Prepare looping timers to re-enter the queue + timer.spent = 0 + timer.timeToRun = world.time + timer.wait + timer.bucketJoin() + else + qdel(timer) + if (MC_TICK_CHECK) - return - - // Break once we've invoked the entire bucket - timer = timer.next - if (timer == head) break + + if (!bucket_list[practical_offset]) + // Empty the bucket, check if anything in the secondary queue should be shifted to this bucket + bucket_list[practical_offset++] = null + var/i = 0 + for (i in 1 to length(second_queue)) + timer = second_queue[i] + if (timer.timeToRun >= TIMER_MAX) + i-- + break - // Empty the bucket, check if anything in the secondary queue should be shifted to this bucket - bucket_list[practical_offset++] = null - var/i = 0 - for (i in 1 to length(second_queue)) - timer = second_queue[i] - if (timer.timeToRun >= TIMER_MAX) - i-- - break + // Check for timers that are scheduled to run in the past + if (timer.timeToRun < head_offset) + bucket_resolution = null // force bucket recreation + stack_trace("[i] Invalid timer state: Timer in long run queue with a time to run less then head_offset. \ + [get_timer_debug_string(timer)] world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]") + break - // Check for timers that are scheduled to run in the past - if (timer.timeToRun < head_offset) - bucket_resolution = null // force bucket recreation - stack_trace("[i] Invalid timer state: Timer in long run queue with a time to run less then head_offset. \ - [get_timer_debug_string(timer)] world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]") + // Check for timers that are not capable of being scheduled to run without rebuilding buckets + if (timer.timeToRun < head_offset + TICKS2DS(practical_offset - 1)) + bucket_resolution = null // force bucket recreation + stack_trace("[i] Invalid timer state: Timer in long run queue that would require a backtrack to transfer to \ + short run queue. [get_timer_debug_string(timer)] world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]") + break - if (timer.callBack && !timer.spent) - timer.callBack.InvokeAsync() - invoked_timers += timer - bucket_count++ - else if(!QDELETED(timer)) - qdel(timer) - continue - - // Check for timers that are not capable of being scheduled to run without rebuilding buckets - if (timer.timeToRun < head_offset + TICKS2DS(practical_offset - 1)) - bucket_resolution = null // force bucket recreation - stack_trace("[i] Invalid timer state: Timer in long run queue that would require a backtrack to transfer to \ - short run queue. [get_timer_debug_string(timer)] world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]") - if (timer.callBack && !timer.spent) - timer.callBack.InvokeAsync() - invoked_timers += timer - bucket_count++ - else if(!QDELETED(timer)) - qdel(timer) - continue - - // Transfer the timer into the bucket, performing necessary circular doubly-linked list operations - bucket_count++ - var/bucket_pos = max(1, BUCKET_POS(timer)) - var/datum/timedevent/bucket_head = bucket_list[bucket_pos] - if (!bucket_head) - bucket_list[bucket_pos] = timer - timer.next = null - timer.prev = null - continue - - if (!bucket_head.prev) - bucket_head.prev = bucket_head - timer.next = bucket_head - timer.prev = bucket_head.prev - timer.next.prev = timer - timer.prev.next = timer - if (i) - second_queue.Cut(1, i+1) - timer = null - - // Process each invoked timer - bucket_count -= length(invoked_timers) - for (var/datum/timedevent/qtimer in invoked_timers) - if(QDELETED(qtimer)) - bucket_count++ - continue - if(!(qtimer.flags & TIMER_LOOP)) - qdel(qtimer) - else // Prepare looping timers to re-enter the queue - bucket_count++ - qtimer.spent = 0 - qtimer.bucketEject() - qtimer.timeToRun = (qtimer.flags & TIMER_CLIENT_TIME ? REALTIMEOFDAY : world.time) + qtimer.wait - qtimer.bucketJoin() - invoked_timers.len = 0 + timer.bucketJoin() + if (i) + second_queue.Cut(1, i+1) + if (MC_TICK_CHECK) + break /** * Generates a string with details about the timed event for debugging purposes @@ -425,12 +377,6 @@ SUBSYSTEM_DEF(timer) nextid++ SStimer.timer_id_dict[id] = src - // Generate debug-friendly name for timer - var/static/list/bitfield_flags = list("TIMER_UNIQUE", "TIMER_OVERRIDE", "TIMER_CLIENT_TIME", "TIMER_STOPPABLE", "TIMER_NO_HASH_WAIT", "TIMER_LOOP") - name = "Timer: [id] (\ref[src]), TTR: [timeToRun], Flags: [jointext(bitfield2list(flags, bitfield_flags), ", ")], \ - callBack: \ref[callBack], callBack.object: [callBack.object]\ref[callBack.object]([getcallingtype()]), \ - callBack.delegate:[callBack.delegate]([callBack.arguments ? callBack.arguments.Join(", ") : ""])" - if ((timeToRun < world.time || timeToRun < SStimer.head_offset) && !(flags & TIMER_CLIENT_TIME)) CRASH("Invalid timer state: Timer created that would require a backtrack to run (addtimer would never let this happen): [SStimer.get_timer_debug_string(src)]") @@ -521,6 +467,12 @@ SUBSYSTEM_DEF(timer) * If the timed event is tracking client time, it will be added to a special bucket. */ /datum/timedevent/proc/bucketJoin() + // Generate debug-friendly name for timer + var/static/list/bitfield_flags = list("TIMER_UNIQUE", "TIMER_OVERRIDE", "TIMER_CLIENT_TIME", "TIMER_STOPPABLE", "TIMER_NO_HASH_WAIT", "TIMER_LOOP") + name = "Timer: [id] (\ref[src]), TTR: [timeToRun], wait:[wait] Flags: [jointext(bitfield2list(flags, bitfield_flags), ", ")], \ + callBack: \ref[callBack], callBack.object: [callBack.object]\ref[callBack.object]([getcallingtype()]), \ + callBack.delegate:[callBack.delegate]([callBack.arguments ? callBack.arguments.Join(", ") : ""]), source: [source]" + // Check if this timed event should be diverted to the client time bucket, or the secondary queue var/list/L if (flags & TIMER_CLIENT_TIME)