Skip to content

Commit

Permalink
Port some 515 optimizations from /tg/ (Monkestation#1361)
Browse files Browse the repository at this point in the history
* Removes overly optimistic warn on byond version (#81185)

I assumed this would be fixed because I assumed it was a bug. My report
goes unresponded to.

* Make list clear nulls faster (#80869)

The old version makes a new list of n null values, and removes it from
the given list.

for larger lists, this newer 515 version should be faster and lead to
less list churn.


![image](https://github.com/tgstation/tgstation/assets/7069733/9c23cc8b-06c4-4a54-99b4-2c44608cf434)

* Makes build tool use 515 -D argument instead of m.dme file (#80494)

Now that we require 515 to build, we can simplify the build process a
bit.

Leaves TGS define part in until TGS gets that functionality

* Remove old 515 fcopy hack (#79952)

This was a workaround for a issue that's now fixed on 1609 (the new
MIN_COMPILER_BUILD)

https://www.byond.com/forum/post/2872856
> Status: Resolved (515.1609)

* Micros bucketJoin with operator"" (#79949)

## About The Pull Request

We make timers a lot. Making a unique string for each of them wastes
time (string churn).
It also means we have to do an extra ref() in the bucketJoin proc.

If we instead throw all the shit we care about in a list and just read
off it later, we get pretty decent savings ((0.013 | 0.022) -> (0.009,
0.012)) (It was way worse when ref() was hyper expensive) It's not much,
but since timers are hot I think it's worthwhile.

It also lets us add further debug information, if we want it.
Could optimize this further if we had less stuff in the list, depends on
what we want displayed as it was on insertion and what we want displayed
as it was at moment of print.

Also also this is 100% the reason I did 515 in the first place and I
need to be free

## Why It's Good For The Game

Uhhhhhhhh more flexability in timer readouts? Cost I was worried about
is mostly gone cause ref() got better I think

---------

Co-authored-by: LemonInTheDark <[email protected]>
Co-authored-by: Kyle Spier-Swenson <[email protected]>
Co-authored-by: AnturK <[email protected]>
Co-authored-by: vvvv-vvvv <[email protected]>
  • Loading branch information
5 people authored Mar 10, 2024
1 parent b03e868 commit 87f6d5c
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 48 deletions.
6 changes: 2 additions & 4 deletions code/__HELPERS/_lists.dm
Original file line number Diff line number Diff line change
Expand Up @@ -391,10 +391,8 @@
* Returns TRUE if the list had nulls, FALSE otherwise
**/
/proc/list_clear_nulls(list/list_to_clear)
var/start_len = list_to_clear.len
var/list/new_list = new(start_len)
list_to_clear -= new_list
return list_to_clear.len < start_len
return (list_to_clear.RemoveAll(null) > 0)


/*
* Returns list containing all the entries from first list that are not present in second.
Expand Down
9 changes: 0 additions & 9 deletions code/__byond_version_compat.dm
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,3 @@

/// Call by name proc reference, checks if the proc is an existing global proc
#define GLOBAL_PROC_REF(X) (/proc/##X)

/// fcopy will crash on 515 linux if given a non-existant file, instead of returning 0 like on 514 linux or 515 windows
/// var case matches documentation for fcopy.
/world/proc/__fcopy(Src, Dst)
if (istext(Src) && !fexists(Src))
return 0
return fcopy(Src, Dst)

#define fcopy(Src, Dst) world.__fcopy(Src, Dst)
5 changes: 2 additions & 3 deletions code/controllers/globals.dm
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@ GLOBAL_REAL(GLOB, /datum/controller/global_vars)

var/datum/controller/exclude_these = new
// I know this is dumb but the nested vars list hangs a ref to the datum. This fixes that
// I have an issue report open, lummox has not responded. It might be a FeaTuRE
// Sooo we gotta be dumb
var/list/controller_vars = exclude_these.vars.Copy()
controller_vars["vars"] = null
gvars_datum_in_built_vars = controller_vars + list(NAMEOF(src, gvars_datum_protected_varlist), NAMEOF(src, gvars_datum_in_built_vars), NAMEOF(src, gvars_datum_init_order))

#if DM_VERSION >= 515 && DM_BUILD > 1620
#warn datum.vars hanging a ref should now be fixed, there should be no reason to remove the vars list from our controller's vars list anymore
#endif
QDEL_IN(exclude_these, 0) //signal logging isn't ready

Initialize()
Expand Down
59 changes: 46 additions & 13 deletions code/controllers/subsystem/timer.dm
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,9 @@ SUBSYSTEM_DEF(timer)
var/list/flags
/// Time at which the timer was invoked or destroyed
var/spent = 0
/// An informative name generated for the timer as its representation in strings, useful for debugging
var/name
/// Holds info about this timer, stored from the moment it was created
/// Used to create a visible "name" whenever the timer is stringified
var/list/timer_info
/// Next timed event in the bucket
var/datum/timedevent/next
/// Previous timed event in the bucket
Expand Down Expand Up @@ -494,6 +495,21 @@ SUBSYSTEM_DEF(timer)
bucket_pos = -1
bucket_joined = FALSE

/datum/timedevent/proc/operator""()
if(!length(timer_info))
return "Event not filled"
var/static/list/bitfield_flags = list("TIMER_UNIQUE", "TIMER_OVERRIDE", "TIMER_CLIENT_TIME", "TIMER_STOPPABLE", "TIMER_NO_HASH_WAIT", "TIMER_LOOP")
#if defined(TIMER_DEBUG)
var/list/callback_args = timer_info[10]
return "Timer: [timer_info[1]] ([text_ref(src)]), TTR: [timer_info[2]], wait:[timer_info[3]] Flags: [jointext(bitfield_to_list(timer_info[4], bitfield_flags), ", ")], \
callBack: [text_ref(timer_info[5])], callBack.object: [timer_info[6]][timer_info[7]]([timer_info[8]]), \
callBack.delegate:[timer_info[9]]([callback_args ? callback_args.Join(", ") : ""]), source: [timer_info[11]]"
#else
return "Timer: [timer_info[1]] ([text_ref(src)]), TTR: [timer_info[2]], wait:[timer_info[3]] Flags: [jointext(bitfield_to_list(timer_info[4], bitfield_flags), ", ")], \
callBack: [text_ref(timer_info[5])], callBack.object: [timer_info[6]]([timer_info[7]]), \
callBack.delegate:[timer_info[8]], source: [timer_info[9]]"
#endif

/**
* Attempts to add this timed event to a bucket, will enter the secondary queue
* if there are no appropriate buckets at this time.
Expand All @@ -504,20 +520,37 @@ SUBSYSTEM_DEF(timer)
*/
/datum/timedevent/proc/bucketJoin()
#if defined(TIMER_DEBUG)
// Generate debug-friendly name for timer, more complex but also more expensive
var/static/list/bitfield_flags = list("TIMER_UNIQUE", "TIMER_OVERRIDE", "TIMER_CLIENT_TIME", "TIMER_STOPPABLE", "TIMER_NO_HASH_WAIT", "TIMER_LOOP")
name = "Timer: [id] ([text_ref(src)]), TTR: [timeToRun], wait:[wait] Flags: [jointext(bitfield_to_list(flags, bitfield_flags), ", ")], \
callBack: [text_ref(callBack)], callBack.object: [callBack.object][text_ref(callBack.object)]([getcallingtype()]), \
callBack.delegate:[callBack.delegate]([callBack.arguments ? callBack.arguments.Join(", ") : ""]), source: [source]"
// Generate debug-friendly list for timer, more complex but also more expensive
timer_info = list(
1 = id,
2 = timeToRun,
3 = wait,
4 = flags,
5 = callBack, /* Safe to hold this directly becasue it's never del'd */
6 = "[callBack.object]",
7 = text_ref(callBack.object),
8 = getcallingtype(),
9 = callBack.delegate,
10 = callBack.arguments ? callBack.arguments.Copy() : null,
11 = "[source]"
)
#else
// Generate a debuggable name for the timer, simpler but wayyyy cheaper, string generation is a bitch and this saves a LOT of time
name = "Timer: [id] ([text_ref(src)]), TTR: [timeToRun], wait:[wait] Flags: [flags], \
callBack: [text_ref(callBack)], callBack.object: [callBack.object]([getcallingtype()]), \
callBack.delegate:[callBack.delegate], source: [source]"
// Generate a debuggable list for the timer, simpler but wayyyy cheaper, string generation (and ref/copy memes) is a bitch and this saves a LOT of time
timer_info = list(
1 = id,
2 = timeToRun,
3 = wait,
4 = flags,
5 = callBack, /* Safe to hold this directly becasue it's never del'd */
6 = "[callBack.object]",
7 = getcallingtype(),
8 = callBack.delegate,
9 = "[source]"
)
#endif

if (bucket_joined)
stack_trace("Bucket already joined! [name]")
stack_trace("Bucket already joined! [src]")

// Check if this timed event should be diverted to the client time bucket, or the secondary queue
var/list/L
Expand All @@ -537,7 +570,7 @@ SUBSYSTEM_DEF(timer)

if (bucket_pos < timer_subsystem.practical_offset && timeToRun < (timer_subsystem.head_offset + TICKS2DS(BUCKET_LEN)))
WARNING("Bucket pos in past: bucket_pos = [bucket_pos] < practical_offset = [timer_subsystem.practical_offset] \
&& timeToRun = [timeToRun] < [timer_subsystem.head_offset + TICKS2DS(BUCKET_LEN)], Timer: [name]")
&& timeToRun = [timeToRun] < [timer_subsystem.head_offset + TICKS2DS(BUCKET_LEN)], Timer: [src]")
bucket_pos = timer_subsystem.practical_offset // Recover bucket_pos to avoid timer blocking queue

var/datum/timedevent/bucket_head = bucket_list[bucket_pos]
Expand Down
2 changes: 0 additions & 2 deletions tools/build/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,6 @@ export const CleanTarget = new Juke.Target({
dependsOn: [TguiCleanTarget],
executes: async () => {
Juke.rm('*.{dmb,rsc}');
Juke.rm('*.mdme*');
Juke.rm('*.m.*');
Juke.rm('_maps/templates.dm');
},
});
Expand Down
19 changes: 2 additions & 17 deletions tools/build/lib/byond.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,24 +159,9 @@ export const DreamMaker = async (dmeFile, options = {}) => {
const { defines } = options;
if (defines && defines.length > 0) {
Juke.logger.info('Using defines:', defines.join(', '));
try {
const injectedContent = defines
.map(x => `#define ${x}\n`)
.join('');
fs.writeFileSync(`${dmeBaseName}.m.dme`, injectedContent);
const dmeContent = fs.readFileSync(`${dmeBaseName}.dme`);
fs.appendFileSync(`${dmeBaseName}.m.dme`, dmeContent);
await runWithWarningChecks(dmPath, [`${dmeBaseName}.m.dme`]);
fs.writeFileSync(`${dmeBaseName}.dmb`, fs.readFileSync(`${dmeBaseName}.m.dmb`));
fs.writeFileSync(`${dmeBaseName}.rsc`, fs.readFileSync(`${dmeBaseName}.m.rsc`));
}
finally {
Juke.rm(`${dmeBaseName}.m.*`);
}
}
else {
await runWithWarningChecks(dmPath, [dmeFile]);

}
await runWithWarningChecks(dmPath, [...defines.map(def => `-D${def}`), dmeFile]);
};


Expand Down

0 comments on commit 87f6d5c

Please sign in to comment.