From b6af63179d603ec964af24306b02feb4daff6fd2 Mon Sep 17 00:00:00 2001 From: Mothblocks <35135081+Mothblocks@users.noreply.github.com> Date: Sun, 2 Apr 2023 21:43:34 -0700 Subject: [PATCH] Optimize /atom/movable/Initialize with faster check for blocks_emissive [140ms on init] (#74453) Quoting the comment: This one is incredible. `if (x) else { /* code */ }` is surprisingly fast, and it's faster than a switch, which is seemingly not a jump table. From what I can tell, a switch case checks every single branch individually, although sane, is slow in a hot proc like this. So, we make the most common `blocks_emissive` value, EMISSIVE_BLOCK_GENERIC, 0, getting to the fast else branch quickly. If it fails, then we can check over every value it can be (here, EMISSIVE_BLOCK_UNIQUE is the only one that matters). This saves several hundred milliseconds of init time. --- code/__DEFINES/lighting.dm | 6 +- code/controllers/admin.dm | 2 +- code/game/atoms_movable.dm | 60 +++++++++++-------- code/game/machinery/doors/airlock.dm | 2 +- code/game/objects/effects/misc.dm | 2 +- .../industrial_lift/industrial_lift.dm | 2 +- .../mob/living/carbon/carbon_defines.dm | 2 +- 7 files changed, 45 insertions(+), 31 deletions(-) diff --git a/code/__DEFINES/lighting.dm b/code/__DEFINES/lighting.dm index dc49595f740..ce86af23bfd 100644 --- a/code/__DEFINES/lighting.dm +++ b/code/__DEFINES/lighting.dm @@ -61,9 +61,11 @@ // Emissive blocking. /// Uses vis_overlays to leverage caching so that very few new items need to be made for the overlay. For anything that doesn't change outline or opaque area much or at all. -#define EMISSIVE_BLOCK_GENERIC 1 +#define EMISSIVE_BLOCK_GENERIC 0 /// Uses a dedicated render_target object to copy the entire appearance in real time to the blocking layer. For things that can change in appearance a lot from the base state, like humans. -#define EMISSIVE_BLOCK_UNIQUE 2 +#define EMISSIVE_BLOCK_UNIQUE 1 +/// Don't block any emissives. Useful for things like, pieces of paper? +#define EMISSIVE_BLOCK_NONE 2 /// The color matrix applied to all emissive overlays. Should be solely dependent on alpha and not have RGB overlap with [EM_BLOCK_COLOR]. #define EMISSIVE_COLOR list(0,0,0,0, 0,0,0,0, 0,0,0,0, 0,0,0,1, 1,1,1,0) diff --git a/code/controllers/admin.dm b/code/controllers/admin.dm index 0d90319f863..d3a508be13a 100644 --- a/code/controllers/admin.dm +++ b/code/controllers/admin.dm @@ -1,7 +1,7 @@ // Clickable stat() button. /obj/effect/statclick name = "Initializing..." - blocks_emissive = NONE + blocks_emissive = EMISSIVE_BLOCK_NONE var/target INITIALIZE_IMMEDIATE(/obj/effect/statclick) diff --git a/code/game/atoms_movable.dm b/code/game/atoms_movable.dm index 749b1d58f62..ee22b78d688 100644 --- a/code/game/atoms_movable.dm +++ b/code/game/atoms_movable.dm @@ -73,8 +73,8 @@ ///is the mob currently ascending or descending through z levels? var/currently_z_moving - /// Either FALSE, [EMISSIVE_BLOCK_GENERIC], or [EMISSIVE_BLOCK_UNIQUE] - var/blocks_emissive = FALSE + /// Either [EMISSIVE_BLOCK_NONE], [EMISSIVE_BLOCK_GENERIC], or [EMISSIVE_BLOCK_UNIQUE] + var/blocks_emissive = EMISSIVE_BLOCK_NONE ///Internal holder for emissive blocker object, do not use directly use blocks_emissive var/atom/movable/render_step/emissive_blocker/em_block @@ -114,28 +114,19 @@ stack_trace("[type] blocks explosives, but does not have the managing element applied") #endif - switch(blocks_emissive) - if(EMISSIVE_BLOCK_GENERIC) - var/static/mutable_appearance/emissive_blocker/blocker = new() - blocker.icon = icon - blocker.icon_state = icon_state - blocker.dir = dir - blocker.appearance_flags |= appearance_flags - blocker.plane = GET_NEW_PLANE(EMISSIVE_PLANE, PLANE_TO_OFFSET(plane)) - // Ok so this is really cursed, but I want to set with this blocker cheaply while - // Still allowing it to be removed from the overlays list later - // So I'm gonna flatten it, then insert the flattened overlay into overlays AND the managed overlays list, directly - // I'm sorry - var/mutable_appearance/flat = blocker.appearance - overlays += flat - if(managed_overlays) - if(islist(managed_overlays)) - managed_overlays += flat - else - managed_overlays = list(managed_overlays, flat) - else - managed_overlays = flat - if(EMISSIVE_BLOCK_UNIQUE) +#if EMISSIVE_BLOCK_GENERIC != 0 + #error EMISSIVE_BLOCK_GENERIC is expected to be 0 to faciliate a weird optimization hack where we rely on it being the most common. + #error Read the comment in code/game/atoms_movable.dm for details. +#endif + + // This one is incredible. + // `if (x) else { /* code */ }` is surprisingly fast, and it's faster than a switch, which is seemingly not a jump table. + // From what I can tell, a switch case checks every single branch individually, although sane, is slow in a hot proc like this. + // So, we make the most common `blocks_emissive` value, EMISSIVE_BLOCK_GENERIC, 0, getting to the fast else branch quickly. + // If it fails, then we can check over every value it can be (here, EMISSIVE_BLOCK_UNIQUE is the only one that matters). + // This saves several hundred milliseconds of init time. + if (blocks_emissive) + if (blocks_emissive == EMISSIVE_BLOCK_UNIQUE) render_target = ref(src) em_block = new(src, render_target) overlays += em_block @@ -146,6 +137,27 @@ managed_overlays = list(managed_overlays, em_block) else managed_overlays = em_block + else + var/static/mutable_appearance/emissive_blocker/blocker = new() + blocker.icon = icon + blocker.icon_state = icon_state + blocker.dir = dir + blocker.appearance_flags |= appearance_flags + blocker.plane = GET_NEW_PLANE(EMISSIVE_PLANE, PLANE_TO_OFFSET(plane)) + // Ok so this is really cursed, but I want to set with this blocker cheaply while + // Still allowing it to be removed from the overlays list later + // So I'm gonna flatten it, then insert the flattened overlay into overlays AND the managed overlays list, directly + // I'm sorry + var/mutable_appearance/flat = blocker.appearance + overlays += flat + if(managed_overlays) + if(islist(managed_overlays)) + managed_overlays += flat + else + managed_overlays = list(managed_overlays, flat) + else + managed_overlays = flat + if(opacity) AddElement(/datum/element/light_blocking) switch(light_system) diff --git a/code/game/machinery/doors/airlock.dm b/code/game/machinery/doors/airlock.dm index 170dbddabed..00d81e4f862 100644 --- a/code/game/machinery/doors/airlock.dm +++ b/code/game/machinery/doors/airlock.dm @@ -93,7 +93,7 @@ smoothing_groups = SMOOTH_GROUP_AIRLOCK interaction_flags_machine = INTERACT_MACHINE_WIRES_IF_OPEN | INTERACT_MACHINE_ALLOW_SILICON | INTERACT_MACHINE_OPEN_SILICON | INTERACT_MACHINE_OPEN - blocks_emissive = NONE // Custom emissive blocker. We don't want the normal behavior. + blocks_emissive = EMISSIVE_BLOCK_NONE // Custom emissive blocker. We don't want the normal behavior. ///The type of door frame to drop during deconstruction var/assemblytype = /obj/structure/door_assembly diff --git a/code/game/objects/effects/misc.dm b/code/game/objects/effects/misc.dm index df248bdc38a..03664e280cb 100644 --- a/code/game/objects/effects/misc.dm +++ b/code/game/objects/effects/misc.dm @@ -98,7 +98,7 @@ light_range = MINIMUM_USEFUL_LIGHT_RANGE light_color = COLOR_WHITE mouse_opacity = MOUSE_OPACITY_TRANSPARENT - blocks_emissive = NONE + blocks_emissive = EMISSIVE_BLOCK_NONE /obj/effect/dummy/lighting_obj/Initialize(mapload, _range, _power, _color, _duration) . = ..() diff --git a/code/modules/industrial_lift/industrial_lift.dm b/code/modules/industrial_lift/industrial_lift.dm index 362aa0d2c29..3f03857907f 100644 --- a/code/modules/industrial_lift/industrial_lift.dm +++ b/code/modules/industrial_lift/industrial_lift.dm @@ -18,7 +18,7 @@ GLOBAL_LIST_EMPTY(lifts) obj_flags = CAN_BE_HIT | BLOCK_Z_OUT_DOWN appearance_flags = PIXEL_SCALE|KEEP_TOGETHER //no TILE_BOUND since we're potentially multitile // If we don't do this, we'll build our overlays early, and fuck up how we're rendered - blocks_emissive = NONE + blocks_emissive = EMISSIVE_BLOCK_NONE ///ID used to determine what lift types we can merge with var/lift_id = BASIC_LIFT_ID diff --git a/code/modules/mob/living/carbon/carbon_defines.dm b/code/modules/mob/living/carbon/carbon_defines.dm index d4686a08c46..1c7185c2482 100644 --- a/code/modules/mob/living/carbon/carbon_defines.dm +++ b/code/modules/mob/living/carbon/carbon_defines.dm @@ -10,7 +10,7 @@ num_hands = 0 //Populated on init through list/bodyparts usable_hands = 0 //Populated on init through list/bodyparts mobility_flags = MOBILITY_FLAGS_CARBON_DEFAULT - blocks_emissive = NONE + blocks_emissive = EMISSIVE_BLOCK_NONE ///List of [/obj/item/organ/internal] in the mob. They don't go in the contents for some reason I don't want to know. var/list/obj/item/organ/internal/organs = list() ///Same as [above][/mob/living/carbon/var/organs], but stores "slot ID" - "organ" pairs for easy access.