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

Assorted NPC performance fixes #35295

Merged
merged 5 commits into from Nov 29, 2019
Merged

Assorted NPC performance fixes #35295

merged 5 commits into from Nov 29, 2019

Conversation

ghost
Copy link

@ghost ghost commented Nov 3, 2019

Summary

SUMMARY: Performance "Assorted NPC performance fixes"

Purpose of change

A series of small NPC performance fixes, testing shows variance between 10% and 30% improvement in long action/waiting times at the refugee centre.

Describe the solution

  1. There were three places that looped through all worn items, for every player and NPC every turn - one was in character::do_turn() , the other two were in player::do_turn() in process_active_items().
    so what I did was amalgamate these to the same loop, rename process_active_items() to process_Items() in general and place it in player::do_turn()
    I hope this is an ok approach, but checking item flags and looping through worn items is surprisingly expensive, and it makes sense to me, to have one loop that does it, instead of many.
    To support this, I also modified item::get_flag() to use find() instead of count() this should be a bit more efficient.

  2. NPC ai getting confident_shoot_range() was a bit more expensive than it seems, so I cached the value.

  3. Little things like - is_active() check in all_npcs() loop was redundant, as g->all_npcs() returns active NPCs anyway, and is_active() was a few 0.1% of perf.

  4. Moved check for sees(monster) in danger assessment above attitude_to() , no point forming attitude before you even see a monster.

  5. Modified the healing_options flow a bit ( more could be done in this area ) as it was running has_healing_options() and forming a list of what healing items were available, before it was decided there was even a need for healing to be done.

  6. Added some fuzziness to address_needs() - NPCs shouldnt be super-vigilant eagle-eyed lightning-reflexed super-soldiers, now that turns are 1-seconds, it stands to reason that they may not react instantly to things and needs, so ive added an rng to most of the decision checks there, this will gain some performance as it will skip some of the checks some of the time.
    ( this dosnt affect things like sensing danger from monsters or aiming or other combat stuff, mostly just reacting to wounded allies, and deciding to get food etc )

7.Adjust_worn() was running every turn to check if a splint on a broken limb needed to be swapped sides, this was iterating over all worn items. - I added a check for any broken limb first to return early.

  1. added some !is_npc() checks in player::do_turn() to skip certain sections - NPCs dont need to process morale for the NOMAD trait, nor do they need to process certain things related to scent or stamina ( although they may need to do so in future )

Let me know if any of these things should be split out into seperate PRs.

let me know if any of these things make no sense to you and really shouldnt be included.

Let me know if you know for sure that this will break something.

The bits I edited in player.cpp in particular - is not really my code area, and I don't know 100% that the changes are ok, but they seem ok in testing.

Describe alternatives you've considered

There is more to be done here, I am sure of it - I will keep trying to find some improvements.

Testing

Waited in the refugee center, walked around, put clothes on and took clothes off, to make sure the player::do_turn() and check_encumbrance() stuff was still ok.

Found significant speedups in the refugee center.

Additional context

N/A

@ghost ghost changed the title Assorted NPC performance fixes [CR]Assorted NPC performance fixes Nov 3, 2019
@kevingranade
Copy link
Member

Moved check for sees(monster) in danger assessment above attitude_to() , no point forming attitude before you even see a monster.

I'm a bit sceptical of this one, I'm pretty sure attitude_to() is cheaper than seen()

@ghost
Copy link
Author

ghost commented Nov 4, 2019

Moved check for sees(monster) in danger assessment above attitude_to() , no point forming attitude before you even see a monster.

I'm a bit sceptical of this one, I'm pretty sure attitude_to() is cheaper than seen()

Tested and your right, theres not much in it, but yeah, ill put that back.

@curstwist curstwist added Code: Performance Performance boosting code (CPU, memory, etc.) NPC / Factions NPCs, AI, Speech, Factions, Ownership [C++] Changes (can be) made in C++. Previously named `Code` labels Nov 4, 2019
@ghost ghost requested a review from mlangsdorf November 4, 2019 18:52
src/npcmove.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@mlangsdorf mlangsdorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's going to be a merge conflict between this and the suffer() refactor, but we can sort it out when one of them merges.

src/npcmove.cpp Outdated Show resolved Hide resolved
src/npcmove.cpp Show resolved Hide resolved
src/npc.cpp Outdated Show resolved Hide resolved
src/player.cpp Outdated Show resolved Hide resolved
src/player.cpp Outdated Show resolved Hide resolved
src/npc.cpp Show resolved Hide resolved
src/item.cpp Outdated Show resolved Hide resolved
davidpwbrown added 2 commits November 10, 2019 19:56
revert erroneous merge

more perf

more perf adjustments

astyle

remove range cache invalidation from starting_weapon()

revert sees(monster) check to wher eit was

use is_limb_broken() function
@ghost
Copy link
Author

ghost commented Nov 10, 2019

I've rebased, and for now, I'll leave out the parts that I added to sufer() , now that suffer lives in character::sufffer.cpp , as I can do that in another PR, once ive got to grips with the new structure,

@ghost ghost changed the title [CR]Assorted NPC performance fixes Assorted NPC performance fixes Nov 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants