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

Easy performance gains in expensive Character::update_bodytemp #774

Closed
Coolthulhu opened this issue Aug 12, 2021 · 5 comments
Closed

Easy performance gains in expensive Character::update_bodytemp #774

Coolthulhu opened this issue Aug 12, 2021 · 5 comments

Comments

@Coolthulhu
Copy link
Member

According to @olanti-p's profiling of a save provided by @Kaylakaze, Character::update_bodytemp takes a surprising amount of time, most of which is easily fixable:

  • Character::get_armor_fire() is called (for every body part) even when blister_count == 0
  • Character::in_climate_control() checks O(n^2) items/flags because it calls Character::worn_with_flag in a loop rather than just once
  • Character::warmth and Character::get_wind_resistance should return a whole map of body_part->that_thing_they_calculate, to avoid going through all worn items for every body part

Additionally, Character::next_climate_control_check is very ugly in that it lets you "keep" climate control for as many as 40 seconds after losing it, or force you to wait 20 seconds to get it. But that's a minor problem compared to performance.

@anothersimulacrum
Copy link
Contributor

Only the topic of easy optimizations, you could also pick up CleverRaven/Cataclysm-DDA#48094

for( auto it : mutation_branch::get_all() ) {

@anothersimulacrum
Copy link
Contributor

anothersimulacrum commented Aug 13, 2021

You can also get a fair bit of extra performance out of Character::get_wind_resistance

Cataclysm-BN/src/character.cpp

Lines 3684 to 3690 in ac3b98b

if( i.made_of( material_id( "leather" ) ) || i.made_of( material_id( "plastic" ) ) ||
i.made_of( material_id( "bone" ) ) ||
i.made_of( material_id( "chitin" ) ) || i.made_of( material_id( "nomex" ) ) ) {
penalty = 10; // 90% effective
} else if( i.made_of( material_id( "cotton" ) ) ) {
penalty = 30;
} else if( i.made_of( material_id( "wool" ) ) ) {

by not constructing those material ids on every call, e.g.

diff --git a/src/character.cpp b/src/character.cpp
index 1152a5a5a4..55b2078be6 100644
--- a/src/character.cpp
+++ b/src/character.cpp
@@ -4771,6 +4771,13 @@ bool Character::in_climate_control()
 std::map<bodypart_id, int> Character::get_wind_resistance( const std::map <bodypart_id,
         std::vector<const item *>> &clothing_map ) const
 {
+    static const material_id m_leather( "leather" );
+    static const material_id m_plastic( "plastic" );
+    static const material_id m_bone( "bone" );
+    static const material_id m_chitin( "chitin" );
+    static const material_id m_nomex( "nomex" );
+    static const material_id m_cotton( "cotton" );
+    static const material_id m_wool( "wool" );

     std::map<bodypart_id, int> ret;
     for( const bodypart_id &bp : get_all_body_parts() ) {
@@ -4795,13 +4802,12 @@ std::map<bodypart_id, int> Character::get_wind_resistance( const std::map <bodyp

         for( const item *it : on_bp.second ) {
             const item &i = *it;
-            if( i.made_of( material_id( "leather" ) ) || i.made_of( material_id( "plastic" ) ) ||
-                i.made_of( material_id( "bone" ) ) ||
-                i.made_of( material_id( "chitin" ) ) || i.made_of( material_id( "nomex" ) ) ) {
+            if( i.made_of( m_leather ) || i.made_of( m_plastic ) || i.made_of( m_bone ) ||
+                i.made_of( m_chitin ) || i.made_of( m_nomex ) ) {
                 penalty = 10; // 90% effective
-            } else if( i.made_of( material_id( "cotton" ) ) ) {
+            } else if( i.made_of( m_cotton ) ) {
                 penalty = 30;
-            } else if( i.made_of( material_id( "wool" ) ) ) {
+            } else if( i.made_of( m_wool ) ) {
                 penalty = 40;
             } else {
                 penalty = 1; // 99% effective

image
image

I am going to convert it to be a property of the materials for DDA (avoiding this problem), but you may want to do differently.

@Kaylakaze
Copy link
Contributor

It really bothers me that these materials and their effectiveness ratings are hardcoded :-/

But hardcoding anything like that really bothers me, in general.

(Obviously not your fault, nor am I suggesting you fix it. Just a note.)

@anothersimulacrum
Copy link
Contributor

Consider also using a cata::list for Character::worn (or maybe player::).

@scarf005
Copy link
Member

resolved by #840

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants