diff --git a/mediapipe/framework/BUILD b/mediapipe/framework/BUILD index 8224b73f..a7d9e0a6 100644 --- a/mediapipe/framework/BUILD +++ b/mediapipe/framework/BUILD @@ -1099,6 +1099,7 @@ cc_library( "//mediapipe/framework/port:ret_check", "//mediapipe/framework/port:status", ], + alwayslink = True, # Defines TestServiceCalculator ) cc_library( diff --git a/mediapipe/framework/graph_service.h b/mediapipe/framework/graph_service.h index 51caf31f..12b2ccb3 100644 --- a/mediapipe/framework/graph_service.h +++ b/mediapipe/framework/graph_service.h @@ -44,7 +44,6 @@ class GraphServiceBase { constexpr GraphServiceBase(const char* key) : key(key) {} - virtual ~GraphServiceBase() = default; inline virtual absl::StatusOr CreateDefaultObject() const { return DefaultInitializationUnsupported(); } @@ -52,14 +51,32 @@ class GraphServiceBase { const char* key; protected: + // `GraphService` objects, deriving `GraphServiceBase` are designed to be + // global constants and not ever deleted through `GraphServiceBase`. Hence, + // protected and non-virtual destructor which helps to make `GraphService` + // trivially destructible and properly defined as global constants. + // + // A class with any virtual functions should have a destructor that is either + // public and virtual or else protected and non-virtual. + // https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-virtual + ~GraphServiceBase() = default; + absl::Status DefaultInitializationUnsupported() const { return absl::UnimplementedError(absl::StrCat( "Graph service '", key, "' does not support default initialization")); } }; +// A global constant to refer a service: +// - Requesting `CalculatorContract::UseService` from calculator +// - Accessing `Calculator/SubgraphContext::Service`from calculator/subgraph +// - Setting before graph initialization `CalculatorGraph::SetServiceObject` +// +// NOTE: In headers, define your graph service reference safely as following: +// `inline constexpr GraphService kYourService("YourService");` +// template -class GraphService : public GraphServiceBase { +class GraphService final : public GraphServiceBase { public: using type = T; using packet_type = std::shared_ptr; @@ -68,7 +85,7 @@ class GraphService : public GraphServiceBase { kDisallowDefaultInitialization) : GraphServiceBase(my_key), default_init_(default_init) {} - absl::StatusOr CreateDefaultObject() const override { + absl::StatusOr CreateDefaultObject() const final { if (default_init_ != kAllowDefaultInitialization) { return DefaultInitializationUnsupported(); } diff --git a/mediapipe/framework/graph_service_manager_test.cc b/mediapipe/framework/graph_service_manager_test.cc index 1895a6f7..23d4af0d 100644 --- a/mediapipe/framework/graph_service_manager_test.cc +++ b/mediapipe/framework/graph_service_manager_test.cc @@ -7,7 +7,7 @@ namespace mediapipe { namespace { -const GraphService kIntService("mediapipe::IntService"); +constexpr GraphService kIntService("mediapipe::IntService"); } // namespace TEST(GraphServiceManager, SetGetServiceObject) { diff --git a/mediapipe/framework/graph_service_test.cc b/mediapipe/framework/graph_service_test.cc index 69992f21..0556aac6 100644 --- a/mediapipe/framework/graph_service_test.cc +++ b/mediapipe/framework/graph_service_test.cc @@ -14,6 +14,8 @@ #include "mediapipe/framework/graph_service.h" +#include + #include "mediapipe/framework/calculator_contract.h" #include "mediapipe/framework/calculator_framework.h" #include "mediapipe/framework/port/canonical_errors.h" @@ -159,7 +161,7 @@ TEST_F(GraphServiceTest, CreateDefault) { struct TestServiceData {}; -const GraphService kTestServiceAllowDefaultInitialization( +constexpr GraphService kTestServiceAllowDefaultInitialization( "kTestServiceAllowDefaultInitialization", GraphServiceBase::kAllowDefaultInitialization); @@ -272,9 +274,13 @@ TEST(AllowDefaultInitializationGraphServiceTest, HasSubstr("Service is unavailable."))); } -const GraphService kTestServiceDisallowDefaultInitialization( - "kTestServiceDisallowDefaultInitialization", - GraphServiceBase::kDisallowDefaultInitialization); +constexpr GraphService + kTestServiceDisallowDefaultInitialization( + "kTestServiceDisallowDefaultInitialization", + GraphServiceBase::kDisallowDefaultInitialization); + +static_assert(std::is_trivially_destructible_v>, + "GraphService is not trivially destructible"); class FailOnUnavailableOptionalDisallowDefaultInitServiceCalculator : public CalculatorBase { diff --git a/mediapipe/framework/test_service.cc b/mediapipe/framework/test_service.cc index 4bafaf28..e7233ebf 100644 --- a/mediapipe/framework/test_service.cc +++ b/mediapipe/framework/test_service.cc @@ -16,15 +16,6 @@ namespace mediapipe { -const GraphService kTestService( - "test_service", GraphServiceBase::kDisallowDefaultInitialization); -const GraphService kAnotherService( - "another_service", GraphServiceBase::kAllowDefaultInitialization); -const GraphService kNoDefaultService( - "no_default_service", GraphServiceBase::kAllowDefaultInitialization); -const GraphService kNeedsCreateService( - "needs_create_service", GraphServiceBase::kAllowDefaultInitialization); - absl::Status TestServiceCalculator::GetContract(CalculatorContract* cc) { cc->Inputs().Index(0).Set(); cc->Outputs().Index(0).SetSameAs(&cc->Inputs().Index(0)); diff --git a/mediapipe/framework/test_service.h b/mediapipe/framework/test_service.h index 2ff5a384..42ebd8df 100644 --- a/mediapipe/framework/test_service.h +++ b/mediapipe/framework/test_service.h @@ -22,14 +22,17 @@ namespace mediapipe { using TestServiceObject = std::map; -extern const GraphService kTestService; -extern const GraphService kAnotherService; +inline constexpr GraphService kTestService( + "test_service", GraphServiceBase::kDisallowDefaultInitialization); +inline constexpr GraphService kAnotherService( + "another_service", GraphServiceBase::kAllowDefaultInitialization); class NoDefaultConstructor { public: NoDefaultConstructor() = delete; }; -extern const GraphService kNoDefaultService; +inline constexpr GraphService kNoDefaultService( + "no_default_service", GraphServiceBase::kAllowDefaultInitialization); class NeedsCreateMethod { public: @@ -40,7 +43,8 @@ class NeedsCreateMethod { private: NeedsCreateMethod() = default; }; -extern const GraphService kNeedsCreateService; +inline constexpr GraphService kNeedsCreateService( + "needs_create_service", GraphServiceBase::kAllowDefaultInitialization); // Use a service. class TestServiceCalculator : public CalculatorBase {