Ports various area/turf performance improvements from /tg/ #2493
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
/datum/proc/process() does nothing and having it in the top 100 called because of this one line is silly.
Goes through and changes some `in area` / `in a` loops to use `get_contained_turfs` to cut down on `in_world` loops. Saves some free lag. :cl: Melbert fix: Some things which affect everything in an area are less laggy, the "all lights are broken" station trait especially /:cl:
## About The Pull Request I realize that we don't really explain anywhere what a lazylist actually is, and being how important of a tool they are I don't think we should rely on word of mouth for them. So I just added a block comment above the macros that people can read about. (We could totally do `#define LAZYLIST list` and then use it in definitions as `var/LAZYLIST/mylist`, so people can control-click the lazylist part and then see the documentation, but I dunno, whatever.) Feel free to suggest changes of wording, add information, or correct any misinformation.
…(#80941) Situation: areas have a list of all turfs in their area. Problem: `/area/space` is an area and has a 6 to 7 digit count of turfs that has to be traversed for every turf we need to remove from it. This can take multiple byond ticks just to preform this action for a single space rune Solution: split the list by zlevel, and only search the right zlevel list when removing turfs from areas. replaces `area.get_contained_turfs()` with a few new procs: * `get_highest_zlevel()` - returns the highest zlevel the area contains turfs in. useful for use with `get_turfs_by_zlevel` * `get_turfs_by_zlevel(zlevel)` - returns a list of turfs in the area in a given zlevel. Useful for code that only cares about a specific zlevel or changes behavior based on zlevel like lighting init. * `get_turfs_from_all_zlevels()` - the replacement for `get_contained_turfs()`, renamed as such so anybody copying/cargo culting code gets a hint that a zlevel specific version might exist. Still used in for loops that type checked so byond would do that all at once * `get_zlevel_turf_lists()` - returns the area's zlevel lists of lists but only for non-empty zlevels. very useful for for loops. The area contents unit test has been rewritten to ensure any improper data triggers failures or runtimes by not having it use the helpers above (some of which ensure a list is always returned) and access the lists directly.
Literally just implements my reviews from #80941 I am frankly a smidge pissed that the pr was merged without them being handled. No code is worth merging past known issues, and if the author is just gonna dip then that's life. I don't like privileging mso on stuff like this, especially because frankly I'm kinda mad at him rn but also because when a pr is made the onus on finishing it falls to the person who made it. Should not need to clean up after someone as a maintainer, and shouldn't normalize doing it. I'm not like mad at zypher directly mind he offered to do this too, just the idea he was espousing here.
This was referenced Sep 6, 2024
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
About The Pull Request
Ports the following PRs from /tg/:
benchmarked on blueshift: 113.8s before, 112.2s after. within margin of error, but can't hurt.
Changelog
No user-facing changes