tgstation / dev-cycles-initiative Goto Github PK

Even with lazy asset loading, every preference still creates lots of icons in their init_possible_values() functions. init_possible_values(), of many preferences, returns both the asset name AND the icons.

Estimated Cost

/datum/asset/json/preferences/generate is 3.3s with CACHE_ASSETS on (not sure how much higher off). Of that 3.3s, 2.21s (67%) is hairstyle/compile_constant_data. 553ms is choiced/compile_constant_data generally, followed by 484ms of facial_hairstyle/compile_constant_data.

Solutions

init_possible_values() can be changed to only give text values, with icon generation being a separate proc. This would be accompanied by a unit test to make sure every value has an associated icon (if icons are applicable to the setting). This would, ideally, defer costs until the preferences menu is loaded when clicking setup character (since preference asset generation is lazy, even when uncached).

This is to facilitate reconnects, and may not be removable, but even if it is not I still want to document this just so we understand what the cost of everything is.

Old messages apparently.

Cables connect VISUALLY with machines by looping objects in adjacent turfs inside UPDATE_ICON_STATE

Wastes cpu time, machines should just mark the cable below them

Similar vein. When connecting cables for the powernet, each pair of cables call update_appearance on each other. A-B is the same as B-A but both operations still happen. We should batch the appearance updates during init, and introduce some sort of state timer like atmos uses, so a cable will know just to not talk to its neighbor since it won't do anything interesting.

Every storage item, such as toolboxes and backpacks (but not closets/crates) initializes all their contents immediately.

Estimated Cost

It's hard to estimate the cost because PopulateContents is tainted by objects like GAGS screwdrivers. The cost is AT MOST 846ms, but more likely around 700ms.

Solutions

The most obvious solution is to lazily initialize the contents of storage until requested. This would fix the issue, but it's not clear if the fragility of this approach is worth the cost.

Metal, glass, coil, circuitboards. Will investigate later the cost. Cells are considered stock parts.

Every subsystem that initializes calls log_world to notify both DD and the players that initializations have completed for the subsystem.

Estimated Cost

log_world totals to 845ms over 76 calls. This can very much differ because I/O is funky.

Solutions

Keeping initializations somewhere they can be read in real time, without opening the game, is a 100% must--that's what fuels a lot of this work to begin with! It's unclear what the solution for this is, but it is logged here regardless.

it doesn't need to
costs like 0.3 to 0.2 seconds

Wizard's den and nuke ops base should only exist when they are actually needed.

Cost is currently unknown, but removing CTF maps saved several seconds.

Potential issue here is people meta'ing that these have spawned based on roundstart times, which did happen in the past with Clock Cult. I don't think it's an enormous deal as long as they keep it to themselves?

cardboard_cutout/Initialize, despite being called twice, is 1.47 seconds, from /proc/generate_cardboard_cutouts, which generates every single cutout using flat icons. These should be changed into normal appearances, or at the very least not be done all at once.

Every atom with integrity in the game calls getArmor, which maps armor to singletons. This includes armor lists, of which nearly 500 exist in typedefs.

Estimated Cost

getArmor is 285ms over 67,671 calls.

Solutions

Armor has always been a pain point for maintainers since it's very hard to tell why values are the way there are. Ideally there would be no armor lists and instead, you would do armor = /datum/armor/fireproof or similar. Anything that needs custom armor could make its own subtype. These could then be used as keys to a singleton. Customizing armor for admins is already not done by editing armor directly and thus there would be no (necessary) difference in UX there.

/datum/greyscale_layer/icon_state/Initialize(icon_file)
	. = ..()
	var/list/icon_states = icon_states(icon_file)
	if(!(icon_state in icon_states))
		CRASH("Configured icon state \[[icon_state]\] was not found in [icon_file]. Double check your json configuration.")

This code should only run behind a focus test.

However, just doing that would make the artist UX noticably worse. To counter this, we should have a GAGS debug panel that would show you this same information when clicked. We can make this interface better than a general runtime ever could.

Mapped closets, used here to refer to closets that have contents on the map that are then put inside, do not benefit from closet lazy loading. The mapped closets we have contribute 275ms+ of init time without ruins, and ignoring GAGS.

This is also nearly all of the cost of #4 (and potentially #19?) since I learned that the machine guns at CentCom are all inside security closets.

Anything common or particularly expensive should be put inside a typepath. Good for mappers anyway.

Profiling code, check init_as_closet_item in Tracy/BYOND profiler.

diff --git a/code/controllers/subsystem/atoms.dm b/code/controllers/subsystem/atoms.dm
index 34eb52f4cda..d8bac77e3b5 100644
--- a/code/controllers/subsystem/atoms.dm
+++ b/code/controllers/subsystem/atoms.dm
@@ -119,6 +119,9 @@ SUBSYSTEM_DEF(atoms)
 
 	testing("Initialized [count] atoms")
 
+/proc/init_as_closet_item(atom/A, list/arguments)
+	return A.Initialize(arglist(arguments))
+
 /// Init this specific atom
 /datum/controller/subsystem/atoms/proc/InitAtom(atom/A, from_template = FALSE, list/arguments)
 	var/the_type = A.type
@@ -131,7 +134,13 @@ SUBSYSTEM_DEF(atoms)
 	var/start_tick = world.time
 	#endif
 
-	var/result = A.Initialize(arglist(arguments))
+	var/result
+
+	var/obj/structure/closet/found_closet
+	if (ismovable(A) && !istype(A, /obj/structure/closet) && (found_closet = locate() in A.loc)?.insertion_allowed(A))
+		result = init_as_closet_item(A, arguments)
+	else
+		result = A.Initialize(arglist(arguments))
 
 	#ifdef UNIT_TESTS
 	if(start_tick != world.time)
diff --git a/code/controllers/subsystem/processing/greyscale.dm b/code/controllers/subsystem/processing/greyscale.dm
index 0c0db7b4f70..cb56ffa6931 100644
--- a/code/controllers/subsystem/processing/greyscale.dm
+++ b/code/controllers/subsystem/processing/greyscale.dm
@@ -34,6 +34,9 @@ PROCESSING_SUBSYSTEM_DEF(greyscale)
 		configurations[i].Refresh(TRUE)
 
 /datum/controller/subsystem/processing/greyscale/proc/GetColoredIconByType(type, list/colors)
+	if (TRUE)
+		return icon('icons/testing/greyscale_error.dmi')
+
 	if(!ispath(type, /datum/greyscale_config))
 		CRASH("An invalid greyscale configuration was given to `GetColoredIconByType()`: [type]")
 	type = "[type]"

/// Checks if this atom uses the GAGS system and if so updates the worn and inhand icons
/obj/item/update_greyscale()
	. = ..()
	if(!greyscale_colors)
		return
	if(greyscale_config_worn)
		worn_icon = SSgreyscale.GetColoredIconByType(greyscale_config_worn, greyscale_colors)
	if(greyscale_config_inhand_left)
		lefthand_file = SSgreyscale.GetColoredIconByType(greyscale_config_inhand_left, greyscale_colors)
	if(greyscale_config_inhand_right)
		righthand_file = SSgreyscale.GetColoredIconByType(greyscale_config_inhand_right, greyscale_colors)

These generations, which are not immediately necessary, are called for every single greyscale item, and costs 0.6s.

These could all be deferred, but that sounds extremely clunky. This will likely be mostly solved by #9, but I wanted to document this cost nonetheless

As a proof of concept, I made the following patch:

diff --git a/code/controllers/subsystem/mapping.dm b/code/controllers/subsystem/mapping.dm
index 59193ce30c5..db5e698feff 100644
--- a/code/controllers/subsystem/mapping.dm
+++ b/code/controllers/subsystem/mapping.dm
@@ -794,6 +794,7 @@ GLOBAL_LIST_EMPTY(the_station_areas)

        var/old_max = max_plane_offset
        max_plane_offset = max(max_plane_offset, current_level)
+       global:max_plane_offset = max_plane_offset
        if(max_plane_offset == old_max)
                return

diff --git a/code/game/turfs/open/space/space.dm b/code/game/turfs/open/space/space.dm
index 7d16f18e19d..45745366b8c 100644
--- a/code/game/turfs/open/space/space.dm
+++ b/code/game/turfs/open/space/space.dm
@@ -26,6 +26,8 @@

        force_no_gravity = TRUE

+       var/static/max_plane_offset = 0
+
 /turf/open/space/basic/New() //Do not convert to Initialize
        SHOULD_CALL_PARENT(FALSE)
        //This is used to optimize the map loader
@@ -47,13 +49,8 @@
        SHOULD_CALL_PARENT(FALSE)
        air = space_gas

        // We make the assumption that the space plane will never be blacklisted, as an optimization
-       if(SSmapping.max_plane_offset)
+       if(max_plane_offset)
                plane = PLANE_SPACE - (PLANE_RANGE * SSmapping.z_level_to_plane_offset[z])

        var/area/our_area = loc

