From c3ae12a55aa7f1fe6ee0b1d4c5fe73ad80da87fb Mon Sep 17 00:00:00 2001 From: SkyratBot <59378654+SkyratBot@users.noreply.github.com> Date: Wed, 6 Dec 2023 00:19:33 +0100 Subject: [PATCH] [MIRROR] Fix get_icon_dimensions being a memory hole where ram goes to die [MDB IGNORE] (#25449) * Fix get_icon_dimensions being a memory hole where ram goes to die (#80129) Icons can be a real file(), a rsc backed file(), a dynamic rsc (dyn.rsc) reference (known as a cache reference in byond docs), or an /icon which is pointing to one of those. You can tell if a isfile() passing icon is a dynamic rsc reference or if its a static file/compile time rsc reference by looking at the stringify version. In icon file references that crc match a dmi that is compiled in the rsc, this will stringify to the compile time path to that dmi. (fun fact, even vv uploaded icons do this, if the file crc matches a file in the rsc the client has, byond doesn't even upload it, it just tells the server to use the rsc version) Runtime generated dynamic icons are an unbounded concept cache identity wise, the same icon can exist millions of ways and holding them in a list as a key can lead to unbounded memory usage if called often by consumers. So now those are not cached. If this proc turns out to be high cost after this change, that can be helped some by overriding /icon/Width() and /icon/Height() and adding a datum level cache to those as well and fixing code that manages icons so that it keeps the icon datum around as much as it can. --------- Co-authored-by: Mothblocks <35135081+Mothblocks@ users.noreply.github.com> * Fix get_icon_dimensions being a memory hole where ram goes to die --------- Co-authored-by: Kyle Spier-Swenson Co-authored-by: Mothblocks <35135081+Mothblocks@ users.noreply.github.com> --- code/__HELPERS/icons.dm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/code/__HELPERS/icons.dm b/code/__HELPERS/icons.dm index 4a60dd23c60..acb03151f1d 100644 --- a/code/__HELPERS/icons.dm +++ b/code/__HELPERS/icons.dm @@ -1477,6 +1477,12 @@ GLOBAL_LIST_EMPTY(transformation_animation_objects) /// Returns a list containing the width and height of an icon file /proc/get_icon_dimensions(icon_path) + // Icons can be a real file(), a rsc backed file(), a dynamic rsc (dyn.rsc) reference (known as a cache reference in byond docs), or an /icon which is pointing to one of those. + // Runtime generated dynamic icons are an unbounded concept cache identity wise, the same icon can exist millions of ways and holding them in a list as a key can lead to unbounded memory usage if called often by consumers. + // Check distinctly that this is something that has this unspecified concept, and thus that we should not cache. + if (!isfile(icon_path) || !length("[icon_path]")) + var/icon/my_icon = icon(icon_path) + return list("width" = my_icon.Width(), "height" = my_icon.Height()) if (isnull(GLOB.icon_dimensions[icon_path])) var/icon/my_icon = icon(icon_path) GLOB.icon_dimensions[icon_path] = list("width" = my_icon.Width(), "height" = my_icon.Height())