From 2c82f67097b00f170861e218de5c73d9275d0c38 Mon Sep 17 00:00:00 2001 From: MediaPipe Team Date: Fri, 10 Feb 2023 16:55:26 -0800 Subject: [PATCH] Add location info in registry (debug mode only) PiperOrigin-RevId: 508786558 --- mediapipe/framework/api2/node.h | 22 ++++++++------- mediapipe/framework/calculator_base_test.cc | 3 ++- mediapipe/framework/deps/registration.h | 30 +++++++++++++++++---- mediapipe/framework/packet.h | 3 ++- mediapipe/framework/subgraph.cc | 6 ++--- 5 files changed, 44 insertions(+), 20 deletions(-) diff --git a/mediapipe/framework/api2/node.h b/mediapipe/framework/api2/node.h index 7061afcae..14c098246 100644 --- a/mediapipe/framework/api2/node.h +++ b/mediapipe/framework/api2/node.h @@ -88,7 +88,8 @@ struct NodeRegistrationStatic { static mediapipe::RegistrationToken Make() { return mediapipe::CalculatorBaseRegistry::Register( T::kCalculatorName, - absl::make_unique>); + absl::make_unique>, + __FILE__, __LINE__); } using RequireStatics = ForceStaticInstantiation<®istration>; @@ -104,8 +105,8 @@ struct SubgraphRegistrationImpl { static NoDestructor registration; static mediapipe::RegistrationToken Make() { - return mediapipe::SubgraphRegistry::Register(T::kCalculatorName, - absl::make_unique); + return mediapipe::SubgraphRegistry::Register( + T::kCalculatorName, absl::make_unique, __FILE__, __LINE__); } using RequireStatics = ForceStaticInstantiation<®istration>; @@ -223,12 +224,13 @@ class SubgraphImpl : public Subgraph, public Intf { // This macro is used to register a calculator that does not use automatic // registration. Deprecated. -#define MEDIAPIPE_NODE_IMPLEMENTATION(Impl) \ - static mediapipe::NoDestructor \ - REGISTRY_STATIC_VAR(calculator_registration, \ - __LINE__)(mediapipe::CalculatorBaseRegistry::Register( \ - Impl::kCalculatorName, \ - absl::make_unique>)) +#define MEDIAPIPE_NODE_IMPLEMENTATION(Impl) \ + static mediapipe::NoDestructor \ + REGISTRY_STATIC_VAR(calculator_registration, \ + __LINE__)(mediapipe::CalculatorBaseRegistry::Register( \ + Impl::kCalculatorName, \ + absl::make_unique>, \ + __FILE__, __LINE__)) // This macro is used to register a non-split-contract calculator. Deprecated. #define MEDIAPIPE_REGISTER_NODE(name) REGISTER_CALCULATOR(name) @@ -239,7 +241,7 @@ class SubgraphImpl : public Subgraph, public Intf { static mediapipe::NoDestructor \ REGISTRY_STATIC_VAR(subgraph_registration, \ __LINE__)(mediapipe::SubgraphRegistry::Register( \ - Impl::kCalculatorName, absl::make_unique)) + Impl::kCalculatorName, absl::make_unique, __FILE__, __LINE__)) } // namespace api2 } // namespace mediapipe diff --git a/mediapipe/framework/calculator_base_test.cc b/mediapipe/framework/calculator_base_test.cc index 42c03696c..c26006e0f 100644 --- a/mediapipe/framework/calculator_base_test.cc +++ b/mediapipe/framework/calculator_base_test.cc @@ -183,7 +183,8 @@ TEST(CalculatorTest, CreateByNameWhitelisted) { CalculatorBaseRegistry::Register( "::mediapipe::test_ns::whitelisted_ns::DeadCalculator", absl::make_unique>); + mediapipe::test_ns::whitelisted_ns::DeadCalculator>>, + __FILE__, __LINE__); // A whitelisted calculator can be found in its own namespace. MP_EXPECT_OK(CalculatorBaseRegistry::CreateByNameInNamespace( // diff --git a/mediapipe/framework/deps/registration.h b/mediapipe/framework/deps/registration.h index 9d80aafea..cc8ba03fe 100644 --- a/mediapipe/framework/deps/registration.h +++ b/mediapipe/framework/deps/registration.h @@ -16,6 +16,7 @@ #define MEDIAPIPE_DEPS_REGISTRATION_H_ #include +#include #include #include #include @@ -161,7 +162,8 @@ class FunctionRegistry { FunctionRegistry(const FunctionRegistry&) = delete; FunctionRegistry& operator=(const FunctionRegistry&) = delete; - RegistrationToken Register(absl::string_view name, Function func) + RegistrationToken Register(absl::string_view name, Function func, + std::string filename, uint64_t line) ABSL_LOCKS_EXCLUDED(lock_) { std::string normalized_name = GetNormalizedName(name); absl::WriterMutexLock lock(&lock_); @@ -171,10 +173,21 @@ class FunctionRegistry { } if (functions_.insert(std::make_pair(normalized_name, std::move(func))) .second) { +#ifndef NDEBUG + locations_.emplace(normalized_name, + std::make_pair(std::move(filename), line)); +#endif return RegistrationToken( [this, normalized_name]() { Unregister(normalized_name); }); } +#ifndef NDEBUG + LOG(FATAL) << "Function with name " << name << " already registered." + << " First registration at " + << locations_.at(normalized_name).first << ":" + << locations_.at(normalized_name).second; +#else LOG(FATAL) << "Function with name " << name << " already registered."; +#endif return RegistrationToken([]() {}); } @@ -291,6 +304,11 @@ class FunctionRegistry { private: mutable absl::Mutex lock_; absl::flat_hash_map functions_ ABSL_GUARDED_BY(lock_); +#ifndef NDEBUG + // Stores filename and line number for useful debug log. + absl::flat_hash_map> locations_ + ABSL_GUARDED_BY(lock_); +#endif // For names included in NamespaceAllowlist, strips the namespace. std::string GetAdjustedName(absl::string_view name) { @@ -321,8 +339,10 @@ class GlobalFactoryRegistry { public: static RegistrationToken Register(absl::string_view name, - typename Functions::Function func) { - return functions()->Register(name, std::move(func)); + typename Functions::Function func, + std::string filename, uint64_t line) { + return functions()->Register(name, std::move(func), std::move(filename), + line); } // Invokes the specified factory function and returns the result. @@ -382,12 +402,12 @@ class GlobalFactoryRegistry { #define MEDIAPIPE_REGISTER_FACTORY_FUNCTION(RegistryType, name, ...) \ static auto* REGISTRY_STATIC_VAR(registration_##name, __LINE__) = \ new mediapipe::RegistrationToken( \ - RegistryType::Register(#name, __VA_ARGS__)) + RegistryType::Register(#name, __VA_ARGS__, __FILE__, __LINE__)) #define REGISTER_FACTORY_FUNCTION_QUALIFIED(RegistryType, var_name, name, ...) \ static auto* REGISTRY_STATIC_VAR(var_name, __LINE__) = \ new mediapipe::RegistrationToken( \ - RegistryType::Register(#name, __VA_ARGS__)) + RegistryType::Register(#name, __VA_ARGS__, __FILE__, __LINE__)) } // namespace mediapipe diff --git a/mediapipe/framework/packet.h b/mediapipe/framework/packet.h index 1024cbc15..af2ec5a98 100644 --- a/mediapipe/framework/packet.h +++ b/mediapipe/framework/packet.h @@ -466,7 +466,8 @@ struct MessageRegistrationImpl { template NoDestructor MessageRegistrationImpl::registration(MessageHolderRegistry::Register( - T{}.GetTypeName(), MessageRegistrationImpl::CreateMessageHolder)); + T{}.GetTypeName(), MessageRegistrationImpl::CreateMessageHolder, + __FILE__, __LINE__)); // For non-Message payloads, this does nothing. template diff --git a/mediapipe/framework/subgraph.cc b/mediapipe/framework/subgraph.cc index 7cbde28bf..6c18c9cac 100644 --- a/mediapipe/framework/subgraph.cc +++ b/mediapipe/framework/subgraph.cc @@ -64,13 +64,13 @@ GraphRegistry::GraphRegistry( void GraphRegistry::Register( const std::string& type_name, std::function()> factory) { - local_factories_.Register(type_name, factory); + local_factories_.Register(type_name, factory, __FILE__, __LINE__); } // TODO: Remove this convenience function. void GraphRegistry::Register(const std::string& type_name, const CalculatorGraphConfig& config) { - local_factories_.Register(type_name, [config] { + Register(type_name, [config] { auto result = absl::make_unique(config); return std::unique_ptr(result.release()); }); @@ -79,7 +79,7 @@ void GraphRegistry::Register(const std::string& type_name, // TODO: Remove this convenience function. void GraphRegistry::Register(const std::string& type_name, const CalculatorGraphTemplate& templ) { - local_factories_.Register(type_name, [templ] { + Register(type_name, [templ] { auto result = absl::make_unique(templ); return std::unique_ptr(result.release()); });