IdManager.h: Improvements

* Ensure ::PtrSame<T, Derived> is true.
* Allow id_base, id_step and id_count to be of enumeration type.
* Fix potential deadlock in kernel explorer.

idm::select:
* Allow to select multiple inherited object types for idm::select.
* Allow function reference types. (they don't allow access to operator() directly, use deducing std::function constructor instead)
* Ensure ::is_same_ptr<T, object_type> is true.

rpcs3/Emu/Cell/lv2/sys_config.cpp

@@ -167,7 +167,7 @@ void lv2_config_service_listener::notify_all()
 		{
 			services.push_back(service.get_shared_ptr());
 		}
-	}, 0);
+	});
 
 	// Sort services by timestamp
 	sort(services.begin(), services.end(), [](const std::shared_ptr<lv2_config_service>& s1, const std::shared_ptr<lv2_config_service>& s2)

rpcs3/Emu/Cell/lv2/sys_fs.h

@@ -160,8 +160,6 @@ extern lv2_fs_mount_point g_mp_sys_dev_hdd1;
 
 struct lv2_fs_object
 {
-	using id_type = lv2_fs_object;
-
 	static const u32 id_base = 3;
 	static const u32 id_step = 1;
 	static const u32 id_count = 255 - id_base;

rpcs3/Emu/Cell/lv2/sys_sync.h

@@ -65,8 +65,6 @@ enum ppu_thread_status : u32;
 // Base class for some kernel objects (shared set of 8192 objects).
 struct lv2_obj
 {
-	using id_type = lv2_obj;
-
 	static const u32 id_step = 0x100;
 	static const u32 id_count = 8192;
 	static constexpr std::pair<u32, u32> id_invl_range = {0, 8};

rpcs3/Emu/IdManager.h

@@ -20,40 +20,36 @@ namespace id_manager
 	// Common global mutex
 	extern shared_mutex g_mutex;
 
+	template <typename T>
+	constexpr std::pair<u32, u32> get_invl_range()
+	{
+		return {0, 0};
+	}
+
+	template <typename T> requires requires () { T::id_invl_range; }
+	constexpr std::pair<u32, u32> get_invl_range()
+	{
+		return T::id_invl_range;
+	}
+
+	template <typename T>
+	concept IdmCompatible = requires () { T::id_base, T::id_step, T::id_count; };
+
 	// ID traits
-	template <typename T, typename = void>
+	template <typename T>
 	struct id_traits
 	{
-		static_assert(sizeof(T) == 0, "ID object must specify: id_base, id_step, id_count");
+		static_assert(IdmCompatible<T>, "ID object must specify: id_base, id_step, id_count");
 
-		static constexpr u32 base    = 1;     // First ID (N = 0)
+		enum : u32
-		static constexpr u32 step    = 1;     // Any ID: N * id_step + id_base
+		{
-		static constexpr u32 count   = 65535; // Limit: N < id_count
+			base = T::id_base, // First ID (N = 0)
-		static constexpr u32 invalid = 0;
+			step = T::id_step, // Any ID: N * id_setp + id_base
-		static constexpr std::pair<u32, u32> invl_range{0, 0};
+			count = T::id_count, // Limit: N < id_count
+			invalid = -+!base, // Invalid ID sample
 		};
 
-	template <typename T, typename = void>
+		static constexpr std::pair<u32, u32> invl_range = get_invl_range<T>();
-	struct invl_range_extract_impl
-	{
-		static constexpr std::pair<u32, u32> invl_range{0, 0};
-	};
-
-	template <typename T>
-	struct invl_range_extract_impl<T, std::void_t<decltype(&T::id_invl_range)>>
-	{
-		static constexpr std::pair<u32, u32> invl_range = T::id_invl_range;
-	};
-
-	template <typename T>
-	struct id_traits<T, std::void_t<decltype(&T::id_base), decltype(&T::id_step), decltype(&T::id_count)>>
-	{
-		static constexpr u32 base    = T::id_base;
-		static constexpr u32 step    = T::id_step;
-		static constexpr u32 count   = T::id_count;
-		static constexpr u32 invalid = -+!base;
-
-		static constexpr std::pair<u32, u32> invl_range = invl_range_extract_impl<T>::invl_range;
 
 		static_assert(count && step && u64{step} * (count - 1) + base < u32{umax} + u64{base != 0 ? 1 : 0}, "ID traits: invalid object range");
 
@@ -61,19 +57,6 @@ namespace id_manager
 		static_assert(!invl_range.second || (u64{invl_range.second} + invl_range.first <= 32 /*....*/ ));
 	};
 
-	// Correct usage testing
-	template <typename T, typename T2, typename = void>
-	struct id_verify : std::integral_constant<bool, std::is_base_of<T, T2>::value>
-	{
-		// If common case, T2 shall be derived from or equal to T
-	};
-
-	template <typename T, typename T2>
-	struct id_verify<T, T2, std::void_t<typename T2::id_type>> : std::integral_constant<bool, std::is_same<T, typename T2::id_type>::value>
-	{
-		// If T2 contains id_type type, T must be equal to it
-	};
-
 	class typeinfo
 	{
 		// Global variable for each registered type
@@ -227,20 +210,6 @@ class idm
 		using result_type = R;
 	};
 
-	template <typename F, typename A1, typename A2>
-	struct function_traits<void (F::*)(A1, A2&) const>
-	{
-		using object_type = A2;
-		using void_type   = void;
-	};
-
-	template <typename F, typename A1, typename A2>
-	struct function_traits<void (F::*)(A1, A2&)>
-	{
-		using object_type = A2;
-		using void_type   = void;
-	};
-
 	// Helper type: pointer + return value propagated
 	template <typename T, typename RT>
 	struct return_pair
@@ -296,7 +265,7 @@ class idm
 	template <typename T, typename Type>
 	static map_data* find_id(u32 id)
 	{
-		static_assert(id_manager::id_verify<T, Type>::value, "Invalid ID type combination");
+		static_assert(PtrSame<T, Type>, "Invalid ID type combination");
 
 		const u32 index = get_index<Type>(id);
 
@@ -332,7 +301,7 @@ class idm
 	template <typename T, typename Type, typename F>
 	static map_data* create_id(F&& provider)
 	{
-		static_assert(id_manager::id_verify<T, Type>::value, "Invalid ID type combination");
+		static_assert(PtrSame<T, Type>, "Invalid ID type combination");
 
 		// ID traits
 		using traits = id_manager::id_traits<Type>;
@@ -373,8 +342,8 @@ public:
 	}
 
 	// Add a new ID of specified type with specified constructor arguments (returns object or nullptr)
-	template <typename T, typename Make = T, typename... Args>
+	template <typename T, typename Make = T, typename... Args> requires (std::is_constructible_v<Make, Args&&...>)
-	static inline std::enable_if_t<std::is_constructible<Make, Args...>::value, std::shared_ptr<Make>> make_ptr(Args&&... args)
+	static inline std::shared_ptr<Make> make_ptr(Args&&... args)
 	{
 		if (auto pair = create_id<T, Make>([&] { return std::make_shared<Make>(std::forward<Args>(args)...); }))
 		{
@@ -385,8 +354,8 @@ public:
 	}
 
 	// Add a new ID of specified type with specified constructor arguments (returns id)
-	template <typename T, typename Make = T, typename... Args>
+	template <typename T, typename Make = T, typename... Args> requires (std::is_constructible_v<Make, Args&&...>)
-	static inline std::enable_if_t<std::is_constructible<Make, Args...>::value, u32> make(Args&&... args)
+	static inline u32 make(Args&&... args)
 	{
 		if (auto pair = create_id<T, Make>([&] { return std::make_shared<Make>(std::forward<Args>(args)...); }))
 		{
@@ -396,20 +365,8 @@ public:
 		return id_manager::id_traits<Make>::invalid;
 	}
 
-	// Add a new ID for an existing object provided (returns new id)
-	template <typename T, typename Made = T>
-	static inline u32 import_existing(const std::shared_ptr<T>& ptr)
-	{
-		if (auto pair = create_id<T, Made>([&] { return ptr; }))
-		{
-			return pair->first;
-		}
-
-		return id_manager::id_traits<Made>::invalid;
-	}
-
 	// Add a new ID for an object returned by provider()
-	template <typename T, typename Made = T, typename F, typename = std::invoke_result_t<F>>
+	template <typename T, typename Made = T, typename F> requires (std::is_invocable_v<F&&>)
 	static inline u32 import(F&& provider)
 	{
 		if (auto pair = create_id<T, Made>(std::forward<F>(provider)))
@@ -420,6 +377,13 @@ public:
 		return id_manager::id_traits<Made>::invalid;
 	}
 
+	// Add a new ID for an existing object provided (returns new id)
+	template <typename T, typename Made = T>
+	static inline u32 import_existing(std::shared_ptr<T> ptr)
+	{
+		return import<T, Made>([&] { return std::move(ptr); });
+	}
+
 	// Access the ID record without locking (unsafe)
 	template <typename T, typename Get = T>
 	static inline map_data* find_unlocked(u32 id)
@@ -497,14 +461,7 @@ public:
 	{
 		reader_lock lock(id_manager::g_mutex);
 
-		const auto found = find_id<T, Get>(id);
+		return get_unlocked<T, Get>(id);
-
-		if (found == nullptr) [[unlikely]]
-		{
-			return nullptr;
-		}
-
-		return std::static_pointer_cast<Get>(found->second);
 	}
 
 	// Get the object, access object under reader lock
@@ -533,57 +490,46 @@ public:
 		}
 	}
 
+	static constexpr std::false_type unlocked{};
+
 	// Access all objects of specified type. Returns the number of objects processed.
-	template <typename T, typename Get = T, typename F, typename FT = decltype(&std::decay_t<F>::operator()), typename FRT = typename function_traits<FT>::void_type>
+	// If function result evaluates to true, stop and return the object and the value.
-	static inline u32 select(F&& func, int = 0)
+	template <typename T, typename... Get, typename F, typename Lock = std::true_type>
+	static inline auto select(F&& func, Lock = {})
 	{
-		static_assert(id_manager::id_verify<T, Get>::value, "Invalid ID type combination");
+		static_assert((PtrSame<T, Get> && ...), "Invalid ID type combination");
 
-		reader_lock lock(id_manager::g_mutex);
+		std::conditional_t<static_cast<bool>(Lock()), reader_lock, const shared_mutex&> lock(id_manager::g_mutex);
 
-		u32 result = 0;
+		using func_traits = function_traits<decltype(&decltype(std::function(std::declval<F>()))::operator())>;
+		using object_type = typename func_traits::object_type;
+		using result_type = typename func_traits::result_type;
 
-		for (auto& id : g_fxo->get<id_manager::id_map<T>>().vec)
+		static_assert(PtrSame<object_type, T>, "Invalid function argument type combination");
-		{
-			if (id.second)
-			{
-				if (std::is_same<T, Get>::value || id.first.type() == get_type<Get>())
-				{
-					func(id.first, *static_cast<typename function_traits<FT>::object_type*>(id.second.get()));
-					result++;
-				}
-			}
-		}
 
-		return result;
+		std::conditional_t<std::is_void_v<result_type>, u32, return_pair<object_type, result_type>> result{};
-	}
-
-	// Access all objects of specified type. If function result evaluates to true, stop and return the object and the value.
-	template <typename T, typename Get = T, typename F, typename FT = decltype(&std::decay_t<F>::operator()), typename FRT = typename function_traits<FT>::result_type>
-	static inline auto select(F&& func)
-	{
-		static_assert(id_manager::id_verify<T, Get>::value, "Invalid ID type combination");
-
-		using object_type = typename function_traits<FT>::object_type;
-		using result_type = return_pair<object_type, FRT>;
-
-		reader_lock lock(id_manager::g_mutex);
 
 		for (auto& id : g_fxo->get<id_manager::id_map<T>>().vec)
 		{
 			if (auto ptr = static_cast<object_type*>(id.second.get()))
 			{
-				if (std::is_same<T, Get>::value || id.first.type() == get_type<Get>())
+				if (sizeof...(Get) == 0 || ((id.first.type() == get_type<Get>()) || ...))
 				{
-					if (FRT result = func(id.first, *ptr))
+					if constexpr (std::is_void_v<result_type>)
 					{
-						return result_type{{id.second, ptr}, std::move(result)};
+						func(id.first, *ptr);
+						result++;
+					}
+					else if ((result.ret = func(id.first, *ptr)))
+					{
+						result.ptr = {id.second, ptr};
+						break;
 					}
 				}
 			}
 		}
 
-		return result_type{nullptr};
+		return result;
 	}
 
 	// Remove the ID

rpcs3/rpcs3qt/kernel_explorer.cpp

@@ -578,7 +578,7 @@ void kernel_explorer::update()
 		add_leaf(find_node(root, additional_nodes::memory_containers), qstr(fmt::format("Memory Container 0x%08x: Used: 0x%x/0x%x (%0.2f/%0.2f MB)", id, used, container.size, used * 1. / (1024 * 1024), container.size * 1. / (1024 * 1024))));
 	});
 
-	std::unique_lock lock_lv2(lv2_obj::g_mutex);
+	std::optional<std::scoped_lock<shared_mutex, shared_mutex>> lock_idm_lv2(std::in_place, id_manager::g_mutex, lv2_obj::g_mutex);
 
 	idm::select<named_thread<ppu_thread>>([&](u32 id, ppu_thread& ppu)
 	{
@@ -587,9 +587,9 @@ void kernel_explorer::update()
 
 		add_leaf(find_node(root, additional_nodes::ppu_threads), qstr(fmt::format(u8"PPU 0x%07x: “%s”, PRIO: %d, Joiner: %s, Status: %s, State: %s, %s func: “%s”", id, *ppu.ppu_tname.load(), +ppu.prio, ppu.joiner.load(), status, ppu.state.load()
 			, ppu.current_function ? "In" : "Last", func ? func : "")));
-	});
+	}, idm::unlocked);
 
-	lock_lv2.unlock();
+	lock_idm_lv2.reset();
 
 	idm::select<named_thread<spu_thread>>([&](u32 /*id*/, spu_thread& spu)
 	{

rpcs3/util/types.hpp

@@ -1039,7 +1039,7 @@ concept PtrCastable = requires(const volatile X* x, const volatile Y* y)
 };
 
 template <typename X, typename Y> requires PtrCastable<X, Y>
-constexpr bool is_same_ptr()
+consteval bool is_same_ptr()
 {
 	if constexpr (std::is_void_v<X> || std::is_void_v<Y> || std::is_same_v<std::remove_cv_t<X>, std::remove_cv_t<Y>>)
 	{
@@ -1050,8 +1050,6 @@ constexpr bool is_same_ptr()
 		return true;
 	}
 	else
-	{
-		if (std::is_constant_evaluated())
 	{
 		bool result = false;
 
@@ -1072,13 +1070,6 @@ constexpr bool is_same_ptr()
 
 		return result;
 	}
-		else
-		{
-			std::aligned_union_t<0, X, Y> s;
-			Y* ptr = reinterpret_cast<Y*>(&s);
-			return static_cast<X*>(ptr) == static_cast<void*>(ptr);
-		}
-	}
 }
 
 template <typename X, typename Y> requires PtrCastable<X, Y>