...and observed a 76ms boost. Not anything too crazy, and may not even be worth it for how weird this approach is for refactors, but interesting if this could be done in a way that isn't as ugly as the global: trick (just need a real space reference). Figured I would document this but not really be too interested in implementing this.

cell/Initialize is 1,652 counts for 292.55ms.
emergency_light/Initialize is 1,080 counts for 195ms.

Making power cells lazy is absurdly difficult and absolutely not worth it for the cost, but changing this one instance would save 195ms, bringing cell/Initialize down to 98ms (where we can optimize it further through optimizing reagent creation, appearance updating, etc).

Similar to #4, ammo boxes currently create all their bullets at the same time, instead of on demand when necessary.

Estimated Cost

/obj/item/ammo_box/proc/top_off is 68ms, but likely much more unconfig'd.

Stacks can form anywhere. Even inside machines

This is really stupid. IDK how much init it costs but we should really fix it it's a waste

This is called nearly 200 times unconfig'd and is 83ms.

/obj/machinery/door/airlock/proc/update_other_id()
    for(var/obj/machinery/door/airlock/Airlock in GLOB.airlocks)
        if(Airlock.closeOtherId == closeOtherId && Airlock != src)
            if(!(Airlock in close_others))
                close_others += Airlock
            if(!(src in Airlock.close_others))
                Airlock.close_others += src

Adding verbs is very costly. Doing verbs += list(...) makes it slightly faster, but these aren't necessary at all and alt-click and alt-right-click work perfectly fine and are likely overwhelmingly preferred by players.

Initializing global variables takes about 0.2 seconds. It doesn't show up in Tracy because the ordering is weird. Add dedicated tracking, and figure out if we can improve the performance.

This is called every time a bespoke element is added.

Estimated Cost

As of December 2, 2022, /datum/controller/subsystem/processing/dcs/proc/GetIdFromArguments is 793ms.

Solutions

GetIdFromArguments exists to ensure that elements with the same parameters always resolve to the same instance. Prior efforts to try to guarantee this statically have not been successful with edge cases.

It is also possible to cache this ID where it is known not to change.

It is not obvious (to me, anyway) how this could be done without introducing a footgun, breaking our 99th percentile goal.

Space. There's a lot of it.

There's one last piece of /turf/open/space/Initialize that isn't hyper-optimized, setting the dynamic icon_state.

Estimated Cost

Though not in its own proc, the icon_state set has been measured at 707ms.

Solutions

The icon state is only used for non-parallax space, which is a rarely used setting, and is more commonly seen in something like camera consoles. A single, nice, tiled icon state should be used. This change is unnoticable to nearly all players.

Guns create every bullet inside them before they are used.

Estimated Cost

/obj/item/gun/ballistic/automatic/wt550/Initialize is 54.22ms. This is from 11 creations in CentCom.

/obj/item/gun/energy/Initialize, with 48 calls, is 35.27ms.

Solutions

The obvious solution is only creating bullets when they are necessary. Gun code is extremely old and so this may not be super trivial.

Some interim solutions could be to just put the guns inside lockers, which lazily create their contents, but this conflicts with our goal for efforts of this project to be invisible to players, and could present real balance implications.

Asset loading is damn slow, even with the delaying we do. Delaying is dumb as heck too.

Estimated Cost

6 seconds

Solution

New with 515 we gain the ability to pass references into html, and have the html render it for us "client side"
We can't make a change taking advantage of this until 515 is the default, but when it is we could add support for this feature to tgui, and start replacing our uses of spritesheets with appearances.

This would work for small things like "show me this fish", and even larger ones like the antagonist buttons, since an appearance can be transformed, filtered, colored, etc.

Would be nice to get a daily profiler report (and potentially Tracy, when I make it a build target) from CI, would help isolate huge new costs

We spend like 0.5 seconds on regexing map files
I bet we could tell rust to apply the regex to the file, and then hand byond a string we can json decode to pull up the info
I'd expect this to save like 0.5 seconds

look for regex/Find()

Lighting is pretty slow on its own, but there aren't a great many potential methods to speed it up.
Something that does come to mind is the color matrix work we do

Whenever a turf's lighting changes, we need to do two overlay operations. These require "flattening" mutable appearances, and involve modifying the turf's appearance, which is slow on its own.

What we used to do was stick lighting "objects" on each turf
But that comes with a persistenct sendmaps cost, which scales with pop and is therefor not worth it with our current lighting churn rates (thank you overlay lighting)

Estimated cost

