[MIRROR] Optimization pass focused on foam code (saves about 30% of cpu usage I think) [MDB IGNORE] (#22530)

* Optimization pass focused on foam code (saves about 30% of cpu usage I think) (#76104)

## About The Pull Request

Foam is crummy at high load rn, both because it runs on a low priority
background subsystem, and because it wastes a bit of time.
Let's reduce usage (while speeding up a bunch of other stuff too), and
give it more cpu generally.

[Optimizes reagent processing
somewhat](https://github.com/tgstation/tgstation/commit/d409bd4afc3c208cd6f00ff406e1e9f78d5ac5ad)

Turns out most of the cost of foam is the reagents it carries, and the
varying effects they have
I'm doing my best here to optimize them without touching "user space"
too much

That means doing things like prechecking if we're gonna spawn on top of
an existing decal (from glitter, flour, etc), and using that same proc
to also avoid spawning on unacceptable turfs (I had to convert
inheritance to a bitflag system to make this work, but I think that's ok
since we want it imparative anyhow)

It's actually nice for code quality too, since it lets me clean up code
that was using raw locates and weird var pong.
god I wish I had implied types man

[Optimizes foam spreading in its most accursed aspect, reagent
copying](https://github.com/tgstation/tgstation/commit/5cc56a64ad1a22ba7467cb0446b9558560259437)

Holy shit reagent code is a lot.

I'm doing a bunch of small things here. istype in init -> typecache,
removing procs that are called once and loop over a list we JUST looped
over (ph and the caching for reactions in particular)

I am mainly trying to optimize copy_to here, since that's what foam
spams
As a part of this, I removed a pair of update_total and handle_reactions
calls that were done on the reagents we are copying FROM

I have no god damn idea why you would want to do that, but if anything
is relying on the copy proc modifying the source, then that code
deserves to break

Speaking of, I cleaned up handle_reaction's main filter loop a lot,
removed a lot of redundant vars and changed it from a full loop w
tracker vars to an early exit pattern

This meant using a loop label, which is unfortunate, but this is the
fastest method, and it does end up cleaning up the code significantly,
Which is nice

Oh also I made the required_other var function even if there is no atom
attached to the reaction, since I don't see why it wouldn't

This last bit is gonna get a bit esoteric so bear with me

Failing calls (which are most of them) to handle_reactions are going to
be fastest if they need to check as few reactions as possible

One reagent in a reaction's required list is marked as the "primary",
and thus gets to trigger checking it.
We need all the reagents to react anyhow, so we might as well only check
if we have one particular one to avoid double checking

Anyhow, in order to make most calls the fastest, we want these reactions
distributed as evenly as possible across all our reagents.
The current way of doing this is just taking the first reagent in the
requirements list and using it, which is not ideal

Instead of that, lets figure out how many reactions each reagent is in,
then divy reactions up based off that and the currently divvied
reactions

This doubles the reagent index count, and takes the most common reagent,
water, from 67 reactions to I think like 22

Does some other general cleaning in reagent code too, etc etc etc

[Fixes runtimes from the forced gravity element being applied more then
once](https://github.com/tgstation/tgstation/commit/941d0676114fd455a585f2c65ffc79b81e8438b7)

I feel like this element should take a trait source or something to make
them potentially unique, it's too easy to accidentally override one with
another

[Removes connect_loc usage in atmos_sensitive, replaces it with direct
reg/unreg](https://github.com/tgstation/tgstation/commit/de1c76029d5c49dff152f0ea168b9e6c4a4a04aa)

I only really used it because I liked the componentization, but it costs
like 0.2 seconds off init alone which is really stupid, so let's just do
this the very slightly harder way

[Micros foam code slightly by inlining a LinkBlockedWithAccess
call](https://github.com/tgstation/tgstation/commit/744da3694cd4a85b3bdf44d754de57d7570bdd1c)

This is in the space of like 0.05 seconds kinda save so I can put it
back if you'd like, the double loop just felt silly

[Changes how foam processes
slightly](https://github.com/tgstation/tgstation/commit/ee5e633e3256fe7df229af71d78424d502459c16)

Rather then treating spreading and processing as separate actions, we do
both in sync.
This makes foam fade faster when spreading, which is good cause the
whole spread but unclearing foam thing looks silly.
It also avoids the potential bad ending of foam spreading into itself,
backwards and forwards. This is better I promise.

[Bumps fluid priority closer to heavy eaters, moves it off
background](https://github.com/tgstation/tgstation/commit/811797f09db7b060f75f15ad06d0ce8982375f47)

Also fixes a bug where foam would travel under public access airlocks.

## Why It's Good For The Game

Saves a lot of cpu just in general, from both init and live.
In theory makes foam faster, tho I'd have to test that on live at
highpop to see if I've actually succeeded or not. Guess we'll see.

* Optimization pass focused on foam code (saves about 30% of cpu usage I think)

---------

Co-authored-by: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com>
Co-authored-by: Bloop <vinylspiders@gmail.com>
This commit is contained in:
SkyratBot
2023-07-18 16:49:51 -04:00
committed by GitHub
co-authored by LemonInTheDark Bloop
parent 0e45b59067
commit 3fd2f8134b
12 changed files with 177 additions and 164 deletions
+111 -103
View File
@@ -38,7 +38,7 @@
//For chemical reactions list lookup list - creates a bit list of info passed to the UI. This is saved to reduce lag from new windows opening, since it's a lot of data.
//Prevent these reactions from appearing in lookup tables (UI code)
var/list/blacklist = (/datum/chemical_reaction/randomized)
var/list/blacklist = typecacheof(/datum/chemical_reaction/randomized)
if(GLOB.chemical_reactions_list_reactant_index)
return
@@ -50,60 +50,86 @@
GLOB.chemical_reactions_results_lookup_list = list() //UI glob
GLOB.chemical_reactions_list_product_index = list() //product to reaction list
var/list/datum/chemical_reaction/reactions = list()
for(var/path in paths)
var/datum/chemical_reaction/D = new path()
var/list/reaction_ids = list()
var/datum/chemical_reaction/reaction = new path()
reactions += reaction
// Ok so we're gonna do a thingTM here
// I want to distribute all our reactions such that each reagent id links to as few as possible
// I get the feeling there's a canonical way of doing this, but I don't know it
// So instead, we're gonna wing it
var/list/reagent_to_react_count = list()
for(var/datum/chemical_reaction/reaction as anything in reactions)
for(var/reagent_id as anything in reaction.required_reagents)
reagent_to_react_count[reagent_id] += 1
var/list/reaction_lookup = GLOB.chemical_reactions_list_reactant_index
// Create filters based on a random reagent id in the required reagents list - this is used to speed up handle_reactions()
// Basically, we only really need to care about ONE reagent, at least when initially filtering, since any others are ignorable
// Doing this separately because it relies on the loop above, and this is easier to parse
for(var/datum/chemical_reaction/reaction as anything in reactions)
var/preferred_id = null
for(var/reagent_id as anything in reaction.required_reagents)
if(!preferred_id)
preferred_id = reagent_id
continue
// If we would have less then they would, take it
if(length(reaction_lookup[reagent_id]) < length(reaction_lookup[preferred_id]))
preferred_id = reagent_id
continue
// If they potentially have more then us, we take it
if(reagent_to_react_count[reagent_id] < reagent_to_react_count[preferred_id])
preferred_id = reagent_id
continue
if(!reaction_lookup[preferred_id])
reaction_lookup[preferred_id] = list()
reaction_lookup[preferred_id] += reaction
for(var/datum/chemical_reaction/reaction as anything in reactions)
var/list/product_ids = list()
var/list/reagents = list()
var/list/product_names = list()
var/bitflags = D.reaction_tags
var/bitflags = reaction.reaction_tags
if(!D.required_reagents || !D.required_reagents.len) //Skip impossible reactions
if(!reaction.required_reagents || !reaction.required_reagents.len) //Skip impossible reactions
continue
GLOB.chemical_reactions_list[path] = D
GLOB.chemical_reactions_list[reaction.type] = reaction
for(var/reaction in D.required_reagents)
reaction_ids += reaction
var/datum/reagent/reagent = find_reagent_object_from_type(reaction)
for(var/reagent_path in reaction.required_reagents)
var/datum/reagent/reagent = find_reagent_object_from_type(reagent_path)
if(!istype(reagent))
stack_trace("Invalid reagent found in [D] required_reagents: [reaction]")
stack_trace("Invalid reagent found in [reaction] required_reagents: [reagent_path]")
continue
reagents += list(list("name" = reagent.name, "id" = reagent.type))
for(var/product in D.results)
for(var/product in reaction.results)
var/datum/reagent/reagent = find_reagent_object_from_type(product)
if(!istype(reagent))
stack_trace("Invalid reagent found in [D] results: [product]")
stack_trace("Invalid reagent found in [reaction] results: [product]")
continue
product_names += reagent.name
product_ids += product
var/product_name
if(!length(product_names))
var/list/names = splittext("[D.type]", "/")
var/list/names = splittext("[reaction.type]", "/")
product_name = names[names.len]
else
product_name = product_names[1]
// Create filters based on each reagent id in the required reagents list - this is specifically for finding reactions from product(reagent) ids/typepaths.
for(var/id in product_ids)
if(is_type_in_list(D.type, blacklist))
continue
if(!GLOB.chemical_reactions_list_product_index[id])
GLOB.chemical_reactions_list_product_index[id] = list()
GLOB.chemical_reactions_list_product_index[id] += D
if(!is_type_in_typecache(reaction.type, blacklist))
//Master list of ALL reactions that is used in the UI lookup table. This is expensive to make, and we don't want to lag the server by creating it on UI request, so it's cached to send to UIs instantly.
GLOB.chemical_reactions_results_lookup_list += list(list("name" = product_name, "id" = reaction.type, "bitflags" = bitflags, "reactants" = reagents))
//Master list of ALL reactions that is used in the UI lookup table. This is expensive to make, and we don't want to lag the server by creating it on UI request, so it's cached to send to UIs instantly.
if(!(is_type_in_list(D.type, blacklist)))
GLOB.chemical_reactions_results_lookup_list += list(list("name" = product_name, "id" = D.type, "bitflags" = bitflags, "reactants" = reagents))
// Create filters based on each reagent id in the required reagents list - this is specifically for finding reactions from product(reagent) ids/typepaths.
for(var/id in product_ids)
if(!GLOB.chemical_reactions_list_product_index[id])
GLOB.chemical_reactions_list_product_index[id] = list()
GLOB.chemical_reactions_list_product_index[id] += reaction
// Create filters based on each reagent id in the required reagents list - this is used to speed up handle_reactions()
for(var/id in reaction_ids)
if(!GLOB.chemical_reactions_list_reactant_index[id])
GLOB.chemical_reactions_list_reactant_index[id] = list()
GLOB.chemical_reactions_list_reactant_index[id] += D
break // Don't bother adding ourselves to other reagent ids, it is redundant
///////////////////////////////Main reagents code/////////////////////////////////////////////
@@ -644,10 +670,8 @@
// pass over previous ongoing reactions before handle_reactions is called
transfer_reactions(target_holder)
src.update_total()
target_holder.update_total()
target_holder.handle_reactions()
src.handle_reactions()
return amount
@@ -922,70 +946,65 @@
var/list/cached_reactions = GLOB.chemical_reactions_list_reactant_index
var/datum/cached_my_atom = my_atom
LAZYNULL(failed_but_capable_reactions)
LAZYNULL(previous_reagent_list)
. = 0
var/list/possible_reactions = list()
for(var/datum/reagent/reagent as anything in cached_reagents)
for(var/datum/chemical_reaction/reaction as anything in cached_reactions[reagent.type]) // Was a big list but now it should be smaller since we filtered it with our reagent id
if(!reaction)
continue
LAZYADD(previous_reagent_list, reagent.type)
// I am SO sorry
reaction_loop:
for(var/datum/chemical_reaction/reaction as anything in cached_reactions[reagent.type]) // Was a big list but now it should be smaller since we filtered it with our reagent id
if(!reaction)
continue
if(!reaction.required_reagents)//Don't bring in empty ones
continue
var/list/cached_required_reagents = reaction.required_reagents
var/total_required_reagents = cached_required_reagents.len
var/total_matching_reagents = 0
var/list/cached_required_catalysts = reaction.required_catalysts
var/total_required_catalysts = cached_required_catalysts.len
var/total_matching_catalysts= 0
var/matching_container = FALSE
var/matching_other = FALSE
var/required_temp = reaction.required_temp
var/is_cold_recipe = reaction.is_cold_recipe
var/meets_temp_requirement = FALSE
var/meets_ph_requirement = FALSE
var/granularity = 1
if(!(reaction.reaction_flags & REACTION_INSTANT))
granularity = CHEMICAL_VOLUME_MINIMUM
if(!reaction.required_reagents)//Don't bring in empty ones
continue
for(var/req_reagent in cached_required_reagents)
if(!has_reagent(req_reagent, (cached_required_reagents[req_reagent]*granularity)))
break
total_matching_reagents++
for(var/_catalyst in cached_required_catalysts)
if(!has_reagent(_catalyst, (cached_required_catalysts[_catalyst]*granularity)))
break
total_matching_catalysts++
if(cached_my_atom)
if(reaction.required_container_accepts_subtypes)
matching_container = !reaction.required_container || istype(cached_my_atom, reaction.required_container)
else
matching_container = !reaction.required_container || cached_my_atom.type == reaction.required_container
var/granularity = 1
if(!(reaction.reaction_flags & REACTION_INSTANT))
granularity = CHEMICAL_VOLUME_MINIMUM
if(isliving(cached_my_atom) && !reaction.mob_react) //Makes it so certain chemical reactions don't occur in mobs
matching_container = FALSE
var/list/cached_required_reagents = reaction.required_reagents
for(var/req_reagent in cached_required_reagents)
if(!has_reagent(req_reagent, (cached_required_reagents[req_reagent]*granularity)))
continue reaction_loop
matching_other = reaction.required_other ? reaction.pre_reaction_other_checks(src) : TRUE
var/list/cached_required_catalysts = reaction.required_catalysts
for(var/_catalyst in cached_required_catalysts)
if(!has_reagent(_catalyst, (cached_required_catalysts[_catalyst]*granularity)))
continue reaction_loop
else
if(!reaction.required_container)
matching_container = TRUE
if(!reaction.required_other)
matching_other = TRUE
if(cached_my_atom)
if(reaction.required_container)
if(reaction.required_container_accepts_subtypes && !istype(cached_my_atom, reaction.required_container))
continue
else if(cached_my_atom.type != reaction.required_container)
continue
if(required_temp == 0 || (is_cold_recipe && chem_temp <= required_temp) || (!is_cold_recipe && chem_temp >= required_temp))
meets_temp_requirement = TRUE
if(isliving(cached_my_atom) && !reaction.mob_react) //Makes it so certain chemical reactions don't occur in mobs
continue
if(((ph >= (reaction.optimal_ph_min - reaction.determin_ph_range)) && (ph <= (reaction.optimal_ph_max + reaction.determin_ph_range))))
meets_ph_requirement = TRUE
else if(reaction.required_container)
continue
if(total_matching_reagents == total_required_reagents && total_matching_catalysts == total_required_catalysts && matching_container && matching_other)
if(meets_temp_requirement && meets_ph_requirement)
possible_reactions += reaction
else
if(reaction.required_other && !reaction.pre_reaction_other_checks(src))
continue
// At this point, we've passed all the hard restrictions and entered into just the soft ones
// So we're gonna start tracking reactions that COULD be completed on continue, instead of just exiting
var/required_temp = reaction.required_temp
var/is_cold_recipe = reaction.is_cold_recipe
if(required_temp != 0 && (is_cold_recipe && chem_temp > required_temp) || (!is_cold_recipe && chem_temp < required_temp))
LAZYADD(failed_but_capable_reactions, reaction)
continue
if(ph < reaction.optimal_ph_min - reaction.determin_ph_range && ph > reaction.optimal_ph_max + reaction.determin_ph_range)
LAZYADD(failed_but_capable_reactions, reaction)
continue
possible_reactions += reaction
update_previous_reagent_list()
//This is the point where we have all the possible reactions from a reagent/catalyst point of view, so we set up the reaction list
for(var/datum/chemical_reaction/selected_reaction as anything in possible_reactions)
if((selected_reaction.reaction_flags & REACTION_INSTANT) || (flags & REAGENT_HOLDER_INSTANT_REACT)) //If we have instant reactions, we process them here
@@ -1197,11 +1216,6 @@
return TRUE
return FALSE
/datum/reagents/proc/update_previous_reagent_list()
LAZYNULL(previous_reagent_list)
for(var/datum/reagent/reagent as anything in reagent_list)
LAZYADD(previous_reagent_list, reagent.type)
///Old reaction mechanics, edited to work on one only
///This is changed from the old - purity of the reagents will affect yield
/datum/reagents/proc/instant_react(datum/chemical_reaction/selected_reaction)
@@ -1275,6 +1289,7 @@
/datum/reagents/proc/update_total()
var/list/cached_reagents = reagent_list
. = 0 // This is a relatively hot proc.
var/total_ph = 0 // I know I know, I'm sorry
for(var/datum/reagent/reagent as anything in cached_reagents)
if((reagent.volume < 0.05) && !is_reacting)
del_reagent(reagent.type)
@@ -1282,8 +1297,15 @@
del_reagent(reagent.type)
else
. += reagent.volume
total_ph += (reagent.ph * reagent.volume)
total_volume = .
recalculate_sum_ph()
if(!.) // No volume, default to the base
ph = CHEMICAL_NORMAL_PH
return .
//Keep limited // should really be defines
ph = clamp(total_ph/total_volume, 0, 14)
/**
* Applies the relevant expose_ proc for every reagent in this holder
@@ -1301,12 +1323,11 @@
if(isnull(A))
return null
var/list/cached_reagents = reagent_list
if(!cached_reagents.len)
if(!reagent_list.len)
return null
var/list/reagents = list()
for(var/datum/reagent/reagent as anything in cached_reagents)
for(var/datum/reagent/reagent as anything in reagent_list)
reagents[reagent] = reagent.volume * volume_modifier
return A.expose_reagents(reagents, src, methods, volume_modifier, show_message)
@@ -1554,19 +1575,6 @@
return FALSE
reagent.ph = clamp(reagent.ph + value, lower_limit, upper_limit)
/*
* Updates the reagents datum pH based off the volume weighted sum of the reagent_list's reagent pH
*/
/datum/reagents/proc/recalculate_sum_ph()
if(!reagent_list || !total_volume) //Ensure that this is true
ph = CHEMICAL_NORMAL_PH
return
var/total_ph = 0
for(var/datum/reagent/reagent as anything in reagent_list)
total_ph += (reagent.ph * reagent.volume)
//Keep limited
ph = clamp(total_ph/total_volume, 0, 14)
/**
* Outputs a log-friendly list of reagents based on an external reagent list.
*
@@ -444,8 +444,7 @@
. = ..()
if(!istype(exposed_turf) || (reac_volume < 1))
return
new/obj/effect/decal/cleanable/food/salt(exposed_turf)
exposed_turf.spawn_unique_cleanable(/obj/effect/decal/cleanable/food/salt)
/datum/reagent/consumable/blackpepper
name = "Black Pepper"
@@ -615,10 +614,9 @@
if(isspaceturf(exposed_turf))
return
var/obj/effect/decal/cleanable/food/flour/reagentdecal = new(exposed_turf)
reagentdecal = locate() in exposed_turf //Might have merged with flour already there.
if(reagentdecal)
reagentdecal.reagents.add_reagent(/datum/reagent/consumable/flour, reac_volume)
var/obj/effect/decal/cleanable/food/flour/flour_decal = exposed_turf.spawn_unique_cleanable(/obj/effect/decal/cleanable/food/flour)
if(flour_decal)
flour_decal.reagents.add_reagent(/datum/reagent/consumable/flour, reac_volume)
/datum/reagent/consumable/cherryjelly
name = "Cherry Jelly"
@@ -937,9 +937,7 @@
if(isspaceturf(exposed_turf))
return
var/obj/effect/decal/cleanable/dirt/dirt_decal = (locate() in exposed_turf.contents)
if(!dirt_decal)
dirt_decal = new(exposed_turf)
exposed_turf.spawn_unique_cleanable(/obj/effect/decal/cleanable/dirt)
/datum/reagent/chlorine
name = "Chlorine"
@@ -1105,9 +1103,7 @@
if((reac_volume < 3) || isspaceturf(exposed_turf))
return
var/obj/effect/decal/cleanable/greenglow/glow = locate() in exposed_turf.contents
if(!glow)
glow = new(exposed_turf)
var/obj/effect/decal/cleanable/greenglow/glow = exposed_turf.spawn_unique_cleanable(/obj/effect/decal/cleanable/greenglow)
if(!QDELETED(glow))
glow.reagents.add_reagent(type, reac_volume)
@@ -1209,7 +1205,9 @@
if((reac_volume < 5))
return
new /obj/effect/decal/cleanable/fuel_pool(exposed_turf, round(reac_volume / 5))
var/obj/effect/decal/cleanable/fuel_pool/pool = exposed_turf.spawn_unique_cleanable(/obj/effect/decal/cleanable/fuel_pool)
if(pool)
pool.burn_amount = max(min(round(reac_volume / 5), 10), 1)
/datum/reagent/space_cleaner
name = "Space Cleaner"
@@ -2368,7 +2366,7 @@
. = ..()
if(!istype(exposed_turf))
return
new glitter_type(exposed_turf)
exposed_turf.spawn_unique_cleanable(glitter_type)
/datum/reagent/glitter/pink
name = "Pink Glitter"
@@ -2629,7 +2627,7 @@
/datum/reagent/gravitum/expose_obj(obj/exposed_obj, volume)
. = ..()
exposed_obj.AddElement(/datum/element/forced_gravity, 0)
addtimer(CALLBACK(exposed_obj, PROC_REF(_RemoveElement), list(/datum/element/forced_gravity, 0)), volume * time_multiplier)
addtimer(CALLBACK(exposed_obj, PROC_REF(_RemoveElement), list(/datum/element/forced_gravity, 0)), volume * time_multiplier, TIMER_UNIQUE|TIMER_OVERRIDE)
/datum/reagent/gravitum/on_mob_metabolize(mob/living/affected_mob)
affected_mob.AddElement(/datum/element/forced_gravity, 0) //0 is the gravity, and in this case weightless
@@ -2806,9 +2804,10 @@
if((reac_volume <= 10)) // Makes sure people don't duplicate ants.
return
var/obj/effect/decal/cleanable/ants/pests = locate() in exposed_turf.contents
var/obj/effect/decal/cleanable/ants/pests = exposed_turf.spawn_unique_cleanable(/obj/effect/decal/cleanable/ants)
if(!pests)
pests = new(exposed_turf)
return
var/spilled_ants = (round(reac_volume,1) - 5) // To account for ant decals giving 3-5 ants on initialize.
pests.reagents.add_reagent(/datum/reagent/ants, spilled_ants)
pests.update_ant_damage()
+1 -1
View File
@@ -17,7 +17,7 @@
var/required_container_accepts_subtypes = FALSE
/// If required_container_accepts_subtypes is FALSE, the exact type of what container this reaction can take place in. Otherwise, what type including subtypes are acceptable.
var/atom/required_container
/// an integer required for the reaction to happen
/// Set this to true to call pre_reaction_other_checks() on react and do some more interesting reaction logic
var/required_other = FALSE
///Determines if a chemical reaction can occur inside a mob