From 60d2d2ee1ae4f7a3c8c93e14fdbd6c210a45cf04 Mon Sep 17 00:00:00 2001 From: SkyratBot <59378654+SkyratBot@users.noreply.github.com> Date: Wed, 29 Mar 2023 23:22:08 +0200 Subject: [PATCH] [MIRROR] Optimizes some gas_mixture procs, Optimizes pipeline processing significantly by 33% [MDB IGNORE] (#20118) * Optimizes some gas_mixture procs, Optimizes pipeline processing significantly by 33% (#74233) ## About The Pull Request It is faster to operate on a gas list, especially if cached, then it is to operate on a datum. Doing this cause I'm seeing cost in merge() post #74230 Hits on a few other important places too. self_breakdown and such. Worth it IMO Could in theory go further by caching the global list. I'm tempted I admit but it needs profiling first and it's late EDIT: I have not slept, and have gone tooo far [Micros /gas_mixture/copy and copy_from, adds a new proc to handle copying with a ratio, copy_from_ratio](https://github.com/tgstation/tgstation/pull/74233/commits/91da0003daa9485962525d3e6bc9170a4c09876b) [91da000](https://github.com/tgstation/tgstation/pull/74233/commits/91da0003daa9485962525d3e6bc9170a4c09876b) The ADD_GAS sidestep saves us 0.1 seconds of init (used to at least. Ensuring we don't break archive is gonna have a cost. I don't want to profile this so I'll estimate maybe 0.05 seconds). The faster version of copy_from is just well, better, and helps to avoid stupid [Optimizes pipeline processing](https://github.com/tgstation/tgstation/pull/74233/commits/bf5a2d2d60554da2ce5fa1ac5f6c4179f6208cb2) [bf5a2d2](https://github.com/tgstation/tgstation/pull/74233/commits/bf5a2d2d60554da2ce5fa1ac5f6c4179f6208cb2) I haven't slept in 36 hours. Have some atmos optimizations Pipelines now keep track of components that require custom reconciliation as a seperate list. This avoids the overhead of filtering all connected atmos machinery. Rather then relying on |= to avoid duplicate gas_mixtures, we instead use a cycle var stored on the mix itself, which is compared with a static unique id from reconcile_air() This fully prevents double processing of gas, and should (hopefully) prevent stupid dupe issues in future Rather then summing volume on the gas mixture itself, we sum it in a local var. This avoids datum var accesses, and saves a slight bit of time Instead of running THERMAL_ENERGY() (and thus heat_capacity(), which iterates all gases in the mix AGAIN) when processing gas, we instead just hook into the existing heat capacity calculation done inside the giver gases loop This saves a significant amount of time, somewhere around 30% of the proc, I think? This doesn't tackle the big headache here, which is the copy_from loop at the base of the proc. I think the solution is to convert pipelines to a sort of polling model. Atmos components don't "own" their mix, they instead have to request a copy of it from the pipeline datum. This would work based off a mutually agreed upon volume amount for that component in that process cycle. We'd use an archived system to figure out what gases to give to components, while removing from the real MOLES list. We could then push gas consumption requests to the pipeline, which would handle them, alongside volume changes, on the next process. Not sure how I'd handle connected pipelines... Merging post reconcile maybe? This is a problem for tomorrow though, I need to go to bed. Saves about 30% of pipeline costs. Profiles taken on kilo, until each reconcile_air hits 5000 calls [old.txt](https://github.com/tgstation/tgstation/files/11075118/Profile.results.total.time.txt) [new.txt](https://github.com/tgstation/tgstation/files/11075133/profiler.txt) * Optimizes some gas_mixture procs, Optimizes pipeline processing significantly by 33% --------- Co-authored-by: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com> --- code/__DEFINES/atmospherics/atmos_helpers.dm | 5 ++- code/datums/atmosphere/_atmosphere.dm | 2 +- code/game/turfs/change_turf.dm | 2 +- .../environmental/LINDA_turf_tile.dm | 2 +- .../atmospherics/gasmixtures/gas_mixture.dm | 36 ++++++++++++--- .../gasmixtures/immutable_mixtures.dm | 3 ++ .../machinery/components/components_base.dm | 2 + .../atmospherics/machinery/datum_pipeline.dm | 45 +++++++++++++------ 8 files changed, 74 insertions(+), 23 deletions(-) diff --git a/code/__DEFINES/atmospherics/atmos_helpers.dm b/code/__DEFINES/atmospherics/atmos_helpers.dm index 4b1b4227c7d..7fab38471bf 100644 --- a/code/__DEFINES/atmospherics/atmos_helpers.dm +++ b/code/__DEFINES/atmospherics/atmos_helpers.dm @@ -35,7 +35,10 @@ var/list/tmp_gaslist = GLOB.gaslist_cache[gas_id]; out_list[gas_id] = tmp_gaslist.Copy(); ///Adds a gas to a gas mixture but checks if is already present, faster than the same proc -#define ASSERT_GAS(gas_id, gas_mixture) if (!gas_mixture.gases[gas_id]) { ADD_GAS(gas_id, gas_mixture.gases) }; +#define ASSERT_GAS(gas_id, gas_mixture) ASSERT_GAS_IN_LIST(gas_id, gas_mixture.gases) + +///Adds a gas to a gas LIST but checks if is already present, accepts a list instead of a datum, so faster if the list is locally cached +#define ASSERT_GAS_IN_LIST(gas_id, gases) if (!gases[gas_id]) { ADD_GAS(gas_id, gases) }; //prefer this to gas_mixture/total_moles in performance critical areas ///Calculate the total moles of the gas mixture, faster than the proc, good for performance critical areas diff --git a/code/datums/atmosphere/_atmosphere.dm b/code/datums/atmosphere/_atmosphere.dm index 431a6695753..1dace1dd1b2 100644 --- a/code/datums/atmosphere/_atmosphere.dm +++ b/code/datums/atmosphere/_atmosphere.dm @@ -49,7 +49,7 @@ amount *= pressure_scalar // If we pick a really small target pressure we want roughly the same mix but less of it all amount = CEILING(amount, 0.1) - ASSERT_GAS(gastype, gasmix) + ASSERT_GAS_IN_LIST(gastype, gaslist) gaslist[gastype][MOLES] += amount // That last one put us over the limit, remove some of it diff --git a/code/game/turfs/change_turf.dm b/code/game/turfs/change_turf.dm index 8beccd08e51..f5db0f38f51 100644 --- a/code/game/turfs/change_turf.dm +++ b/code/game/turfs/change_turf.dm @@ -295,7 +295,7 @@ GLOBAL_LIST_INIT(blacklisted_automated_baseturfs, typecacheof(list( var/list/giver_gases = mix.gases for(var/giver_id in giver_gases) - ASSERT_GAS(giver_id, total) + ASSERT_GAS_IN_LIST(giver_id, total_gases) total_gases[giver_id][MOLES] += giver_gases[giver_id][MOLES] total.temperature = energy / heat_cap diff --git a/code/modules/atmospherics/environmental/LINDA_turf_tile.dm b/code/modules/atmospherics/environmental/LINDA_turf_tile.dm index 1252f7985ae..ee2939824f3 100644 --- a/code/modules/atmospherics/environmental/LINDA_turf_tile.dm +++ b/code/modules/atmospherics/environmental/LINDA_turf_tile.dm @@ -483,7 +483,7 @@ var/list/giver_gases = mix.gases for(var/giver_id in giver_gases) - ASSERT_GAS(giver_id, shared_mix) + ASSERT_GAS_IN_LIST(giver_id, shared_gases) shared_gases[giver_id][MOLES] += giver_gases[giver_id][MOLES] if(!imumutable_in_group) diff --git a/code/modules/atmospherics/gasmixtures/gas_mixture.dm b/code/modules/atmospherics/gasmixtures/gas_mixture.dm index 4711ff40a95..16f89d13a96 100644 --- a/code/modules/atmospherics/gasmixtures/gas_mixture.dm +++ b/code/modules/atmospherics/gasmixtures/gas_mixture.dm @@ -30,6 +30,9 @@ GLOBAL_LIST_INIT(gaslist_cache, init_gaslist_cache()) var/list/reaction_results /// Whether to call garbage_collect() on the sharer during shares, used for immutable mixtures var/gc_share = FALSE + /// When this gas mixture was last touched by pipeline processing + /// I am sorry + var/pipeline_cycle = -1 /datum/gas_mixture/New(volume) gases = new @@ -159,7 +162,7 @@ GLOBAL_LIST_INIT(gaslist_cache, init_gaslist_cache()) var/list/giver_gases = giver.gases //gas transfer for(var/giver_id in giver_gases) - ASSERT_GAS(giver_id, src) + ASSERT_GAS_IN_LIST(giver_id, cached_gases) cached_gases[giver_id][MOLES] += giver_gases[giver_id][MOLES] SEND_SIGNAL(src, COMSIG_GASMIX_MERGED) @@ -276,20 +279,41 @@ GLOBAL_LIST_INIT(gaslist_cache, init_gaslist_cache()) ///Creates new, identical gas mixture ///Returns: duplicate gas mixture /datum/gas_mixture/proc/copy() - var/list/cached_gases = gases + // Type as /list/list to make spacemandmm happy with the inlined access we do down there + var/list/list/cached_gases = gases var/datum/gas_mixture/copy = new type var/list/copy_gases = copy.gases copy.temperature = temperature for(var/id in cached_gases) - ADD_GAS(id, copy.gases) - copy_gases[id][MOLES] = cached_gases[id][MOLES] + // Sort of a sideways way of doing ADD_GAS() + // Faster tho, gotta save those cpu cycles + copy_gases[id] = cached_gases[id].Copy() + copy_gases[id][ARCHIVE] = 0 return copy + +///Copies variables from sample +///Returns: TRUE if we are mutable, FALSE otherwise +/datum/gas_mixture/proc/copy_from(datum/gas_mixture/sample) + var/list/cached_gases = gases //accessing datum vars is slower than proc vars + // Type as /list/list to make spacemandmm happy with the inlined access we do down there + var/list/list/sample_gases = sample.gases + + //remove all gases + cached_gases.Cut() + + temperature = sample.temperature + for(var/id in sample_gases) + cached_gases[id] = sample_gases[id].Copy() + cached_gases[id][ARCHIVE] = 0 + + return TRUE + ///Copies variables from sample, moles multiplicated by partial ///Returns: TRUE if we are mutable, FALSE otherwise -/datum/gas_mixture/proc/copy_from(datum/gas_mixture/sample, partial = 1) +/datum/gas_mixture/proc/copy_from_ratio(datum/gas_mixture/sample, partial = 1) var/list/cached_gases = gases //accessing datum vars is slower than proc vars var/list/sample_gases = sample.gases @@ -298,7 +322,7 @@ GLOBAL_LIST_INIT(gaslist_cache, init_gaslist_cache()) temperature = sample.temperature for(var/id in sample_gases) - ASSERT_GAS(id,src) + ASSERT_GAS_IN_LIST(id, cached_gases) cached_gases[id][MOLES] = sample_gases[id][MOLES] * partial return TRUE diff --git a/code/modules/atmospherics/gasmixtures/immutable_mixtures.dm b/code/modules/atmospherics/gasmixtures/immutable_mixtures.dm index 103c2653fd4..ab0517f9d02 100644 --- a/code/modules/atmospherics/gasmixtures/immutable_mixtures.dm +++ b/code/modules/atmospherics/gasmixtures/immutable_mixtures.dm @@ -34,6 +34,9 @@ /datum/gas_mixture/immutable/copy_from() return FALSE //we're immutable. +/datum/gas_mixture/immutable/copy_from_ratio() + return FALSE //we're immutable. + /datum/gas_mixture/immutable/temperature_share(datum/gas_mixture/sharer, conduction_coefficient, sharer_temperature, sharer_heat_capacity) . = ..() temperature = initial_temperature diff --git a/code/modules/atmospherics/machinery/components/components_base.dm b/code/modules/atmospherics/machinery/components/components_base.dm index 4dee3f6d6fb..887d74dcdff 100644 --- a/code/modules/atmospherics/machinery/components/components_base.dm +++ b/code/modules/atmospherics/machinery/components/components_base.dm @@ -130,6 +130,8 @@ parents[i] = null // Disconnects from the machinery side. reference.other_atmos_machines -= src + if(custom_reconcilation) + reference.require_custom_reconcilation -= src /** * We explicitly qdel pipeline when this particular pipeline diff --git a/code/modules/atmospherics/machinery/datum_pipeline.dm b/code/modules/atmospherics/machinery/datum_pipeline.dm index 864227905a3..66ec013d132 100644 --- a/code/modules/atmospherics/machinery/datum_pipeline.dm +++ b/code/modules/atmospherics/machinery/datum_pipeline.dm @@ -4,6 +4,10 @@ var/list/obj/machinery/atmospherics/pipe/members var/list/obj/machinery/atmospherics/components/other_atmos_machines + /// List of other_atmos_machines that have custom_reconcilation set + /// We're essentially caching this to avoid needing to filter over it when processing our machines + var/list/obj/machinery/atmospherics/components/require_custom_reconcilation + ///Should we equalize air amoung all our members? var/update = TRUE @@ -14,6 +18,7 @@ other_airs = list() members = list() other_atmos_machines = list() + require_custom_reconcilation = list() SSair.networks += src /datum/pipeline/Destroy() @@ -121,6 +126,8 @@ /datum/pipeline/proc/add_machinery_member(obj/machinery/atmospherics/components/considered_component) other_atmos_machines |= considered_component + if(considered_component.custom_reconcilation) + require_custom_reconcilation |= considered_component var/list/returned_airs = considered_component.return_pipenet_airs(src) if (!length(returned_airs) || (null in returned_airs)) stack_trace("addMachineryMember: Nonexistent (empty list) or null machinery gasmix added to pipeline datum from [considered_component] \ @@ -156,10 +163,13 @@ air.merge(parent_pipeline.air) for(var/obj/machinery/atmospherics/components/reference_component in parent_pipeline.other_atmos_machines) reference_component.replace_pipenet(parent_pipeline, src) + if(reference_component.custom_reconcilation) + require_custom_reconcilation |= reference_component other_atmos_machines |= parent_pipeline.other_atmos_machines other_airs |= parent_pipeline.other_airs parent_pipeline.members.Cut() parent_pipeline.other_atmos_machines.Cut() + parent_pipeline.require_custom_reconcilation.Cut() update = TRUE qdel(parent_pipeline) @@ -182,7 +192,7 @@ for(var/obj/machinery/atmospherics/pipe/member in members) member.air_temporary = new member.air_temporary.volume = member.volume - member.air_temporary.copy_from(air, member.volume / air.volume) + member.air_temporary.copy_from_ratio(air, member.volume / air.volume) member.air_temporary.temperature = air.temperature @@ -238,11 +248,9 @@ continue gas_mixture_list += pipeline.other_airs gas_mixture_list += pipeline.air - for(var/obj/machinery/atmospherics/components/atmos_machine as anything in pipeline.other_atmos_machines) - if(!atmos_machine.custom_reconcilation) - continue + for(var/obj/machinery/atmospherics/components/atmos_machine as anything in pipeline.require_custom_reconcilation) pipeline_list |= atmos_machine.return_pipenets_for_reconcilation(src) - gas_mixture_list |= atmos_machine.return_airs_for_reconcilation(src) + gas_mixture_list += atmos_machine.return_airs_for_reconcilation(src) var/total_thermal_energy = 0 var/total_heat_capacity = 0 @@ -250,27 +258,38 @@ var/list/total_gases = total_gas_mixture.gases + var/volume_sum = 0 + + var/static/process_id = 0 + process_id = (process_id + 1) % (SHORT_REAL_LIMIT - 1) + for(var/datum/gas_mixture/gas_mixture as anything in gas_mixture_list) - total_gas_mixture.volume += gas_mixture.volume + // Ensure we never walk the same mix twice + if(gas_mixture.pipeline_cycle == process_id) + gas_mixture_list -= gas_mixture + continue + gas_mixture.pipeline_cycle = process_id + volume_sum += gas_mixture.volume // This is sort of a combined merge + heat_capacity calculation var/list/giver_gases = gas_mixture.gases + var/heat_capacity = 0 //gas transfer for(var/giver_id in giver_gases) var/giver_gas_data = giver_gases[giver_id] - ASSERT_GAS(giver_id, total_gas_mixture) + ASSERT_GAS_IN_LIST(giver_id, total_gases) total_gases[giver_id][MOLES] += giver_gas_data[MOLES] - total_heat_capacity += giver_gas_data[MOLES] * giver_gas_data[GAS_META][META_GAS_SPECIFIC_HEAT] + heat_capacity += giver_gas_data[MOLES] * giver_gas_data[GAS_META][META_GAS_SPECIFIC_HEAT] - total_thermal_energy += THERMAL_ENERGY(gas_mixture) + total_heat_capacity += heat_capacity + total_thermal_energy += gas_mixture.temperature * heat_capacity total_gas_mixture.temperature = total_heat_capacity ? (total_thermal_energy / total_heat_capacity) : 0 - + total_gas_mixture.volume = volume_sum total_gas_mixture.garbage_collect() if(total_gas_mixture.volume > 0) //Update individual gas_mixtures by volume ratio - for(var/mixture in gas_mixture_list) - var/datum/gas_mixture/gas_mixture = mixture - gas_mixture.copy_from(total_gas_mixture, gas_mixture.volume / total_gas_mixture.volume) + for(var/datum/gas_mixture/gas_mixture as anything in gas_mixture_list) + gas_mixture.copy_from_ratio(total_gas_mixture, gas_mixture.volume / volume_sum)