I think it was like 0.7 seconds, tho it's hard to say for sure and I am kinda pulling that number out of my ass

Solutions

Outside of just making lighting faster somehow, if we got to a point where sendmaps costs were less important (say if it was threaded effectively) we could return to the object model and save a good chunk
This would likely require 515, and even then may not be possible. we'll have to see

Shrink to 32x32.

species/proc/on_species_gain is 358ms over 26 calls (config'd, more without because of prefs menu). Median time per call is 13.8ms.

The cost of this is 80% from species/proc/replace_body, and 19% from species/proc/regenerate_organs.

74% of species/proc/replace_body is /obj/item/bodypart/proc/replace_limb (182ms over 156 calls). Of this, 53% is try_attach_limb, 46% is drop_limb.

The cost for both eventually ends up being mostly in carbon/update_body -> carbon/proc/update_body_parts -> getting icons, updating overlays, removing overlays, etc etc etc. It's an enormous waste of processing power.

Savings amount (.182) is based off of replace_limb.

Every machine initializes all of its machine parts immediately.

Estimated Cost

/obj/item/circuitboard/machine/apply_default_parts is 280ms.

Solutions

Machine parts should be instantiated only when necessary. This in and of itself is simple enough, however an issue arises with an extremely common pattern of:

for (var/obj/item/stock_parts/capacitor/capacitor in machine_parts)
    power += capacitor.capacitating_power

This pattern is seen for every stock part. You could change a lot of this by turning them into datums, and then a find-and-replace should work, but it makes the change tricky.

Power cells would likely need to always exist.

GAGS has to perform very expensive icon operations for every color variation that is needed. This creates a very nice interface, where you can just slot in icon, but it carries a huge weight on init times.

Worse, the cost of GAGS makes profiling the cost of anything related to GAGS harder to measure. More on this later.

Estimated Cost

/datum/controller/subsystem/processing/greyscale/proc/GetColoredIconByType is 1.46 seconds of init time.

Howver, 91% of calls are only 22ms. The rest are fairly scattered at the end. This is most likely the cache.

The bulk cost is simply in icon operations. GetPixel (1.19s cost, from /datum/greyscale_config/proc/GenerateBundle) especially is very slow, but because it is serving its purpose of forcing the icon to generate--any icon operation would be doing the same.

Solutions

It is possible to precompile statically-inferrable GAGS usages. As in, everything we can predict is going to use GAGS (like from constant greyscale_config and greyscale_colors values) can be put into a .dmi, pushing the costs away from init time. This would come with an extra compile time cost, but it could only be incurred when the inputs are different, like if a .dmi changes or a variable changes. Dynamic generation still exists, but we statically create as much as we can. There is a prototype of this in Rust, but we could even potentially have a DM backend for this and run dd.exe when it comes out to use it.

It is also potentially possible for objects to choose between generating icons and using something like overlays, when applicable. However this could carry non-negligible runtime costs from either adding the overlays, or maptick costs if vis_contents was used. It also is under the assumption the bulk of this cost is in these simple icons, which we have not yet proven.

What sucks

\ref is real slow, at least in the scale of qdel. This is primarily because adding a getting ref requires inserting a new string into the string tree, and we bloat that sucker to hell.
We use refs for SSgarbage's queue, because it lets us assert that byond's garbage collector will do its work if there are no other refs remaining

How fix

515 adds a var that gives us the amount of references an object has remaining. We could leverage this alongside some information we know about how many refs an object "should" have by that point to save the cost of all those \refs

That works out to about 1500ms

incrementMaxZ increases the max-z by 1 every time its called. This is twice as slow as incrementing the max-z with the number of z-levels we expect to use. That number should be precalculated and preallocated, then go back to incrementing when that is done.

Code that creates NTNet interfaces, broadcasts signals, etc is very slow.

Estimated Cost

• /datum/component/ntnet_interface/Initialize is 125.11ms.
• /obj/proc/init_network_id is 114.75ms over 1,076 calls.
  • Most of this is in log_telecomms (81.72ms) and is easily fixable. There is a path where it replaces the current network with a new one, which is ran regardless of if the networks are the same, which happens thousands of times in init. Check telecomms.log of any round!

Solutions

NTNet and the general radio framework is an enormous piece of code that is completely invisible to players, as there is no interesting way to utilize NTNet itself. There exist things like the NTNet circuit components, but they are on their own isolated network and can be trivially replaced.

NTNet and the radio frequencies from atmos stuff can be completely removed. Aside from that, fixing the log_telecomms will lower a significant cost.