Use unordered_set to deduplicate active items (#66310)

* index active items using unordered_set

* Add invalidation of the index cache

* Fix include

* clear the index when invalidated

* rearrange

* Comment updates

* andrei's patch: use safe_reference instead of bare pointers

Co-authored-by:  andrei8l <[email protected]>

* Remove remove() dead code

* Try to satisfy clang-tidy

* Make it build

---------

Co-authored-by: andrei8l <[email protected]>

src/active_item_cache.cpp

@@ -6,19 +6,6 @@
 #include "item.h"
 #include "safe_reference.h"
 
-namespace
-{
-
-void _remove_if( std::list<item_reference> &active_items, item const *it )
-{
-    active_items.remove_if( [it]( const item_reference & active_item ) {
-        item *const target = active_item.item_ref.get();
-        return !target || target == it;
-    } );
-}
-
-} // namespace
-
 float item_reference::spoil_multiplier()
 {
     return std::accumulate(
@@ -28,27 +15,6 @@ float item_reference::spoil_multiplier()
     } );
 }
 
-void active_item_cache::remove( const item *it )
-{
-    for( item const *iter : it->all_items_ptr() ) {
-        _remove_if( active_items[iter->processing_speed()], iter );
-    }
-    _remove_if( active_items[it->processing_speed()], it );
-    if( it->can_revive() ) {
-        special_items[ special_item_type::corpse ].remove_if( [it]( const item_reference & active_item ) {
-            item *const target = active_item.item_ref.get();
-            return !target || target == it;
-        } );
-    }
-    if( it->get_use( "explosion" ) ) {
-        special_items[ special_item_type::explosive ].remove_if( [it]( const item_reference &
-        active_item ) {
-            item *const target = active_item.item_ref.get();
-            return !target || target == it;
-        } );
-    }
-}
-
 bool active_item_cache::add( item &it, point location, item *parent,
                              std::vector<item_pocket const *> const &pocket_chain )
 {
@@ -60,15 +26,21 @@ bool active_item_cache::add( item &it, point location, item *parent,
             ret |= add( *pkit, location, &it, pockets );
         }
     }
-    if( it.processing_speed() == item::NO_PROCESSING ) {
+
+    int speed = it.processing_speed();
+    if( speed == item::NO_PROCESSING ) {
         return ret;
     }
-    std::list<item_reference> &target_list = active_items[it.processing_speed()];
+    std::unordered_set<safe_reference<item>> &target_index = active_items_index[speed];
+    std::list<item_reference> &target_list = active_items[speed];
+    if( target_index.empty() ) {
+        // If the index has been cleared, rebuild it first.
+        for( item_reference &iter : target_list ) {
+            target_index.emplace( iter.item_ref );
+        }
+    }
     // If the item is already in the cache for some reason, don't add a second reference
-    if( std::find_if( target_list.begin(),
-    target_list.end(), [&it]( const item_reference & active_item_ref ) {
-    return &it == active_item_ref.item_ref.get();
-    } ) != target_list.end() ) {
+    if( target_index.find( it.get_safe_reference() ) != target_index.end() ) {
         return true;
     }
     item_reference ref{ location, it.get_safe_reference(), parent, pocket_chain };
@@ -79,6 +51,7 @@ bool active_item_cache::add( item &it, point location, item *parent,
         special_items[special_item_type::explosive].emplace_back( ref );
     }
     target_list.emplace_back( std::move( ref ) );
+    target_index.emplace( it.get_safe_reference() );
     return true;
 }
 
@@ -98,6 +71,7 @@ std::vector<item_reference> active_item_cache::get()
                 all_cached_items.emplace_back( *it );
                 ++it;
             } else {
+                active_items_index[kv.first].clear();
                 it = kv.second.erase( it );
             }
         }
@@ -123,6 +97,8 @@ std::vector<item_reference> active_item_cache::get_for_processing()
                 ++it;
             } else {
                 // The item has been destroyed, so remove the reference from the cache
+                // Also invalidate the index
+                active_items_index[kv.first].clear();
                 it = kv.second.erase( it );
             }
         }

src/active_item_cache.h

@@ -5,6 +5,7 @@
 #include <cstddef>
 #include <list>
 #include <unordered_map>
+#include <unordered_set>
 #include <vector>
 
 #include "point.h"
@@ -38,22 +39,23 @@ struct hash<special_item_type> {
         return static_cast<size_t>( k );
     }
 };
+
+template<>
+struct hash<safe_reference<item>> {
+    std::size_t operator()( safe_reference<item > const &s ) const noexcept {
+        return hash<item * > {}( s.get() );
+    }
+};
 } // namespace std
 
 class active_item_cache
 {
     private:
         std::unordered_map<int, std::list<item_reference>> active_items;
         std::unordered_map<special_item_type, std::list<item_reference>> special_items;
+        std::unordered_map<int, std::unordered_set<safe_reference<item>>> active_items_index;
 
     public:
-        /**
-         * Removes the item if it is in the cache. Does nothing if the item is not in the cache.
-         * Relies on the fact that item::processing_speed() is a constant.
-         * Also removes any items that have been destroyed in the list containing it
-         */
-        void remove( const item *it );
-
         /**
          * Adds the reference to the cache. Does nothing if the reference is already in the cache.
          * Relies on the fact that item::processing_speed() is a constant.

src/safe_reference.h

@@ -53,6 +53,12 @@ class safe_reference
         std::weak_ptr<T> impl;
 };
 
+template<typename T>
+constexpr bool operator==( safe_reference<T> const &lhs, safe_reference<T> const &rhs )
+{
+    return lhs && rhs && lhs.get() == rhs.get();
+}
+
 class safe_reference_anchor
 {
     public: