[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 <kyleshome@gmail.com>
This commit is contained in:
SkyratBot
2020-11-29 22:28:18 +01:00
committed by GitHub
parent 8b755970a1
commit 94cb4e0ce4
+46 -94
View File
@@ -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)