Skip to content
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

Performance: Cache NPC goal #54312

Merged
merged 1 commit into from
Jan 12, 2022
Merged

Conversation

dseguin
Copy link
Member

@dseguin dseguin commented Jan 12, 2022

Summary

None

Purpose of change

While testing NPCs in #54307 I noticed the game was running pretty slow:

perf_before

Describe the solution

It looks like overmapbuffer::find_closest is a little too expensive to be running every turn. I decided to cache this value, and refresh it when the NPC moves between OMTs and on game load. In-game, the turn speed went from ~5 seconds/turn to ~2 turns/second (x10 improvement). This is compiled with all debug flags and NOOPT=1.

perf shows a pretty significant improvement:

perf_after

Describe alternatives you've considered

I guess npc::long_term_goal_action could be delayed for X turns before being called again (not sure about the repercussions of that).

Testing

Stepping through npc::set_omt_destination, the goal is cached for each need. Moving OMTs forces the goal to be recalculated (which is noticeable in-game, being a bit slower). The location candidates for each need don't change anyway, so there's no benefit from running this check every time.

Additional context

This is mostly a problem for NPCs without a set destination (which is most NPCs, apparently).

@NetSysFire NetSysFire added [C++] Changes (can be) made in C++. Previously named `Code` Code: Performance Performance boosting code (CPU, memory, etc.) NPC / Factions NPCs, AI, Speech, Factions, Ownership labels Jan 12, 2022
@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jan 12, 2022
@dseguin dseguin marked this pull request as ready for review January 12, 2022 01:59
The goal cache is invalidated when the NPC moves to a different OMT.
@kevingranade kevingranade merged commit 6ae3e36 into CleverRaven:master Jan 12, 2022
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 12, 2022
@dseguin dseguin deleted the cache_npc_goal branch January 12, 2022 17:32
scarf005 pushed a commit to scarf005/Cataclysm-BN that referenced this pull request Dec 8, 2024
The goal cache is invalidated when the NPC moves to a different OMT.

see: CleverRaven/Cataclysm-DDA#54312

Co-authored-by: David Seguin <davidseguin@live.ca>
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.) json-styled JSON lint passed, label assigned by github actions NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants