-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize sees
by removing reachability cache and fixing LRU cache.
#70546
Merged
Maleclypse
merged 11 commits into
CleverRaven:master
from
prharvey:remove_reachability_cache
Jan 6, 2024
Merged
Optimize sees
by removing reachability cache and fixing LRU cache.
#70546
Maleclypse
merged 11 commits into
CleverRaven:master
from
prharvey:remove_reachability_cache
Jan 6, 2024
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
prharvey
changed the title
Optimise
Optimize Dec 31, 2023
sees
by removing reachability cache and fixing LRU cache.sees
by removing reachability cache and fixing LRU cache.
github-actions
bot
added
Code: Tests
Measurement, self-control, statistics, balancing.
[C++]
Changes (can be) made in C++. Previously named `Code`
Code: Performance
Performance boosting code (CPU, memory, etc.)
json-styled
JSON lint passed, label assigned by github actions
astyled
astyled PR, label is assigned by github actions
BasicBuildPassed
This PR builds correctly, label assigned by github actions
and removed
BasicBuildPassed
This PR builds correctly, label assigned by github actions
labels
Dec 31, 2023
github-actions
bot
added
the
BasicBuildPassed
This PR builds correctly, label assigned by github actions
label
Jan 1, 2024
…plementation with one that uses the skew_vision_cache.
prharvey
force-pushed
the
remove_reachability_cache
branch
from
January 5, 2024 19:28
3ed2fda
to
c262eeb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
astyled
astyled PR, label is assigned by github actions
BasicBuildPassed
This PR builds correctly, label assigned by github actions
[C++]
Changes (can be) made in C++. Previously named `Code`
Code: Performance
Performance boosting code (CPU, memory, etc.)
Code: Tests
Measurement, self-control, statistics, balancing.
json-styled
JSON lint passed, label assigned by github actions
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.
Summary
Performance "Optimize creature vision checks."
Purpose of change
Creature vision checks are incredibly expensive, mostly due to the reachability cache thrashing.
Describe the solution
Removed the reachability cache entirely. It no longer serves a purpose now that reachability zones are used instead. Fixed a bug in the LRU cache implementation that made it an LRI cache instead. Rearrange some predicates in
sees
based on expense; note that this order will likely change again after my micro-optimizations PRs are merged.Creature vision based on light level was also simplified. Looking from the shadows into light uses the max vision range of either light level, looking from light into shadows uses the min vision range of either light level. The current implementation makes no sense whatsoever, and looking at PR history doesn't make it clear at all.Removed and will PR separately.Describe alternatives you've considered
Solve the cache thrashing problem in the reachability cache. Not worth the time given such map spanning caches are expensive to maintain anyways, especially if we are increasing reality bubble size, and it currently has limited utility. A less permissive version of this cache based on octants would be interesting, but it's just way too expensive to generate, currently.
Testing
Unit tests pass. Played for ~30 minutes and didn't notice anything off about monster vision.
Additional context
Note these flame graphs were taken using the new pathfinder, in order to make the change more obvious.
Flame before:
Flame after: