Skip to content

Commit

Permalink
Optimizes some gas_mixture procs, Optimizes pipeline processing signi…
Browse files Browse the repository at this point in the history
…ficantly by 33% (#74233)

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](tgstation/tgstation@91da000)

[91da000](tgstation/tgstation@91da000)

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](tgstation/tgstation@bf5a2d2)

[bf5a2d2](tgstation/tgstation@bf5a2d2)

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)
  • Loading branch information
LemonInTheDark authored and JixS4v committed Sep 2, 2024
1 parent 8061855 commit 907c1f4
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 29 deletions.
5 changes: 4 additions & 1 deletion code/__DEFINES/atmospherics/atmos_helpers.dm
Original file line number Diff line number Diff line change
Expand Up @@ -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) };

///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) };
Expand Down
2 changes: 1 addition & 1 deletion code/game/turfs/change_turf.dm
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,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
Expand Down
2 changes: 1 addition & 1 deletion code/modules/atmospherics/environmental/LINDA_turf_tile.dm
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,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)
Expand Down
36 changes: 30 additions & 6 deletions code/modules/atmospherics/gasmixtures/gas_mixture.dm
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ GLOBAL_LIST_INIT(gaslist_cache, init_gaslist_cache())
var/list/analyzer_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
Expand Down Expand Up @@ -158,7 +161,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]

return TRUE
Expand Down Expand Up @@ -251,20 +254,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

Expand All @@ -273,7 +297,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 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
54 changes: 34 additions & 20 deletions code/modules/atmospherics/machinery/datum_pipeline.dm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -14,6 +18,7 @@
other_airs = list()
members = list()
other_atmos_machines = list()
require_custom_reconcilation = list()
SSair.networks += src

/datum/pipeline/Destroy()
Expand Down Expand Up @@ -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("add_machinery_member: Nonexistent (empty list) or null machinery gasmix added to pipeline datum from [considered_component] \
Expand Down Expand Up @@ -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)

Expand All @@ -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

Expand Down Expand Up @@ -255,43 +265,47 @@
continue
gas_mixture_list += pipeline.other_airs
gas_mixture_list += pipeline.air
for(var/atmosmch in pipeline.other_atmos_machines)
if (istype(atmosmch, /obj/machinery/atmospherics/components/binary/valve))
var/obj/machinery/atmospherics/components/binary/valve/considered_valve = atmosmch
if(considered_valve.on)
pipeline_list |= considered_valve.parents[1]
pipeline_list |= considered_valve.parents[2]
else if (istype(atmosmch, /obj/machinery/atmospherics/components/unary/portables_connector))
var/obj/machinery/atmospherics/components/unary/portables_connector/considered_connector = atmosmch
if(considered_connector.connected_device)
gas_mixture_list += considered_connector.connected_device.return_air()
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)

var/total_thermal_energy = 0
var/total_heat_capacity = 0
var/datum/gas_mixture/total_gas_mixture = new(0)


for(var/mixture in gas_mixture_list)
var/datum/gas_mixture/gas_mixture = mixture
total_gas_mixture.volume += gas_mixture.volume
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)
// 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]

ADD_MOLES(giver_id, total_gas_mixture, 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)

0 comments on commit 907c1f4

Please sign in to comment.