Merge pull request #44544 from Aivean/optimize-rot-check

Improve performance of materials::get_rotting
CleverRaven · Oct 2, 2020 · 602e91b · 602e91b
2 parents f64f02f + 91af4a8
commit 602e91b
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 5 deletions.
diff --git a/src/generic_factory.h b/src/generic_factory.h
@@ -473,6 +473,38 @@ class generic_factory
             return obj( id ).id;
         }
         /**@}*/
+
+        /**
+         * Wrapper around generic_factory::version.
+         * Allows to have local caches that invalidate when corresponding generic factory invalidates.
+         * Note: when created using it's default constructor, Version is guaranteed to be invalid.
+        */
+        class Version
+        {
+                friend generic_factory<T>;
+            public:
+                Version() = default;
+            private:
+                Version( int64_t version ) : version( version ) {}
+                int64_t  version = -1;
+            public:
+                bool operator==( const Version &rhs ) const {
+                    return version == rhs.version;
+                }
+                bool operator!=( const Version &rhs ) const {
+                    return !( rhs == *this );
+                }
+        };
+
+        // current version of this generic_factory
+        Version get_version() {
+            return Version( version );
+        }
+
+        // checks whether given version is the same as current version of this generic_factory
+        bool is_valid( const Version &v ) {
+            return v.version == version;
+        }
 };
 
 /**

diff --git a/src/material.cpp b/src/material.cpp
@@ -308,12 +308,20 @@ material_list materials::get_compactable()
 
 std::set<material_id> materials::get_rotting()
 {
-    material_list all = get_all();
-    std::set<material_id> rotting;
-    for( const material_type &m : all ) {
-        if( m.rotting() ) {
-            rotting.emplace( m.ident() );
+    static generic_factory<material_type>::Version version;
+    static std::set<material_id> rotting;
+
+    // freshly created version is guaranteed to be invalid
+    if( !material_data.is_valid( version ) ) {
+        material_list all = get_all();
+        rotting.clear();
+        for( const material_type &m : all ) {
+            if( m.rotting() ) {
+                rotting.emplace( m.ident() );
+            }
         }
+        version = material_data.get_version();
     }
+
     return rotting;
 }
diff --git a/tests/generic_factory_test.cpp b/tests/generic_factory_test.cpp
@@ -109,6 +109,32 @@ TEST_CASE( "generic_factory_common_null_ids", "[generic_factory]" )
     CHECK( field_type_str_id::NULL_ID().is_valid() );
 }
 
+TEST_CASE( "generic_factory_version_wrapper", "[generic_factory]" )
+{
+    generic_factory<test_obj> test_factory( "test_factory" );
+    generic_factory<test_obj>::Version v1;
+    generic_factory<test_obj>::Version v2;
+
+    CHECK_FALSE( test_factory.is_valid( v1 ) );
+    test_factory.reset();
+    CHECK_FALSE( test_factory.is_valid( v1 ) );
+
+    v1 = test_factory.get_version();
+
+    CHECK( test_factory.is_valid( v1 ) );
+    CHECK_FALSE( v1 == v2 );
+    CHECK( v1 != v2 );
+
+    v2 = v1;
+
+    CHECK( v1 == v2 );
+    CHECK_FALSE( v1 != v2 );
+
+    test_factory.reset();
+    CHECK_FALSE( test_factory.is_valid( v1 ) );
+    CHECK_FALSE( test_factory.is_valid( v2 ) );
+}
+
 TEST_CASE( "string_ids_comparison", "[generic_factory][string_id]" )
 {
     //  checks equality correctness for the following combinations of parameters: