From fcad1c76dcdc58d242070f5056b1bfbc35bf9a10 Mon Sep 17 00:00:00 2001 From: Mothblocks <35135081+Mothblocks@users.noreply.github.com> Date: Thu, 17 Aug 2023 14:59:06 -0700 Subject: [PATCH] Move hot procs to CODEOWNERS for dev cycles initiative (#77599) Someone touched these recently and added seconds of init time, I don't want it to happen again without me knowing because it is hard to remove after the fact without just killing the entire features that rely on it --- .github/CODEOWNERS | 4 + code/controllers/subsystem/atoms.dm | 57 -------- code/game/atoms.dm | 75 ---------- code/game/atoms_initializing_EXPENSIVE.dm | 131 ++++++++++++++++++ code/game/turfs/open/space/space.dm | 48 ------- code/game/turfs/open/space/space_EXPENSIVE.dm | 47 +++++++ tgstation.dme | 2 + 7 files changed, 184 insertions(+), 180 deletions(-) create mode 100644 code/game/atoms_initializing_EXPENSIVE.dm create mode 100644 code/game/turfs/open/space/space_EXPENSIVE.dm diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index a39df8a740c..e10a9db90c8 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -203,6 +203,10 @@ /tools/WebhookProcessor/ @BraveMole @TiviPlus +# Expensive files that touching basically always cause performance problems +## Init times +**/*_EXPENSIVE.dm @Mothblocks @LemonInTheDark + # SIC SEMPER TYRANNIS /code/modules/hydroponics/grown/citrus.dm @optimumtact diff --git a/code/controllers/subsystem/atoms.dm b/code/controllers/subsystem/atoms.dm index 933542746be..953cbd3bd4c 100644 --- a/code/controllers/subsystem/atoms.dm +++ b/code/controllers/subsystem/atoms.dm @@ -111,63 +111,6 @@ SUBSYSTEM_DEF(atoms) testing("Initialized [count] 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)) - // 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. - #ifdef UNIT_TESTS - var/start_tick = world.time - #endif - - var/result = A.Initialize(arglist(arguments)) - - #ifdef UNIT_TESTS - if(start_tick != world.time) - BadInitializeCalls[the_type] |= BAD_INIT_SLEPT - #endif - - var/qdeleted = FALSE - - switch(result) - if (INITIALIZE_HINT_NORMAL) - // pass - if(INITIALIZE_HINT_LATELOAD) - if(arguments[1]) //mapload - late_loaders += A - else - A.LateInitialize() - if(INITIALIZE_HINT_QDEL) - qdel(A) - qdeleted = TRUE - else - BadInitializeCalls[the_type] |= BAD_INIT_NO_HINT - - if(!A) //possible harddel - qdeleted = TRUE - else if(!(A.flags_1 & INITIALIZED_1)) - BadInitializeCalls[the_type] |= BAD_INIT_DIDNT_INIT - else - SEND_SIGNAL(A, COMSIG_ATOM_AFTER_SUCCESSFUL_INITIALIZE) - SEND_GLOBAL_SIGNAL(COMSIG_GLOB_ATOM_AFTER_POST_INIT, A) - var/atom/location = A.loc - if(location) - /// Sends a signal that the new atom `src`, has been created at `loc` - SEND_SIGNAL(location, COMSIG_ATOM_AFTER_SUCCESSFUL_INITIALIZED_ON, A, arguments[1]) - var/area/atom_area = get_area(location) - if(atom_area) - SEND_SIGNAL(atom_area, COMSIG_AREA_INITIALIZED_IN, A) - if(created_atoms && from_template && ispath(the_type, /atom/movable))//we only want to populate the list with movables - created_atoms += A.get_all_contents() - - return qdeleted || QDELING(A) - /datum/controller/subsystem/atoms/proc/map_loader_begin(source) set_tracked_initalized(INITIALIZATION_INSSATOMS, source) diff --git a/code/game/atoms.dm b/code/game/atoms.dm index e0fd748ad6c..4b2a829c29a 100644 --- a/code/game/atoms.dm +++ b/code/game/atoms.dm @@ -203,81 +203,6 @@ //we were deleted return -/** - * The primary method that objects are setup in SS13 with - * - * we don't use New as we have better control over when this is called and we can choose - * to delay calls or hook other logic in and so forth - * - * During roundstart map parsing, atoms are queued for intialization in the base atom/New(), - * After the map has loaded, then Initalize is called on all atoms one by one. NB: this - * is also true for loading map templates as well, so they don't Initalize until all objects - * in the map file are parsed and present in the world - * - * If you're creating an object at any point after SSInit has run then this proc will be - * immediately be called from New. - * - * mapload: This parameter is true if the atom being loaded is either being intialized during - * the Atom subsystem intialization, or if the atom is being loaded from the map template. - * If the item is being created at runtime any time after the Atom subsystem is intialized then - * it's false. - * - * The mapload argument occupies the same position as loc when Initialize() is called by New(). - * loc will no longer be needed after it passed New(), and thus it is being overwritten - * with mapload at the end of atom/New() before this proc (atom/Initialize()) is called. - * - * You must always call the parent of this proc, otherwise failures will occur as the item - * will not be seen as initalized (this can lead to all sorts of strange behaviour, like - * the item being completely unclickable) - * - * You must not sleep in this proc, or any subprocs - * - * Any parameters from new are passed through (excluding loc), naturally if you're loading from a map - * there are no other arguments - * - * Must return an [initialization hint][INITIALIZE_HINT_NORMAL] or a runtime will occur. - * - * Note: the following functions don't call the base for optimization and must copypasta handling: - * * [/turf/proc/Initialize] - * * [/turf/open/space/proc/Initialize] - */ -/atom/proc/Initialize(mapload, ...) - SHOULD_NOT_SLEEP(TRUE) - SHOULD_CALL_PARENT(TRUE) - - if(flags_1 & INITIALIZED_1) - stack_trace("Warning: [src]([type]) initialized multiple times!") - flags_1 |= INITIALIZED_1 - - SET_PLANE_IMPLICIT(src, plane) - - if(greyscale_config && greyscale_colors) //we'll check again at item/init for inhand/belt/worn configs. - update_greyscale() - - //atom color stuff - if(color) - add_atom_colour(color, FIXED_COLOUR_PRIORITY) - - if (light_system == STATIC_LIGHT && light_power && light_range) - update_light() - - SETUP_SMOOTHING() - - if(uses_integrity) - atom_integrity = max_integrity - TEST_ONLY_ASSERT((!armor || istype(armor)), "[type] has an armor that contains an invalid value at intialize") - - // apply materials properly from the default custom_materials value - // This MUST come after atom_integrity is set above, as if old materials get removed, - // atom_integrity is checked against max_integrity and can BREAK the atom. - // The integrity to max_integrity ratio is still preserved. - set_custom_materials(custom_materials) - - if(ispath(ai_controller)) - ai_controller = new ai_controller(src) - - return INITIALIZE_HINT_NORMAL - /** * Late Intialization, for code that should run after all atoms have run Intialization * diff --git a/code/game/atoms_initializing_EXPENSIVE.dm b/code/game/atoms_initializing_EXPENSIVE.dm new file mode 100644 index 00000000000..ae729ad1f48 --- /dev/null +++ b/code/game/atoms_initializing_EXPENSIVE.dm @@ -0,0 +1,131 @@ +/// 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)) + // 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. + #ifdef UNIT_TESTS + var/start_tick = world.time + #endif + + var/result = A.Initialize(arglist(arguments)) + + #ifdef UNIT_TESTS + if(start_tick != world.time) + BadInitializeCalls[the_type] |= BAD_INIT_SLEPT + #endif + + var/qdeleted = FALSE + + switch(result) + if (INITIALIZE_HINT_NORMAL) + // pass + if(INITIALIZE_HINT_LATELOAD) + if(arguments[1]) //mapload + late_loaders += A + else + A.LateInitialize() + if(INITIALIZE_HINT_QDEL) + qdel(A) + qdeleted = TRUE + else + BadInitializeCalls[the_type] |= BAD_INIT_NO_HINT + + if(!A) //possible harddel + qdeleted = TRUE + else if(!(A.flags_1 & INITIALIZED_1)) + BadInitializeCalls[the_type] |= BAD_INIT_DIDNT_INIT + else + SEND_SIGNAL(A, COMSIG_ATOM_AFTER_SUCCESSFUL_INITIALIZE) + SEND_GLOBAL_SIGNAL(COMSIG_GLOB_ATOM_AFTER_POST_INIT, A) + var/atom/location = A.loc + if(location) + /// Sends a signal that the new atom `src`, has been created at `loc` + SEND_SIGNAL(location, COMSIG_ATOM_AFTER_SUCCESSFUL_INITIALIZED_ON, A, arguments[1]) + var/area/atom_area = get_area(location) + if(atom_area) + SEND_SIGNAL(atom_area, COMSIG_AREA_INITIALIZED_IN, A) + if(created_atoms && from_template && ispath(the_type, /atom/movable))//we only want to populate the list with movables + created_atoms += A.get_all_contents() + + return qdeleted || QDELING(A) + +/** + * The primary method that objects are setup in SS13 with + * + * we don't use New as we have better control over when this is called and we can choose + * to delay calls or hook other logic in and so forth + * + * During roundstart map parsing, atoms are queued for intialization in the base atom/New(), + * After the map has loaded, then Initalize is called on all atoms one by one. NB: this + * is also true for loading map templates as well, so they don't Initalize until all objects + * in the map file are parsed and present in the world + * + * If you're creating an object at any point after SSInit has run then this proc will be + * immediately be called from New. + * + * mapload: This parameter is true if the atom being loaded is either being intialized during + * the Atom subsystem intialization, or if the atom is being loaded from the map template. + * If the item is being created at runtime any time after the Atom subsystem is intialized then + * it's false. + * + * The mapload argument occupies the same position as loc when Initialize() is called by New(). + * loc will no longer be needed after it passed New(), and thus it is being overwritten + * with mapload at the end of atom/New() before this proc (atom/Initialize()) is called. + * + * You must always call the parent of this proc, otherwise failures will occur as the item + * will not be seen as initalized (this can lead to all sorts of strange behaviour, like + * the item being completely unclickable) + * + * You must not sleep in this proc, or any subprocs + * + * Any parameters from new are passed through (excluding loc), naturally if you're loading from a map + * there are no other arguments + * + * Must return an [initialization hint][INITIALIZE_HINT_NORMAL] or a runtime will occur. + * + * Note: the following functions don't call the base for optimization and must copypasta handling: + * * [/turf/proc/Initialize] + * * [/turf/open/space/proc/Initialize] + */ +/atom/proc/Initialize(mapload, ...) + SHOULD_NOT_SLEEP(TRUE) + SHOULD_CALL_PARENT(TRUE) + + if(flags_1 & INITIALIZED_1) + stack_trace("Warning: [src]([type]) initialized multiple times!") + flags_1 |= INITIALIZED_1 + + SET_PLANE_IMPLICIT(src, plane) + + if(greyscale_config && greyscale_colors) //we'll check again at item/init for inhand/belt/worn configs. + update_greyscale() + + //atom color stuff + if(color) + add_atom_colour(color, FIXED_COLOUR_PRIORITY) + + if (light_system == STATIC_LIGHT && light_power && light_range) + update_light() + + SETUP_SMOOTHING() + + if(uses_integrity) + atom_integrity = max_integrity + TEST_ONLY_ASSERT((!armor || istype(armor)), "[type] has an armor that contains an invalid value at intialize") + + // apply materials properly from the default custom_materials value + // This MUST come after atom_integrity is set above, as if old materials get removed, + // atom_integrity is checked against max_integrity and can BREAK the atom. + // The integrity to max_integrity ratio is still preserved. + set_custom_materials(custom_materials) + + if(ispath(ai_controller)) + ai_controller = new ai_controller(src) + + return INITIALIZE_HINT_NORMAL diff --git a/code/game/turfs/open/space/space.dm b/code/game/turfs/open/space/space.dm index 04eedb6619f..17c33b1c00b 100644 --- a/code/game/turfs/open/space/space.dm +++ b/code/game/turfs/open/space/space.dm @@ -35,54 +35,6 @@ GLOBAL_VAR_INIT(starlight_color, COLOR_STARLIGHT) //This is used to optimize the map loader return -/** - * Space Initialize - * - * Doesn't call parent, see [/atom/proc/Initialize]. - * When adding new stuff to /atom/Initialize, /turf/Initialize, etc - * don't just add it here unless space actually needs it. - * - * There is a lot of work that is intentionally not done because it is not currently used. - * This includes stuff like smoothing, blocking camera visibility, etc. - * If you are facing some odd bug with specifically space, check if it's something that was - * intentionally ommitted from this implementation. - */ -/turf/open/space/Initialize(mapload) - SHOULD_CALL_PARENT(FALSE) - air = space_gas - - if (PERFORM_ALL_TESTS(focus_only/multiple_space_initialization)) - if(flags_1 & INITIALIZED_1) - stack_trace("Warning: [src]([type]) initialized multiple times!") - flags_1 |= INITIALIZED_1 - - light_color = GLOB.starlight_color - - // We make the assumption that the space plane will never be blacklisted, as an optimization - if(SSmapping.max_plane_offset) - plane = PLANE_SPACE - (PLANE_RANGE * SSmapping.z_level_to_plane_offset[z]) - - var/area/our_area = loc - if(!our_area.area_has_base_lighting && space_lit) //Only provide your own lighting if the area doesn't for you - // Intentionally not add_overlay for performance reasons. - // add_overlay does a bunch of generic stuff, like creating a new list for overlays, - // queueing compile, cloning appearance, etc etc etc that is not necessary here. - overlays += GLOB.fullbright_overlays[GET_TURF_PLANE_OFFSET(src) + 1] - - if (!mapload) - if(requires_activation) - SSair.add_to_active(src, TRUE) - - if(SSmapping.max_plane_offset) - var/turf/T = GET_TURF_ABOVE(src) - if(T) - T.multiz_turf_new(src, DOWN) - T = GET_TURF_BELOW(src) - if(T) - T.multiz_turf_new(src, UP) - - return INITIALIZE_HINT_NORMAL - //ATTACK GHOST IGNORING PARENT RETURN VALUE /turf/open/space/attack_ghost(mob/dead/observer/user) if(destination_z) diff --git a/code/game/turfs/open/space/space_EXPENSIVE.dm b/code/game/turfs/open/space/space_EXPENSIVE.dm new file mode 100644 index 00000000000..44a15ac66d9 --- /dev/null +++ b/code/game/turfs/open/space/space_EXPENSIVE.dm @@ -0,0 +1,47 @@ +/** + * Space Initialize + * + * Doesn't call parent, see [/atom/proc/Initialize]. + * When adding new stuff to /atom/Initialize, /turf/Initialize, etc + * don't just add it here unless space actually needs it. + * + * There is a lot of work that is intentionally not done because it is not currently used. + * This includes stuff like smoothing, blocking camera visibility, etc. + * If you are facing some odd bug with specifically space, check if it's something that was + * intentionally ommitted from this implementation. + */ +/turf/open/space/Initialize(mapload) + SHOULD_CALL_PARENT(FALSE) + air = space_gas + + if (PERFORM_ALL_TESTS(focus_only/multiple_space_initialization)) + if(flags_1 & INITIALIZED_1) + stack_trace("Warning: [src]([type]) initialized multiple times!") + flags_1 |= INITIALIZED_1 + + light_color = GLOB.starlight_color + + // We make the assumption that the space plane will never be blacklisted, as an optimization + if(SSmapping.max_plane_offset) + plane = PLANE_SPACE - (PLANE_RANGE * SSmapping.z_level_to_plane_offset[z]) + + var/area/our_area = loc + if(!our_area.area_has_base_lighting && space_lit) //Only provide your own lighting if the area doesn't for you + // Intentionally not add_overlay for performance reasons. + // add_overlay does a bunch of generic stuff, like creating a new list for overlays, + // queueing compile, cloning appearance, etc etc etc that is not necessary here. + overlays += GLOB.fullbright_overlays[GET_TURF_PLANE_OFFSET(src) + 1] + + if (!mapload) + if(requires_activation) + SSair.add_to_active(src, TRUE) + + if(SSmapping.max_plane_offset) + var/turf/T = GET_TURF_ABOVE(src) + if(T) + T.multiz_turf_new(src, DOWN) + T = GET_TURF_BELOW(src) + if(T) + T.multiz_turf_new(src, UP) + + return INITIALIZE_HINT_NORMAL diff --git a/tgstation.dme b/tgstation.dme index 048d4c91318..8dbac806223 100644 --- a/tgstation.dme +++ b/tgstation.dme @@ -1610,6 +1610,7 @@ #include "code\game\alternate_appearance.dm" #include "code\game\atom_defense.dm" #include "code\game\atoms.dm" +#include "code\game\atoms_initializing_EXPENSIVE.dm" #include "code\game\atoms_movable.dm" #include "code\game\communications.dm" #include "code\game\data_huds.dm" @@ -2446,6 +2447,7 @@ #include "code\game\turfs\open\floor\reinforced_floor.dm" #include "code\game\turfs\open\floor\plating\misc_plating.dm" #include "code\game\turfs\open\space\space.dm" +#include "code\game\turfs\open\space\space_EXPENSIVE.dm" #include "code\game\turfs\open\space\transit.dm" #include "code\modules\actionspeed\_actionspeed_modifier.dm" #include "code\modules\actionspeed\modifiers\addiction.dm"